From f4415c21c57dd2caa8638b4e7f9670a9f563e88c Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 19 Jul 2006 20:25:09 +0000 Subject: [PATCH] mapper compilation work ongoing, someday it'll work....moved around the initialization of MapperProperty objects to be after all mappers are created to better handle circular compilations. do_init() method is called on all properties now which are more aware of their "inherited" status if so. eager loads explicitly disallowed on self-referential relationships, or relationships to an inheriting mapper (which is also self-referential) --- CHANGES | 7 +++ lib/sqlalchemy/orm/mapper.py | 92 ++++++++++++++++++++------------ lib/sqlalchemy/orm/properties.py | 61 ++++++++++++--------- test/orm/cycles.py | 15 ++++++ test/orm/polymorph.py | 26 ++++++++- 5 files changed, 139 insertions(+), 62 deletions(-) diff --git a/CHANGES b/CHANGES index 8875727c29..f96d321c5e 100644 --- a/CHANGES +++ b/CHANGES @@ -35,6 +35,13 @@ and allowing the original propname to be accessible in select_by(). - fix to typing in clause construction which specifically helps type issues with polymorphic_union (CAST/ColumnClause propigates its type to proxy columns) +- mapper compilation work ongoing, someday it'll work....moved +around the initialization of MapperProperty objects to be after +all mappers are created to better handle circular compilations. +do_init() method is called on all properties now which are more +aware of their "inherited" status if so. +- eager loads explicitly disallowed on self-referential relationships, or +relationships to an inheriting mapper (which is also self-referential) 0.2.5 - fixed endless loop bug in select_by(), if the traversal hit diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index de126f3d9c..8c8ce4cffc 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -149,18 +149,29 @@ class Mapper(object): if self.__is_compiled: return self - self._do_compile() - - # look for another mapper thats not compiled, and compile it. - # this will utlimately compile all mappers, including any that - # need to set up backrefs on this mapper. + # compile all primary mappers for mapper in mapper_registry.values(): if not mapper.__is_compiled: - mapper.compile() - break + mapper._do_compile() + + # initialize properties on all mappers + for mapper in mapper_registry.values(): + if not mapper.__props_init: + mapper._initialize_properties() + + # if we're not primary, compile us + if self.non_primary: + self._do_compile() + self._initialize_properties() return self - + + def _check_compile(self): + if self.non_primary: + self._do_compile() + self._initialize_properties() + return self + def _do_compile(self): """compile this mapper into its final internal format. @@ -171,12 +182,13 @@ class Mapper(object): return self #print "COMPILING!", self.class_key, "non primary: ", self.non_primary self.__is_compiled = True + self.__props_init = False self._compile_extensions() self._compile_inheritance() self._compile_tables() self._compile_properties() self._compile_selectable() - self._initialize_properties() +# self._initialize_properties() return self @@ -336,9 +348,7 @@ class Mapper(object): self.inherits._inheriting_mappers.add(self) for key, prop in self.inherits.__props.iteritems(): if not self.__props.has_key(key): - p = prop.copy() - if p.adapt(self): - self._compile_property(key, p, init=False) + p = prop.adapt_to_inherited(key, self) # load properties from the main table object, # not overriding those set up in the 'properties' argument @@ -352,6 +362,7 @@ class Mapper(object): if prop is None: prop = ColumnProperty(column) self.__props[column.key] = prop + prop.set_parent(self) elif isinstance(prop, ColumnProperty): prop.columns.append(column) else: @@ -372,7 +383,8 @@ class Mapper(object): for key, prop in l: if getattr(prop, 'key', None) is None: prop.init(key, self) - + self.__props_init = True + def _compile_selectable(self): """if the 'select_table' keyword argument was specified, set up a second "surrogate mapper" that will be used for select operations. @@ -429,7 +441,7 @@ class Mapper(object): if session is not None and mapper is not None: self._entity_name = entity_name session._register_new(self) - + if oldinit is not None: try: oldinit(self, *args, **kwargs) @@ -452,12 +464,19 @@ class Mapper(object): self.class_.c = self.c def base_mapper(self): - """returns the ultimate base mapper in an inheritance chain""" + """return the ultimate base mapper in an inheritance chain""" if self.inherits is not None: return self.inherits.base_mapper() else: return self + def isa(self, other): + """return True if the given mapper inherits from this mapper""" + m = other + while m is not self and m.inherits is not None: + m = m.inherits + return m is self + def add_properties(self, dict_of_properties): """adds the given dictionary of properties to this mapper, using add_property.""" for key, value in dict_of_properties.iteritems(): @@ -505,7 +524,8 @@ class Mapper(object): raise exceptions.ArgumentError("'%s' is not an instance of MapperProperty or Column" % repr(prop)) self.__props[key] = prop - + prop.set_parent(self) + if isinstance(prop, ColumnProperty): col = self.select_table.corresponding_column(prop.columns[0], keys_ok=True, raiseerr=False) if col is None: @@ -519,9 +539,7 @@ class Mapper(object): prop.init(key, self) for mapper in self._inheriting_mappers: - p = prop.copy() - if p.adapt(mapper): - mapper._compile_property(key, p, init=False) + p = prop.adapt_to_inherited(key, mapper) def __str__(self): return "Mapper|" + self.class_.__name__ + "|" + (self.entity_name is not None and "/%s" % self.entity_name or "") + str(self.local_table) @@ -1097,22 +1115,26 @@ class MapperProperty(object): def setup(self, key, statement, **options): """called when a statement is being constructed. """ return self + def set_parent(self, parent): + self.parent = parent def init(self, key, parent): - """called during Mapper compilation to compile each MapperProperty.""" + """called after all mappers are compiled to assemble relationships between + mappers, establish instrumented class attributes""" self.key = key - self.parent = parent self.localparent = parent - self.do_init(key, parent) - def adapt(self, newparent): - """adapts this MapperProperty to a new parent, assuming the new parent is an inheriting - descendant of the old parent. Should return True if the adaptation was successful, or - False if this MapperProperty cannot be adapted to the new parent (the case for "False" is, - the parent mapper has a polymorphic select, and this property represents a column that is not - represented in the new mapper's mapped table)""" - #self.parent = newparent - self.localparent = newparent - return True - def do_init(self, key, parent): + if not hasattr(self, 'inherits'): + self.inherits = None + self.do_init() + def adapt_to_inherited(self, key, newparent): + """adapt this MapperProperty to a new parent, assuming the new parent is an inheriting + descendant of the old parent. """ + p = self.copy() + newparent._compile_property(key, p, init=False) + p.localparent = newparent + p.parent = self.parent + p.inherits = getattr(self, 'inherits', self) + return p + def do_init(self): """template method for subclasses""" pass def register_deleted(self, object, uow): @@ -1127,7 +1149,7 @@ class MapperProperty(object): """a return value of True indicates we are the primary MapperProperty for this loader's attribute on our mapper's class. It means we can set the object's attribute behavior at the class level. otherwise we have to set attribute behavior on a per-instance level.""" - return self.parent._is_primary_mapper() + return self.inherits is None and self.parent._is_primary_mapper() class SynonymProperty(MapperProperty): """a marker object used by query.select_by to allow a property name to refer to another. @@ -1342,8 +1364,8 @@ def has_mapper(object): def object_mapper(object, raiseerror=True): """given an object, returns the primary Mapper associated with the object instance""" try: - mapper = mapper_registry[ClassKey(object.__class__, getattr(object, '_entity_name'))] - except (KeyError, AttributeError): + mapper = mapper_registry[ClassKey(object.__class__, getattr(object, '_entity_name', None))] + except (KeyError, AttributeError): if raiseerror: raise exceptions.InvalidRequestError("Class '%s' entity name '%s' has no mapper associated with it" % (object.__class__.__name__, getattr(object, '_entity_name', None))) else: diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 752ea23af1..6b3bb7883a 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -37,11 +37,11 @@ class ColumnProperty(mapper.MapperProperty): statement.append_column(eagertable.corresponding_column(c)) else: statement.append_column(c) - def do_init(self, key, parent): + def do_init(self): # establish a SmartProperty property manager on the object for this key - if parent._is_primary_mapper(): + if self.is_primary(): #print "regiser col on class %s key %s" % (parent.class_.__name__, key) - sessionlib.attribute_manager.register_attribute(parent.class_, key, uselist = False) + sessionlib.attribute_manager.register_attribute(self.parent.class_, self.key, uselist = False) def execute(self, session, instance, row, identitykey, imap, isnew): if isnew: #print "POPULATING OBJ", instance.__class__.__name__, "COL", self.columns[0]._label, "WITH DATA", row[self.columns[0]], "ROW IS A", row.__class__.__name__, "COL ID", id(self.columns[0]) @@ -59,11 +59,11 @@ class DeferredColumnProperty(ColumnProperty): ColumnProperty.__init__(self, *columns) def copy(self): return DeferredColumnProperty(*self.columns) - def do_init(self, key, parent): + def do_init(self): # establish a SmartProperty property manager on the object for this key, # containing a callable to load in the attribute if self.is_primary(): - sessionlib.attribute_manager.register_attribute(parent.class_, key, uselist=False, callable_=lambda i:self.setup_loader(i)) + sessionlib.attribute_manager.register_attribute(self.parent.class_, self.key, uselist=False, callable_=lambda i:self.setup_loader(i)) def setup_loader(self, instance): if not self.localparent.is_assigned(instance): return mapper.object_mapper(instance).props[self.key].setup_loader(instance) @@ -180,7 +180,7 @@ class PropertyLoader(mapper.MapperProperty): x.__dict__.update(self.__dict__) return x - def do_init_subclass(self, key, parent): + def do_init_subclass(self): """template method for subclasses of PropertyLoader""" pass @@ -191,14 +191,13 @@ class PropertyLoader(mapper.MapperProperty): else: return self.argument.class_ - def do_init(self, key, parent): - import sqlalchemy.orm + def do_init(self): if isinstance(self.argument, type): - self.mapper = mapper.class_mapper(self.argument, compile=False)._do_compile() + self.mapper = mapper.class_mapper(self.argument, compile=False)._check_compile() else: - self.mapper = self.argument._do_compile() + self.mapper = self.argument._check_compile() - self.mapper = self.mapper.get_select_mapper()._do_compile() + self.mapper = self.mapper.get_select_mapper()._check_compile() if self.association is not None: if isinstance(self.association, type): @@ -213,10 +212,10 @@ class PropertyLoader(mapper.MapperProperty): if self.secondaryjoin is None: self.secondaryjoin = sql.join(self.mapper.unjoined_table, self.secondary).onclause if self.primaryjoin is None: - self.primaryjoin = sql.join(parent.unjoined_table, self.secondary).onclause + self.primaryjoin = sql.join(self.parent.unjoined_table, self.secondary).onclause else: if self.primaryjoin is None: - self.primaryjoin = sql.join(parent.unjoined_table, self.target).onclause + self.primaryjoin = sql.join(self.parent.unjoined_table, self.target).onclause # if the foreign key wasnt specified and theres no assocaition table, try to figure # out who is dependent on who. we dont need all the foreign keys represented in the join, @@ -237,9 +236,19 @@ class PropertyLoader(mapper.MapperProperty): if self.uselist is None: self.uselist = True - self._compile_synchronizers() - self._dependency_processor = dependency.create_dependency_processor(self.key, self.syncrules, self.cascade, secondary=self.secondary, association=self.association, is_backref=self.is_backref, post_update=self.post_update) + + if self.inherits is not None: + if hasattr(self.inherits, '_dependency_processor'): + self._dependency_processor = self.inherits._dependency_processor + + if not hasattr(self, '_dependency_processor'): + self._compile_synchronizers() + self._dependency_processor = dependency.create_dependency_processor(self.key, self.syncrules, self.cascade, secondary=self.secondary, association=self.association, is_backref=self.is_backref, post_update=self.post_update) + if self.inherits is not None and not hasattr(self.inherits, '_dependency_processor'): + self.inherits._dependency_processor = self._dependency_processor + + # primary property handler, set up class attributes if self.is_primary(): # if a backref name is defined, set up an extension to populate @@ -248,14 +257,14 @@ class PropertyLoader(mapper.MapperProperty): self.attributeext = self.backref.get_extension() # set our class attribute - self._set_class_attribute(parent.class_, key) + self._set_class_attribute(self.parent.class_, self.key) if self.backref is not None: self.backref.compile(self) - elif not sessionlib.attribute_manager.is_class_managed(parent.class_, key): - raise exceptions.ArgumentError("Attempting to assign a new relation '%s' to a non-primary mapper on class '%s'. New relations can only be added to the primary mapper, i.e. the very first mapper created for class '%s' " % (key, parent.class_.__name__, parent.class_.__name__)) + elif self.inherits is None and not sessionlib.attribute_manager.is_class_managed(self.parent.class_, self.key): + raise exceptions.ArgumentError("Attempting to assign a new relation '%s' to a non-primary mapper on class '%s'. New relations can only be added to the primary mapper, i.e. the very first mapper created for class '%s' " % (self.key, self.parent.class_.__name__, self.parent.class_.__name__)) - self.do_init_subclass(key, parent) + self.do_init_subclass() def _register_attribute(self, class_, callable_=None): sessionlib.attribute_manager.register_attribute(class_, self.key, uselist = self.uselist, extension=self.attributeext, cascade=self.cascade, trackparent=True, callable_=callable_) @@ -332,7 +341,7 @@ class PropertyLoader(mapper.MapperProperty): self.syncrules.compile(self.primaryjoin, parent_tables, target_tables) class LazyLoader(PropertyLoader): - def do_init_subclass(self, key, parent): + def do_init_subclass(self): (self.lazywhere, self.lazybinds, self.lazyreverse) = create_lazy_clause(self.parent.unjoined_table, self.primaryjoin, self.secondaryjoin, self.foreignkey) # determine if our "lazywhere" clause is the same as the mapper's # get() clause. then we can just use mapper.get() @@ -440,10 +449,12 @@ def create_lazy_clause(table, primaryjoin, secondaryjoin, foreignkey): class EagerLoader(LazyLoader): """loads related objects inline with a parent query.""" - def do_init_subclass(self, key, parent, recursion_stack=None): + def do_init_subclass(self, recursion_stack=None): + if self.parent.isa(self.mapper): + raise exceptions.ArgumentError("Error creating eager relationship '%s' on parent class '%s' to child class '%s': Cant use eager loading on a self referential relationship." % (self.key, repr(self.parent.class_), repr(self.mapper.class_))) if recursion_stack is None: - LazyLoader.do_init_subclass(self, key, parent) - parent._has_eager = True + LazyLoader.do_init_subclass(self) + self.parent._has_eager = True self.eagertarget = self.target.alias() if self.secondary: @@ -500,7 +511,7 @@ class EagerLoader(LazyLoader): self.mapper.props[prop.key] = p # print "we are:", id(self), self.target.name, (self.secondary and self.secondary.name or "None"), self.parent.mapped_table.name # print "prop is",id(prop), prop.target.name, (prop.secondary and prop.secondary.name or "None"), prop.parent.mapped_table.name - p.do_init_subclass(prop.key, prop.parent, recursion_stack) + p.do_init_subclass(recursion_stack) p._create_eager_chain(recursion_stack=recursion_stack) p.eagerprimary = p.eagerprimary.copy_container() # aliasizer = sql_util.Aliasizer(p.parent.mapped_table, aliases={p.parent.mapped_table:self.eagertarget}) @@ -723,7 +734,7 @@ class EagerLazyOption(GenericOption): oldprop = mapper.props[key] newprop = class_.__new__(class_) newprop.__dict__.update(oldprop.__dict__) - newprop.do_init_subclass(key, mapper) + newprop.do_init_subclass() mapper._compile_property(key, newprop) class DeferredOption(GenericOption): diff --git a/test/orm/cycles.py b/test/orm/cycles.py index fb051f47ae..8a41613649 100644 --- a/test/orm/cycles.py +++ b/test/orm/cycles.py @@ -79,7 +79,22 @@ class SelfReferentialTest(AssertMixin): sess.delete(a) sess.flush() + def testeagerassertion(self): + """test that an eager self-referential relationship raises an error.""" + class C1(Tester): + pass + class C2(Tester): + pass + + m1 = mapper(C1, t1, properties = { + 'c1s' : relation(C1, lazy=False), + }) + try: + m1.compile() + assert False + except exceptions.ArgumentError: + assert True class BiDirectionalOneToManyTest(AssertMixin): """tests two mappers with a one-to-many relation to each other.""" def setUpAll(self): diff --git a/test/orm/polymorph.py b/test/orm/polymorph.py index 34b7ddde22..410af94d81 100644 --- a/test/orm/polymorph.py +++ b/test/orm/polymorph.py @@ -105,8 +105,7 @@ class MultipleTableTest(testbase.PersistTest): print session.query(Person).select() def testcompile2(self): - """this test fails. mapper compilation completely doesnt work for this right now and likely - needs to be rewritten again.""" + """test that a mapper can reference a property whose mapper inherits from this one.""" person_join = polymorphic_union( { 'engineer':people.join(engineers), 'manager':people.join(managers), @@ -124,6 +123,29 @@ class MultipleTableTest(testbase.PersistTest): #person_mapper.compile() class_mapper(Manager).compile() + + def testcompile3(self): + """test that a mapper referencing an inheriting mapper in a self-referential relationship does + not allow an eager load to be set up.""" + person_join = polymorphic_union( { + 'engineer':people.join(engineers), + 'manager':people.join(managers), + 'person':people.select(people.c.type=='person'), + }, None, 'pjoin') + + person_mapper = mapper(Person, people, select_table=person_join, polymorphic_on=person_join.c.type, + polymorphic_identity='person', + properties = dict(managers = relation(Manager, lazy=False)) + ) + + mapper(Engineer, engineers, inherits=person_mapper, polymorphic_identity='engineer') + mapper(Manager, managers, inherits=person_mapper, polymorphic_identity='manager') + + try: + class_mapper(Manager).compile() + assert False + except exceptions.ArgumentError: + assert True def do_test(self, include_base=False, lazy_relation=True, redefine_colprop=False): """tests the polymorph.py example, with several options: -- 2.47.2