Add "existing" populators for subqueryload

Fixed bug in subquery loading where an object encountered as an
"existing" row, e.g. already loaded from a different path in the
same query, would not invoke subquery loaders for unloaded attributes
that specified this loading.  This issue is in the same area
as that of :ticket:`3431`, :ticket:`3811` which involved
similar issues with joined loading.

Change-Id: If111a76b0812010905b0ac811276a816779d297f
Fixes: #3854

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 484945ebd4dc1ec36f3a1769a390f89911041b37..fc4877e6c9427afb49058776398b04e46b254b7d 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -21,6 +21,17 @@
 .. changelog::
     :version: 1.1.5
 
+    .. change:: 3854
+        :tags: bug, orm
+        :tickets: 3854
+
+        Fixed bug in subquery loading where an object encountered as an
+        "existing" row, e.g. already loaded from a different path in the
+        same query, would not invoke subquery loaders for unloaded attributes
+        that specified this loading.  This issue is in the same area
+        as that of :ticket:`3431`, :ticket:`3811` which involved
+        similar issues with joined loading.
+
     .. change:: 3888
         :tags: bug, postgresql
         :tickets: 3888

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index ace82871f16aa1aaf0ad1096ea4e4cc531a65c05..4e3574b5406a9c23e7b8eb8531d7b08b8df1e471 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1087,7 +1087,15 @@ class SubqueryLoader(AbstractRelationshipLoader):
             state.get_impl(self.key).\
                 set_committed_value(state, dict_, collection)
 
-        populators["new"].append((self.key, load_collection_from_subq))
+        def load_collection_from_subq_existing_row(state, dict_, row):
+            if self.key not in dict_:
+                load_collection_from_subq(state, dict_, row)
+
+        populators["new"].append(
+            (self.key, load_collection_from_subq))
+        populators["existing"].append(
+            (self.key, load_collection_from_subq_existing_row))
+
         if context.invoke_all_eagers:
             populators["eager"].append((self.key, collections.loader))
 
@@ -1108,7 +1116,14 @@ class SubqueryLoader(AbstractRelationshipLoader):
             state.get_impl(self.key).\
                 set_committed_value(state, dict_, scalar)
 
-        populators["new"].append((self.key, load_scalar_from_subq))
+        def load_scalar_from_subq_existing_row(state, dict_, row):
+            if self.key not in dict_:
+                load_scalar_from_subq(state, dict_, row)
+
+        populators["new"].append(
+            (self.key, load_scalar_from_subq))
+        populators["existing"].append(
+            (self.key, load_scalar_from_subq_existing_row))
         if context.invoke_all_eagers:
             populators["eager"].append((self.key, collections.loader))
 

diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index c8e8636fe511ef9327d63fe0e874cfe234e4b3cb..ce99260587772a42b03ce30bcc1b6887640ef5dc 100644 (file)
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -1,4 +1,4 @@
-from sqlalchemy.testing import eq_, is_, is_not_
+from sqlalchemy.testing import eq_, is_, is_not_, is_true
 from sqlalchemy import testing
 from sqlalchemy.testing.schema import Table, Column
 from sqlalchemy import Integer, String, ForeignKey, bindparam, inspect
@@ -2072,3 +2072,122 @@ class JoinedNoLoadConflictTest(fixtures.DeclarativeMappedTest):
             [Child(name='c1')]
         )
 
+
+class TestExistingRowPopulation(fixtures.DeclarativeMappedTest):
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class A(Base):
+            __tablename__ = 'a'
+
+            id = Column(Integer, primary_key=True)
+            b_id = Column(ForeignKey('b.id'))
+            a2_id = Column(ForeignKey('a2.id'))
+            a2 = relationship("A2")
+            b = relationship("B")
+
+        class A2(Base):
+            __tablename__ = 'a2'
+
+            id = Column(Integer, primary_key=True)
+            b_id = Column(ForeignKey('b.id'))
+            b = relationship("B")
+
+        class B(Base):
+            __tablename__ = 'b'
+
+            id = Column(Integer, primary_key=True)
+
+            c1_m2o_id = Column(ForeignKey('c1_m2o.id'))
+            c2_m2o_id = Column(ForeignKey('c2_m2o.id'))
+
+            c1_o2m = relationship("C1o2m")
+            c2_o2m = relationship("C2o2m")
+            c1_m2o = relationship("C1m2o")
+            c2_m2o = relationship("C2m2o")
+
+        class C1o2m(Base):
+            __tablename__ = 'c1_o2m'
+
+            id = Column(Integer, primary_key=True)
+            b_id = Column(ForeignKey('b.id'))
+
+        class C2o2m(Base):
+            __tablename__ = 'c2_o2m'
+
+            id = Column(Integer, primary_key=True)
+            b_id = Column(ForeignKey('b.id'))
+
+        class C1m2o(Base):
+            __tablename__ = 'c1_m2o'
+
+            id = Column(Integer, primary_key=True)
+
+        class C2m2o(Base):
+            __tablename__ = 'c2_m2o'
+
+            id = Column(Integer, primary_key=True)
+
+    @classmethod
+    def insert_data(cls):
+        A, A2, B, C1o2m, C2o2m, C1m2o, C2m2o = cls.classes(
+            'A', 'A2', 'B', 'C1o2m', 'C2o2m', 'C1m2o', 'C2m2o'
+        )
+
+        s = Session()
+
+        b = B(
+            c1_o2m=[C1o2m()],
+            c2_o2m=[C2o2m()],
+            c1_m2o=C1m2o(),
+            c2_m2o=C2m2o(),
+        )
+
+        s.add(A(b=b, a2=A2(b=b)))
+        s.commit()
+
+    def test_o2m(self):
+        A, A2, B, C1o2m, C2o2m = self.classes(
+            'A', 'A2', 'B', 'C1o2m', 'C2o2m'
+        )
+
+        s = Session()
+
+        # A -J-> B -L-> C1
+        # A -J-> B -S-> C2
+
+        # A -J-> A2 -J-> B -S-> C1
+        # A -J-> A2 -J-> B -L-> C2
+
+        q = s.query(A).options(
+            joinedload(A.b).subqueryload(B.c2_o2m),
+            joinedload(A.a2).joinedload(A2.b).subqueryload(B.c1_o2m)
+        )
+
+        a1 = q.all()[0]
+
+        is_true('c1_o2m' in a1.b.__dict__)
+        is_true('c2_o2m' in a1.b.__dict__)
+
+    def test_m2o(self):
+        A, A2, B, C1m2o, C2m2o = self.classes(
+            'A', 'A2', 'B', 'C1m2o', 'C2m2o'
+        )
+
+        s = Session()
+
+        # A -J-> B -L-> C1
+        # A -J-> B -S-> C2
+
+        # A -J-> A2 -J-> B -S-> C1
+        # A -J-> A2 -J-> B -L-> C2
+
+        q = s.query(A).options(
+            joinedload(A.b).subqueryload(B.c2_m2o),
+            joinedload(A.a2).joinedload(A2.b).subqueryload(B.c1_m2o)
+        )
+
+        a1 = q.all()[0]
+        is_true('c1_m2o' in a1.b.__dict__)
+        is_true('c2_m2o' in a1.b.__dict__)