diff options
| -rw-r--r-- | CHANGES | 11 | ||||
| -rw-r--r-- | VERSION | 2 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/base.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/__init__.py | 9 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/attributes.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 163 | ||||
| -rw-r--r-- | lib/sqlalchemy/topological.py | 29 | ||||
| -rw-r--r-- | lib/sqlalchemy/util.py | 9 | ||||
| -rw-r--r-- | test/orm/inheritance/basic.py | 12 | ||||
| -rw-r--r-- | test/orm/inheritance/polymorph.py | 2 | ||||
| -rw-r--r-- | test/orm/inheritance/polymorph2.py | 3 | ||||
| -rw-r--r-- | test/orm/inheritance/productspec.py | 16 | ||||
| -rw-r--r-- | test/orm/mapper.py | 33 | ||||
| -rw-r--r-- | test/orm/unitofwork.py | 2 |
14 files changed, 143 insertions, 150 deletions
@@ -1,6 +1,17 @@ ======= CHANGES ======= +0.4.0beta6 +---------- + +- mapper compilation has been reorganized such that most compilation + occurs upon mapper construction. this allows us to have fewer + calls to mapper.compile() and also to allow class-based properties + to force a compilation (i.e. User.addresses == 7 will compile all + mappers; this is [ticket:758]). The only caveat here is that + an inheriting mapper now looks for its inherited mapper upon construction; + so mappers within inheritance relationships need to be constructed in + inheritance order (which should be the normal case anyway). 0.4.0beta5 ---------- @@ -1 +1 @@ -0.4.0beta5 +0.4.0beta6 diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 45d84f90d..bd6f5b97c 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1663,7 +1663,6 @@ class DefaultRunner(schema.SchemaVisitor): def visit_sequence(self, seq): """Do nothing. - Sequences are not supported by default. """ return None diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 40806bdb0..0d4dec4bc 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -529,18 +529,15 @@ def compile_mappers(): def clear_mappers(): """Remove all mappers that have been created thus far. - When new mappers are created, they will be assigned to their - classes as their primary mapper. + The mapped classes will return to their initial "unmapped" + state and can be re-mapped with new mappers. """ - mapperlib._COMPILE_MUTEX.acquire() try: for mapper in mapper_registry.values(): mapper.dispose() mapper_registry.clear() - # TODO: either dont use ArgSingleton, or - # find a way to clear only ClassKey instances from it - sautil.ArgSingleton.instances.clear() + mapperlib.ClassKey.dispose() finally: mapperlib._COMPILE_MUTEX.release() diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 74e9eb333..f369c5396 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -725,7 +725,6 @@ class AttributeManager(object): callable, the callable will only be executed if the given `passive` flag is False. """ - attr = getattr(obj.__class__, key) x = attr.get(obj, passive=passive) if x is PASSIVE_NORESULT: diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 9ae83460d..5f60d26e0 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -11,7 +11,7 @@ from sqlalchemy.sql import util as sqlutil from sqlalchemy.orm import util as mapperutil from sqlalchemy.orm.util import ExtensionCarrier from sqlalchemy.orm import sync -from sqlalchemy.orm.interfaces import MapperProperty, EXT_CONTINUE, SynonymProperty +from sqlalchemy.orm.interfaces import MapperProperty, EXT_CONTINUE, SynonymProperty, PropComparator deferred_load = None __all__ = ['Mapper', 'class_mapper', 'object_mapper', 'mapper_registry'] @@ -141,23 +141,27 @@ class Mapper(object): # was sent to this mapper. self.__surrogate_mapper = None - # whether or not our compile() method has been called already. - self.__is_compiled = False + self.__props_init = False - # if this mapper is to be a primary mapper (i.e. the non_primary flag is not set), - # associate this Mapper with the given class_ and entity name. subsequent - # calls to class_mapper() for the class_/entity name combination will return this - # mapper. + self.__should_log_info = logging.is_info_enabled(self.logger) + self.__should_log_debug = logging.is_debug_enabled(self.logger) + self._compile_class() + self._compile_extensions() + self._compile_inheritance() + self._compile_tables() + self._compile_properties() + self._compile_selectable() - self.__should_log_debug = logging.is_debug_enabled(self.logger) self.__log("constructed") def __log(self, msg): - self.logger.info("(" + self.class_.__name__ + "|" + (self.entity_name is not None and "/%s" % self.entity_name or "") + (self.local_table and self.local_table.name or str(self.local_table)) + (not self._is_primary_mapper() and "|non-primary" or "") + ") " + msg) + if self.__should_log_info: + self.logger.info("(" + self.class_.__name__ + "|" + (self.entity_name is not None and "/%s" % self.entity_name or "") + (self.local_table and self.local_table.name or str(self.local_table)) + (not self._is_primary_mapper() and "|non-primary" or "") + ") " + msg) def __log_debug(self, msg): - self.logger.debug("(" + self.class_.__name__ + "|" + (self.entity_name is not None and "/%s" % self.entity_name or "") + (self.local_table and self.local_table.name or str(self.local_table)) + (not self._is_primary_mapper() and "|non-primary" or "") + ") " + msg) + if self.__should_log_debug: + self.logger.debug("(" + self.class_.__name__ + "|" + (self.entity_name is not None and "/%s" % self.entity_name or "") + (self.local_table and self.local_table.name or str(self.local_table)) + (not self._is_primary_mapper() and "|non-primary" or "") + ") " + msg) def _is_orphan(self, obj): optimistic = has_identity(obj) @@ -179,7 +183,6 @@ class Mapper(object): def get_property(self, key, resolve_synonyms=False, raiseerr=True): """return MapperProperty with the given key.""" - self.compile() prop = self.__props.get(key, None) if resolve_synonyms: while isinstance(prop, SynonymProperty): @@ -189,11 +192,12 @@ class Mapper(object): return prop def iterate_properties(self): - self.compile() return self.__props.itervalues() iterate_properties = property(iterate_properties, doc="returns an iterator of all MapperProperty objects.") def dispose(self): + # disaable any attribute-based compilation + self.__props_init = True attribute_manager.reset_class_managed(self.class_) if hasattr(self.class_, 'c'): del self.class_.c @@ -205,65 +209,50 @@ class Mapper(object): def compile(self): """Compile this mapper into its final internal format. - - This is the *external* version of the method which is not - reentrant. """ - if self.__is_compiled: + if self.__props_init: return self _COMPILE_MUTEX.acquire() try: # double-check inside mutex - if self.__is_compiled: + if self.__props_init: return self - self._compile_all() + # 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() + self.__initialize_properties() return self finally: _COMPILE_MUTEX.release() - def _compile_all(self): - # compile all primary mappers - for mapper in mapper_registry.values(): - if not mapper.__is_compiled: - mapper._do_compile() - - # initialize properties on all mappers - for mapper in mapper_registry.values(): - if not mapper.__props_init: - mapper._initialize_properties() - def _check_compile(self): - if self.non_primary: - self._do_compile() - self._initialize_properties() + if self.non_primary and not self.__props_init: + self.__initialize_properties() return self + + def __initialize_properties(self): + """Call the ``init()`` method on all ``MapperProperties`` + attached to this mapper. - def _do_compile(self): - """Compile this mapper into its final internal format. - - This is the *internal* version of the method which is assumed - to be called within compile() and is reentrant. + This happens after all mappers have completed compiling + everything else up until this point, so that all dependencies + are fully available. """ - if self.__is_compiled: - return self - self.__log("_do_compile() started") - self.__is_compiled = True - self.__props_init = False - self._compile_extensions() - self._compile_inheritance() - self._compile_tables() - self._compile_properties() - self._compile_selectable() - self.__log("_do_compile() complete") - return self + self.__log("_initialize_properties() started") + l = [(key, prop) for key, prop in self.__props.iteritems()] + for key, prop in l: + if getattr(prop, 'key', None) is None: + prop.init(key, self) + self.__log("_initialize_properties() complete") + self.__props_init = True + def _compile_extensions(self): """Go through the global_extensions list as well as the list @@ -301,9 +290,9 @@ class Mapper(object): if self.inherits is not None: if isinstance(self.inherits, type): - self.inherits = class_mapper(self.inherits, compile=False)._do_compile() + self.inherits = class_mapper(self.inherits, compile=False) else: - self.inherits = self.inherits._do_compile() + self.inherits = self.inherits if not issubclass(self.class_, self.inherits.class_): raise exceptions.ArgumentError("Class '%s' does not inherit from '%s'" % (self.class_.__name__, self.inherits.class_.__name__)) if self._is_primary_mapper() != self.inherits._is_primary_mapper(): @@ -537,15 +526,34 @@ class Mapper(object): result.setdefault(fk.column, util.Set()).add(col) return result - + + class _CompileOnAttr(PropComparator): + """placeholder class attribute which fires mapper compilation on access""" + def __init__(self, class_, key): + self.class_ = class_ + self.key = key + def __getattribute__(self, key): + cls = object.__getattribute__(self, 'class_') + clskey = object.__getattribute__(self, 'key') + + # get the class' mapper; will compile all mappers + class_mapper(cls) + + if cls.__dict__.get(clskey) is self: + # FIXME: there should not be any scenarios where + # a mapper compile leaves this CompileOnAttr in + # place. + warnings.warn(RuntimeWarning("Attribute '%s' on class '%s' was not replaced during mapper compilation operation" % (clskey, cls.__name__))) + # clean us up explicitly + delattr(cls, clskey) + + return getattr(getattr(cls, clskey), key) + def _compile_properties(self): """Inspect the properties dictionary sent to the Mapper's constructor as well as the mapped_table, and create ``MapperProperty`` objects corresponding to each mapped column and relation. - - Also grab ``MapperProperties`` from the inherited mapper, if - any, and create copies of them to attach to this Mapper. """ # object attribute names mapped to MapperProperty objects @@ -575,7 +583,7 @@ class Mapper(object): self.columns[column.key] = self.select_table.corresponding_column(column, keys_ok=True, raiseerr=True) column_key = (self.column_prefix or '') + column.key - prop = self.__props.get(column.key, None) + prop = self.__props.get(column_key, None) if prop is None: if (self.include_properties is not None and @@ -590,6 +598,11 @@ class Mapper(object): prop = ColumnProperty(column) self.__props[column_key] = prop + + # TODO: centralize _CompileOnAttr logic, move into MapperProperty classes + if not hasattr(self.class_, column_key) and not self.non_primary: + setattr(self.class_, column_key, Mapper._CompileOnAttr(self.class_, column_key)) + prop.set_parent(self) self.__log("adding ColumnProperty %s" % (column_key)) elif isinstance(prop, ColumnProperty): @@ -612,23 +625,6 @@ class Mapper(object): self.columntoproperty.setdefault(column, []).append(prop) - def _initialize_properties(self): - """Call the ``init()`` method on all ``MapperProperties`` - attached to this mapper. - - This happens after all mappers have completed compiling - everything else up until this point, so that all dependencies - are fully available. - """ - - self.__log("_initialize_properties() started") - l = [(key, prop) for key, prop in self.__props.iteritems()] - for key, prop in l: - if getattr(prop, 'key', None) is None: - prop.init(key, self) - self.__log("_initialize_properties() complete") - 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 @@ -762,10 +758,7 @@ class Mapper(object): """ self.properties[key] = prop - if self.__is_compiled: - # if we're compiled, make sure all the other mappers are compiled too - self._compile_all() - self._compile_property(key, prop, init=True) + self._compile_property(key, prop, init=self.__props_init) def _create_prop_from_column(self, column): column = util.to_list(column) @@ -805,9 +798,15 @@ class Mapper(object): prop = col self.__props[key] = prop + + if setparent: prop.set_parent(self) + # TODO: centralize _CompileOnAttr logic, move into MapperProperty classes + if (not isinstance(prop, SynonymProperty) or prop.proxy) and not self.non_primary and not hasattr(self.class_, key): + setattr(self.class_, key, Mapper._CompileOnAttr(self.class_, key)) + if isinstance(prop, ColumnProperty): # relate the mapper's "select table" to the given ColumnProperty col = self.select_table.corresponding_column(prop.columns[0], keys_ok=True, raiseerr=False) @@ -831,6 +830,7 @@ class Mapper(object): def _is_primary_mapper(self): """Return True if this mapper is the primary mapper for its class key (class + entity_name).""" + # FIXME: cant we just look at "non_primary" flag ? return mapper_registry.get(self.class_key, None) is self def primary_mapper(self): @@ -864,7 +864,6 @@ class Mapper(object): from the extension chain. """ - self.compile() s = self.extension.get_session() if s is EXT_CONTINUE: raise exceptions.InvalidRequestError("No contextual Session is established. Use a MapperExtension that implements get_session or use 'import sqlalchemy.mods.threadlocal' to establish a default thread-local contextual session.") @@ -1581,9 +1580,7 @@ class ClassKey(object): def __repr__(self): return "ClassKey(%s, %s)" % (repr(self.class_), repr(self.entity_name)) - def dispose(self): - type(self).dispose_static(self.class_, self.entity_name) - + def has_identity(object): return hasattr(object, '_instance_key') diff --git a/lib/sqlalchemy/topological.py b/lib/sqlalchemy/topological.py index a13923885..d38c5cf4a 100644 --- a/lib/sqlalchemy/topological.py +++ b/lib/sqlalchemy/topological.py @@ -6,18 +6,6 @@ """Topological sorting algorithms. -The key to the unit of work is to assemble a list of dependencies -amongst all the different mappers that have been defined for classes. - -Related tables with foreign key constraints have a definite insert -order, deletion order, objects need dependent properties from parent -objects set up before saved, etc. - -These are all encoded as dependencies, in the form *mapper X is -dependent on mapper Y*, meaning mapper Y's objects must be saved -before those of mapper X, and mapper X's objects must be deleted -before those of mapper Y. - The topological sort is an algorithm that receives this list of dependencies as a *partial ordering*, that is a list of pairs which might say, *X is dependent on Y*, *Q is dependent on Z*, but does not @@ -28,18 +16,6 @@ then only towards just some of the other elements. For a particular partial ordering, there can be many possible sorts that satisfy the conditions. -An intrinsic *gotcha* to this algorithm is that since there are many -possible outcomes to sorting a partial ordering, the algorithm can -return any number of different results for the same input; just -running it on a different machine architecture, or just random -differences in the ordering of dictionaries, can change the result -that is returned. While this result is guaranteed to be true to the -incoming partial ordering, if the partial ordering itself does not -properly represent the dependencies, code that works fine will -suddenly break, then work again, then break, etc. Most of the bugs -I've chased down while developing the *unit of work* have been of this -nature - very tricky to reproduce and track down, particularly before -I realized this characteristic of the algorithm. """ from sqlalchemy import util @@ -158,8 +134,9 @@ class QueueDependencySorter(object): """Topological sort adapted from wikipedia's article on the subject. It creates a straight-line list of elements, then a second pass - groups non-dependent actions together to build more of a tree - structure with siblings. + batches non-dependent elements as siblings in a tree structure. Future + versions of this algorithm may separate the "convert to a tree" + step. """ def __init__(self, tuples, allitems): diff --git a/lib/sqlalchemy/util.py b/lib/sqlalchemy/util.py index 321540419..633e6b0c1 100644 --- a/lib/sqlalchemy/util.py +++ b/lib/sqlalchemy/util.py @@ -122,10 +122,11 @@ def hash(string): class ArgSingleton(type): instances = {} - def dispose_static(self, *args): - hashkey = (self, args) - #if hashkey in ArgSingleton.instances: - del ArgSingleton.instances[hashkey] + def dispose(cls): + for key in ArgSingleton.instances: + if key[0] is cls: + del ArgSingleton.instances[key] + dispose = classmethod(dispose) def __call__(self, *args): hashkey = (self, args) diff --git a/test/orm/inheritance/basic.py b/test/orm/inheritance/basic.py index 870f3e869..fbdb4019e 100644 --- a/test/orm/inheritance/basic.py +++ b/test/orm/inheritance/basic.py @@ -170,14 +170,10 @@ class ConstructionTest(ORMTest): class Product(Content): pass content_types = mapper(ContentType, content_type) - contents = mapper(Content, content, properties={ - 'content_type':relation(content_types) - }, polymorphic_identity='contents') - - products = mapper(Product, product, inherits=contents, polymorphic_identity='products') - try: - compile_mappers() + contents = mapper(Content, content, properties={ + 'content_type':relation(content_types) + }, polymorphic_identity='contents') assert False except exceptions.ArgumentError, e: assert str(e) == "Mapper 'Mapper|Content|content' specifies a polymorphic_identity of 'contents', but no mapper in it's hierarchy specifies the 'polymorphic_on' column argument" @@ -377,8 +373,8 @@ class DistinctPKTest(ORMTest): def test_explicit_composite_pk(self): person_mapper = mapper(Person, person_table) - mapper(Employee, employee_table, inherits=person_mapper, primary_key=[person_table.c.id, employee_table.c.id]) try: + mapper(Employee, employee_table, inherits=person_mapper, primary_key=[person_table.c.id, employee_table.c.id]) self._do_test(True) assert False except RuntimeWarning, e: diff --git a/test/orm/inheritance/polymorph.py b/test/orm/inheritance/polymorph.py index caee34b09..f067b330b 100644 --- a/test/orm/inheritance/polymorph.py +++ b/test/orm/inheritance/polymorph.py @@ -90,7 +90,7 @@ class CompileTest(PolymorphTest): session.flush() print session.query(Engineer).select() - print session.query(Person).select() + print session.query(Person).select() def testcompile2(self): """test that a mapper can reference a property whose mapper inherits from this one.""" diff --git a/test/orm/inheritance/polymorph2.py b/test/orm/inheritance/polymorph2.py index 6e03c97e9..a58800c56 100644 --- a/test/orm/inheritance/polymorph2.py +++ b/test/orm/inheritance/polymorph2.py @@ -237,7 +237,6 @@ def generate_test(jointype="join1", usedata=False): if usedata: mapper(Data, data) - mapper(Manager, managers, inherits=Person, inherit_condition=people.c.person_id==managers.c.person_id, polymorphic_identity='manager') if usedata: mapper(Person, people, select_table=poly_union, polymorphic_identity='person', polymorphic_on=people.c.type, properties={ @@ -253,6 +252,8 @@ def generate_test(jointype="join1", usedata=False): } ) + mapper(Manager, managers, inherits=Person, inherit_condition=people.c.person_id==managers.c.person_id, polymorphic_identity='manager') + sess = create_session() p = Person(name='person1') p2 = Person(name='person2') diff --git a/test/orm/inheritance/productspec.py b/test/orm/inheritance/productspec.py index 2459cd36e..5c8c64bed 100644 --- a/test/orm/inheritance/productspec.py +++ b/test/orm/inheritance/productspec.py @@ -272,12 +272,6 @@ class InheritTest(ORMTest): ) ) - detail_mapper = mapper(Detail, inherits=Product, - polymorphic_identity='detail') - - raster_document_mapper = mapper(RasterDocument, inherits=Document, - polymorphic_identity='raster_document') - product_mapper = mapper(Product, products_table, polymorphic_on=products_table.c.product_type, polymorphic_identity='product', properties={ @@ -285,8 +279,8 @@ class InheritTest(ORMTest): backref='product', cascade='all, delete-orphan'), }) - assembly_mapper = mapper(Assembly, inherits=Product, - polymorphic_identity='assembly') + detail_mapper = mapper(Detail, inherits=Product, + polymorphic_identity='detail') document_mapper = mapper(Document, documents_table, polymorphic_on=documents_table.c.document_type, @@ -297,6 +291,12 @@ class InheritTest(ORMTest): ), ) + raster_document_mapper = mapper(RasterDocument, inherits=Document, + polymorphic_identity='raster_document') + + assembly_mapper = mapper(Assembly, inherits=Product, + polymorphic_identity='assembly') + session = create_session() a1 = Assembly(name='a1') diff --git a/test/orm/mapper.py b/test/orm/mapper.py index 905ad5bfe..f00c1c59d 100644 --- a/test/orm/mapper.py +++ b/test/orm/mapper.py @@ -212,14 +212,7 @@ class MapperTest(MapperSuperTest): m1 = mapper(Address, addresses) m2 = mapper(User, users, properties = dict(addresses=relation(Address,private=True,lazy=False)) ) - assert m1._Mapper__is_compiled is False - assert m2._Mapper__is_compiled is False - -# compile_mappers() - print "NEW USER" u=User() - print "NEW USER DONE" - assert m2._Mapper__is_compiled is True u.user_name='Justin' a = Address() a.address_id=17 # to work around the hardcoded IDs in this test suite.... @@ -236,12 +229,30 @@ class MapperTest(MapperSuperTest): s.refresh(u) #hangs def testprops(self): - """tests the various attributes of the properties attached to classes""" m = mapper(User, users, properties = { 'addresses' : relation(mapper(Address, addresses)) }).compile() self.assert_(User.addresses.property is m.get_property('addresses')) + + def testcompileonprop(self): + mapper(User, users, properties = { + 'addresses' : relation(mapper(Address, addresses)) + }) + User.addresses.any(Address.email_address=='foo@bar.com') + clear_mappers() + mapper(User, users, properties = { + 'addresses' : relation(mapper(Address, addresses)) + }) + assert (User.user_id==3).compare(users.c.user_id==3) + + clear_mappers() + + class Foo(User):pass + mapper(User, users) + mapper(Foo, addresses, inherits=User) + assert getattr(Foo().__class__, 'user_name').get is not None + def testpropfilters(self): t = Table('person', MetaData(), Column('id', Integer, primary_key=True), @@ -276,12 +287,13 @@ class MapperTest(MapperSuperTest): column_prefix="p_") p_m.compile() + #compile_mappers() def assert_props(cls, want): have = set([n for n in dir(cls) if not n.startswith('_')]) want = set(want) want.add('c') - self.assert_(have == want) + self.assert_(have == want, repr(have) + " " + repr(want)) assert_props(Person, ['id', 'name', 'type']) assert_props(Employee, ['boss', 'boss_id', 'employee_number', @@ -440,6 +452,9 @@ class MapperTest(MapperSuperTest): adname = synonym('addresses') )) + assert hasattr(User, 'adlist') + assert not hasattr(User, 'adname') + u = sess.query(User).get_by(uname='jack') self.assert_result(u.adlist, Address, *(user_address_result[0]['addresses'][1])) diff --git a/test/orm/unitofwork.py b/test/orm/unitofwork.py index 6dd4a08e8..63456b638 100644 --- a/test/orm/unitofwork.py +++ b/test/orm/unitofwork.py @@ -464,7 +464,7 @@ class ForeignPKTest(ORMTest): 'sites' : relation(PersonSite), }, ) - + compile_mappers() assert list(m2.get_property('sites').foreign_keys) == [peoplesites.c.person] p = Person() p.person = 'im the key' |