From: Mike Bayer Date: Sat, 6 Oct 2007 01:12:19 +0000 (+0000) Subject: - null foreign key on a m2o doesn't trigger a lazyload [ticket:803] X-Git-Tag: rel_0_4_0~70 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f97414b5203d2c05ccaf18942ecd7f42a7c4611f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - null foreign key on a m2o doesn't trigger a lazyload [ticket:803] - slight simpliication to mapper.populate_instance() - lamenting the different codepaths between query._get() and DeferredLoader.lazyload() - query._get() uses all()[0] for single-row load to avoid complexity of first() (same as LazyLoader) --- diff --git a/CHANGES b/CHANGES index dffc5d3a56..4c1c8b8d9d 100644 --- a/CHANGES +++ b/CHANGES @@ -35,6 +35,8 @@ CHANGES - fixed sqlite reflection of BOOL/BOOLEAN [ticket:808] +- null foreign key on a m2o doesn't trigger a lazyload [ticket:803] + 0.4.0beta6 ---------- diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index b2bffd6ea5..5e20cf6b64 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1490,8 +1490,8 @@ class Mapper(object): # "snapshot" of the stack, which represents a path from the lead mapper in the query to this one, # including relation() names. the key also includes "self", and allows us to distinguish between # other mappers within our inheritance hierarchy - populators = selectcontext.attributes.get(((isnew or ispostselect) and 'new_populators' or 'existing_populators', self, snapshot, ispostselect), None) - if populators is None: + (new_populators, existing_populators) = selectcontext.attributes.get(('populators', self, snapshot, ispostselect), (None, None)) + if new_populators is None: # no populators; therefore this is the first time we are receiving a row for # this result set. issue create_row_processor() on all MapperProperty objects # and cache in the select context. @@ -1511,13 +1511,13 @@ class Mapper(object): if poly_select_loader is not None: post_processors.append(poly_select_loader) - selectcontext.attributes[('new_populators', self, snapshot, ispostselect)] = new_populators - selectcontext.attributes[('existing_populators', self, snapshot, ispostselect)] = existing_populators + selectcontext.attributes[('populators', self, snapshot, ispostselect)] = (new_populators, existing_populators) selectcontext.attributes[('post_processors', self, ispostselect)] = post_processors - if isnew or ispostselect: - populators = new_populators - else: - populators = existing_populators + + if isnew or ispostselect: + populators = new_populators + else: + populators = existing_populators for p in populators: p(instance, row, ispostselect=ispostselect, isnew=isnew, **flags) diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 0c62737cbe..0d04b768cb 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -719,7 +719,8 @@ class Query(object): q = q.with_lockmode(lockmode) q = q.filter(self.select_mapper._get_clause) q = q.params(params)._select_context_options(populate_existing=reload, version_check=(lockmode is not None)) - return q.first() + # call using all() to avoid LIMIT compilation complexity + return q.all()[0] except IndexError: return None diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 09b51c203f..5725b5f8eb 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -209,6 +209,8 @@ class DeferredColumnLoader(LoaderStrategy): else: statement, params = create_statement() + # TODO: have the "fetch of one row" operation go through the same channels as a query._get() + # deferred load of several attributes should be a specialized case of a query refresh operation conn = session.connection(mapper=localparent, instance=instance) result = conn.execute(statement, params) try: @@ -348,9 +350,15 @@ class LazyLoader(AbstractRelationLoader): for col, bind in self.lazybinds.iteritems(): params[bind.key] = self.parent.get_attr_by_column(instance, col) ident = [] + nonnulls = False for primary_key in self.select_mapper.primary_key: bind = self.lazyreverse[primary_key] - ident.append(params[bind.key]) + v = params[bind.key] + if v is not None: + nonnulls = True + ident.append(v) + if not nonnulls: + return None if options: q = q.options(*options) return q.get(ident) diff --git a/test/orm/lazy_relations.py b/test/orm/lazy_relations.py index 6cea2a4bc1..0440d11a39 100644 --- a/test/orm/lazy_relations.py +++ b/test/orm/lazy_relations.py @@ -130,6 +130,7 @@ class LazyTest(QueryTest): assert getattr(User, 'addresses').hasparent(user.addresses[0], optimistic=True) assert not class_mapper(Address)._is_orphan(user.addresses[0]) + def test_limit(self): """test limit operations combined with lazy-load relationships.""" @@ -274,5 +275,35 @@ class LazyTest(QueryTest): assert a.user is u1 +class M2OGetTest(QueryTest): + keep_mappers = False + keep_data = False + + def setup_mappers(self): + pass + + def test_m2o_noload(self): + """test that a NULL foreign key doesn't trigger a lazy load""" + mapper(User, users) + + mapper(Address, addresses, properties={ + 'user':relation(User) + }) + + sess = create_session() + ad1 = Address(email_address='somenewaddress', id=12) + sess.save(ad1) + sess.flush() + sess.clear() + + ad2 = sess.query(Address).get(1) + ad3 = sess.query(Address).get(ad1.id) + def go(): + # one lazy load + assert ad2.user.name == 'jack' + # no lazy load + assert ad3.user is None + self.assert_sql_count(testbase.db, go, 1) + if __name__ == '__main__': testbase.main()