- eager relation loading bug fixed for eager relation on multiple

  descendant classes [ticket:486]

diff --git a/CHANGES b/CHANGES
index b95605a3aa193d7ae15ea424d1195e235a113d00..955b78088332c9115c081a78bfc2f1fa5505974e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -27,6 +27,8 @@
   all loading-related methods get called [ticket:454]
   - eager relation to an inheriting mapper wont fail if no rows returned for
   the relationship.
+  - eager relation loading bug fixed for eager relation on multiple
+  descendant classes [ticket:486]
   - fix for very large topological sorts, courtesy ants.aasma at gmail [ticket:423]
   - eager loading is slightly more strict about detecting "self-referential"
   relationships, specifically between polymorphic mappers.

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 0ebe397adcb7bfe5640b895263df6bf8406749a6..ef96b039762f2d314557564c9a9c288695e4458c 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -445,6 +445,8 @@ class EagerLoader(AbstractRelationLoader):
         except KeyError:
             clauses = EagerLoader.AliasedClauses(self, parentclauses)
             self.clauses[parentclauses] = clauses
+            
+        if context.mapper not in self.clauses_by_lead_mapper:
             self.clauses_by_lead_mapper[context.mapper] = clauses
 
         if self.secondaryjoin is not None:
@@ -481,7 +483,7 @@ class EagerLoader(AbstractRelationLoader):
                 # decorate the row according to the stored AliasedClauses for this eager load
                 clauses = self.clauses_by_lead_mapper[selectcontext.mapper]
                 decorator = clauses._row_decorator
-            except KeyError:
+            except KeyError, k:
                 # no stored AliasedClauses: eager loading was not set up in the query and
                 # AliasedClauses never got initialized
                 return None
@@ -492,8 +494,10 @@ class EagerLoader(AbstractRelationLoader):
             identity_key = self.mapper.identity_key_from_row(decorated_row)
             # and its good
             return decorator
-        except KeyError:
+        except KeyError, k:
             # no identity key - dont return a row processor, will cause a degrade to lazy
+            if self._should_log_debug:
+                self.logger.debug("could not locate identity key from row '%s'; missing column '%s'" % (repr(decorated_row), str(k)))
             return None
 
     def process_row(self, selectcontext, instance, row, identitykey, isnew):

diff --git a/test/orm/eagertest3.py b/test/orm/eagertest3.py
index 1db0298477ff568898f4734080f64c8a5f42032a..e1592889520a366af46a37d0e08a37b58e9d7ec7 100644 (file)
--- a/test/orm/eagertest3.py
+++ b/test/orm/eagertest3.py
@@ -325,7 +325,97 @@ class EagerTest4(testbase.ORMTest):
         assert d.count() == 2
         assert d[0] is d2
 
-
+class EagerTest5(testbase.ORMTest):
+    """test the construction of AliasedClauses for the same eager load property but different 
+    parent mappers, due to inheritance"""
+    def define_tables(self, metadata):
+        global base, derived, derivedII, comments
+        base = Table(
+            'base', metadata,
+            Column('uid', String, primary_key=True), 
+            Column('x', String)
+            )
+
+        derived = Table(
+            'derived', metadata,
+            Column('uid', String, ForeignKey(base.c.uid), primary_key=True),
+            Column('y', String)
+            )
+
+        derivedII = Table(
+            'derivedII', metadata,
+            Column('uid', String, ForeignKey(base.c.uid), primary_key=True),
+            Column('z', String)
+            )
+
+        comments = Table(
+            'comments', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('uid', String, ForeignKey(base.c.uid)),
+            Column('comment', String)
+            )
+    def test_basic(self):
+        class Base(object):
+            def __init__(self, uid, x):
+                self.uid = uid
+                self.x = x
+
+        class Derived(Base):
+            def __init__(self, uid, x, y):
+                self.uid = uid
+                self.x = x
+                self.y = y
+
+        class DerivedII(Base):
+            def __init__(self, uid, x, z):
+                self.uid = uid
+                self.x = x
+                self.z = z
+
+        class Comment(object):
+            def __init__(self, uid, comment):
+                self.uid = uid
+                self.comment = comment
+
+
+        commentMapper = mapper(Comment, comments)
+
+        baseMapper = mapper(
+            Base, base,
+                properties={
+                'comments': relation(
+                    Comment, lazy=False, cascade='all, delete-orphan'
+                    )
+                }
+            )
+
+        derivedMapper = mapper(Derived, derived, inherits=baseMapper)
+        derivedIIMapper = mapper(DerivedII, derivedII, inherits=baseMapper)
+        sess = create_session()
+        d = Derived(1, 'x', 'y')
+        d.comments = [Comment(1, 'comment')]
+        d2 = DerivedII(2, 'xx', 'z')
+        d2.comments = [Comment(2, 'comment')]
+        sess.save(d)
+        sess.save(d2)
+        sess.flush()
+        sess.clear()
+        # this eager load sets up an AliasedClauses for the "comment" relationship,
+        # then stores it in clauses_by_lead_mapper[mapper for Derived]
+        d = sess.query(Derived).get(1)
+        sess.clear()
+        assert len([c for c in d.comments]) == 1
+
+        # this eager load sets up an AliasedClauses for the "comment" relationship,
+        # and should store it in clauses_by_lead_mapper[mapper for DerivedII].
+        # the bug was that the previous AliasedClause create prevented this population
+        # from occurring.
+        d2 = sess.query(DerivedII).get(2)
+        sess.clear()
+        # object is not in the session; therefore the lazy load cant trigger here,
+        # eager load had to succeed
+        assert len([c for c in d2.comments]) == 1
+        
     
 if __name__ == "__main__":    
     testbase.main()