From: Mike Bayer Date: Mon, 16 Jan 2017 17:10:08 +0000 (-0500) Subject: Add "existing" populators for subqueryload X-Git-Tag: rel_1_1_5~10^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3aefc6e8a4a02892de91bdac9a917d1697c90ea6;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 484945ebd4..fc4877e6c9 100644 --- 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 ace82871f1..4e3574b540 100644 --- 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 c8e8636fe5..ce99260587 100644 --- 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__)