- adjustment to the previous checkin regarding inheritance to not conflict with globals

- fix to self-referential eager loading such that if the same mapped
instance appears in two or more distinct sets of columns in the same
result set, its eagerly loaded collection will be populated regardless
of whether or not all of the rows contain a set of "eager" columns for
that collection.  this would also show up as a KeyError when fetching
results with join_depth turned on.

diff --git a/CHANGES b/CHANGES
index 94368253cd6a7fee4ec9b80ec0474b1d43d9f33b..dded89564d3decc2938d056e7213fc4d52d6ebf3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -32,6 +32,13 @@ CHANGES
    - fixed endless loop issue when using lazy="dynamic" on both 
      sides of a bi-directional relationship [ticket:872]
 
+   - fix to self-referential eager loading such that if the same mapped
+     instance appears in two or more distinct sets of columns in the same
+     result set, its eagerly loaded collection will be populated regardless
+     of whether or not all of the rows contain a set of "eager" columns for
+     that collection.  this would also show up as a KeyError when fetching
+     results with join_depth turned on.
+     
    - fixed bug where Query would not apply a subquery to the SQL when LIMIT
      was used in conjunction with an inheriting mapper where the eager 
      loader was only in the parent mapper.

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index e6dc0a91190b7fc3574e01fa0f0e09b0b8684f09..054ac04e740faf3bf1a8189ba997bd87eea71c84 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -46,7 +46,7 @@ class Query(object):
         self._populate_existing = False
         self._version_check = False
         self._autoflush = True
-        self._eager_loaders = util.Set(chain(*[mapper._eager_loaders for mapper in [m for m in self.mapper.iterate_to_root()]]))
+        self._eager_loaders = util.Set(chain(*[mp._eager_loaders for mp in [m for m in self.mapper.iterate_to_root()]]))
         self._attributes = {}
         self._current_path = ()
         self._primary_adapter=None

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index a5f65006df792e327736cea76d25b375a27d6d0d..096a42bb71dffd5ba844137024127d061221dfca 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -591,7 +591,12 @@ class EagerLoader(AbstractRelationLoader):
                         # so that we further descend into properties
                         self.select_mapper._instance(selectcontext, decorated_row, None)
                 else:
-                    if isnew:
+                    appender_key = ('appender', id(instance), self.key)
+                    if isnew or appender_key not in selectcontext.attributes:
+                        # appender_key can be absent from selectcontext.attributes with isnew=False
+                        # when self-referential eager loading is used; the same instance may be present
+                        # in two distinct sets of result columns
+                        
                         if self._should_log_debug:
                             self.logger.debug("initialize UniqueAppender on %s" % mapperutil.attribute_str(instance, self.key))
 
@@ -599,13 +604,13 @@ class EagerLoader(AbstractRelationLoader):
                         appender = util.UniqueAppender(collection, 'append_without_event')
 
                         # store it in the "scratch" area, which is local to this load operation.
-                        selectcontext.attributes[('appender', id(instance), self.key)] = appender
-                    result_list = selectcontext.attributes[('appender', id(instance), self.key)]
+                        selectcontext.attributes[appender_key] = appender
+                    
+                    result_list = selectcontext.attributes[appender_key]
                     if self._should_log_debug:
                         self.logger.debug("eagerload list instance on %s" % mapperutil.attribute_str(instance, self.key))
-                        
-                    self.select_mapper._instance(selectcontext, decorated_row, result_list)
 
+                    self.select_mapper._instance(selectcontext, decorated_row, result_list)
 
             if self._should_log_debug:
                 self.logger.debug("Returning eager instance loader for %s" % str(self))

diff --git a/test/orm/eager_relations.py b/test/orm/eager_relations.py
index 076f44005abaa1148cdcb185f9187ff8611a382b..acb5e0da736013d649395aafac4b8eb2b2717e88 100644 (file)
--- a/test/orm/eager_relations.py
+++ b/test/orm/eager_relations.py
@@ -582,6 +582,48 @@ class SelfReferentialEagerTest(ORMTest):
             ]) == d
         self.assert_sql_count(testbase.db, go, 1)
 
+    
+    def test_lazy_fallback_doesnt_affect_eager(self):
+        class Node(Base):
+            def append(self, node):
+                self.children.append(node)
+
+        mapper(Node, nodes, properties={
+            'children':relation(Node, lazy=False, join_depth=1)
+        })
+        sess = create_session()
+        n1 = Node(data='n1')
+        n1.append(Node(data='n11'))
+        n1.append(Node(data='n12'))
+        n1.append(Node(data='n13'))
+        n1.children[1].append(Node(data='n121'))
+        n1.children[1].append(Node(data='n122'))
+        n1.children[1].append(Node(data='n123'))
+        sess.save(n1)
+        sess.flush()
+        sess.clear()
+
+        # eager load with join depth 1.  when eager load of 'n1'
+        # hits the children of 'n12', no columns are present, eager loader
+        # degrades to lazy loader; fine.  but then, 'n12' is *also* in the
+        # first level of columns since we're loading the whole table.
+        # when those rows arrive, now we *can* eager load its children and an
+        # eager collection should be initialized.  essentially the 'n12' instance
+        # is present in not just two different rows but two distinct sets of columns
+        # in this result set.
+        def go():
+            allnodes = sess.query(Node).order_by(Node.data).all()
+            n12 = allnodes[2]
+            assert n12.data == 'n12'
+            print "N12 IS", id(n12)
+            print [c.data for c in n12.children]
+            assert [
+                Node(data='n121'),
+                Node(data='n122'),
+                Node(data='n123')
+            ] == list(n12.children)
+        self.assert_sql_count(testbase.db, go, 1)
+        
     def test_with_deferred(self):
         class Node(Base):
             def append(self, node):