- 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 dffc5d3a562597d0d3858a09282051ad6ccb5ed9..4c1c8b8d9dd372171bc3b92f55a5b8023bc66c78 100644 (file)
--- 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 b2bffd6ea523cf64135e2dacd6393cedf7fd782b..5e20cf6b641b2688adceec18ce158ce194124261 100644 (file)
--- 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 0c62737cbefb5b9f49a0b4def9309e149187b973..0d04b768cb455379c67acf32524d782e19627a7c 100644 (file)
--- 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 09b51c203f2bad6e5bececca26bd5c126e8d4d3a..5725b5f8eb4c44e906b9e39250fde43332016951 100644 (file)
--- 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 6cea2a4bc17c449f05a785b492a868cc97ccd5b2..0440d11a39be8db9061b4a066f577ea69a82dc6f 100644 (file)
--- 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()