From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 10 Apr 2017 17:25:50 +0000 (-0400)
Subject: Compare entities also on chop_path
X-Git-Tag: rel_1_2_0b1~106^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=86c43d2e0f0c7c28b933f470678571077c5d5035;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Compare entities also on chop_path

When comparing query._current_path to options, the path chop
was not taking into account that the query or the options are
against aliased classes that don't match the mapper.

The issue does not seem to take place for the Load() version
of _chop_path.

Fixed bug to improve upon the specificity of loader options that
take effect subsequent to the lazy load of a related entity, so
that the loader options will match to an aliased or non-aliased
entity more specifically if those options include entity information.

Fixes: #3963
Change-Id: Ifdff37d579042fcc62bdeabce9e2413e9a03fbba
---

diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst
index 2070b92c03..a64a5d3803 100644
--- a/doc/build/changelog/changelog_12.rst
+++ b/doc/build/changelog/changelog_12.rst
@@ -13,6 +13,15 @@
 .. changelog::
     :version: 1.2.0b1
 
+    .. change:: 3963
+        :tags: bug, orm
+        :tickets: 3963
+
+        Fixed bug to improve upon the specificity of loader options that
+        take effect subsequent to the lazy load of a related entity, so
+        that the loader options will match to an aliased or non-aliased
+        entity more specifically if those options include entity information.
+
     .. change:: 3740
         :tags: bug, sql
         :tickets: 3740
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index bae2b73e26..60ed2be304 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -376,7 +376,8 @@ class _UnboundLoad(Load):
                         _WILDCARD_TOKEN,) and c_token != p_prop.key:
                     return None
             elif isinstance(c_token, PropComparator):
-                if c_token.property is not p_prop:
+                if c_token.property is not p_prop or \
+                        c_token._parententity is not p_mapper:
                     return None
         else:
             i += 1
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 0d324b3ffb..3c669d90d6 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -4362,6 +4362,165 @@ class EntityViaMultiplePathTestTwo(fixtures.DeclarativeMappedTest):
         )
 
 
+class LazyLoadOptSpecificityTest(fixtures.DeclarativeMappedTest):
+    """test for [ticket:3963]"""
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class A(Base):
+            __tablename__ = 'a'
+            id = Column(Integer, primary_key=True)
+            bs = relationship("B")
+
+        class B(Base):
+            __tablename__ = 'b'
+            id = Column(Integer, primary_key=True)
+            a_id = Column(ForeignKey('a.id'))
+            cs = relationship("C")
+
+        class C(Base):
+            __tablename__ = 'c'
+            id = Column(Integer, primary_key=True)
+            b_id = Column(ForeignKey('b.id'))
+
+    @classmethod
+    def insert_data(cls):
+        A, B, C = cls.classes("A", "B", "C")
+        s = Session()
+        s.add(A(id=1, bs=[B(cs=[C()])]))
+        s.add(A(id=2))
+        s.commit()
+
+    def _run_tests(self, query, expected):
+        def go():
+            for a, _ in query:
+                for b in a.bs:
+                    b.cs
+        self.assert_sql_count(testing.db, go, expected)
+
+    def test_string_options_aliased_whatever(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(
+            joinedload("bs").joinedload("cs"))
+        self._run_tests(q, 1)
+
+    def test_string_options_unaliased_whatever(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(A, aa).filter(
+            aa.id == 2).filter(A.id == 1).options(
+            joinedload("bs").joinedload("cs"))
+        self._run_tests(q, 1)
+
+    def test_lazyload_aliased_abs_bcs_one(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(
+            joinedload(A.bs).joinedload(B.cs))
+        self._run_tests(q, 3)
+
+    def test_lazyload_aliased_abs_bcs_two(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(
+            defaultload(A.bs).joinedload(B.cs))
+        self._run_tests(q, 3)
+
+    def test_pathed_lazyload_aliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        opt = Load(A).joinedload(A.bs).joinedload(B.cs)
+
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(opt)
+        self._run_tests(q, 3)
+
+    def test_pathed_lazyload_plus_joined_aliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        opt = Load(aa).defaultload(aa.bs).joinedload(B.cs)
+
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(opt)
+        self._run_tests(q, 2)
+
+    def test_pathed_joinedload_aliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        opt = Load(aa).joinedload(aa.bs).joinedload(B.cs)
+
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(opt)
+        self._run_tests(q, 1)
+
+    def test_lazyload_plus_joined_aliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(
+            defaultload(aa.bs).joinedload(B.cs))
+        self._run_tests(q, 2)
+
+    def test_joinedload_aliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(aa, A).filter(
+            aa.id == 1).filter(A.id == 2).options(
+            joinedload(aa.bs).joinedload(B.cs))
+        self._run_tests(q, 1)
+
+    def test_lazyload_unaliased_abs_bcs_one(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(A, aa).filter(
+            aa.id == 2).filter(A.id == 1).options(
+            joinedload(aa.bs).joinedload(B.cs))
+        self._run_tests(q, 3)
+
+    def test_lazyload_unaliased_abs_bcs_two(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(A, aa).filter(
+            aa.id == 2).filter(A.id == 1).options(
+            defaultload(aa.bs).joinedload(B.cs))
+        self._run_tests(q, 3)
+
+    def test_lazyload_plus_joined_unaliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(A, aa).filter(
+            aa.id == 2).filter(A.id == 1).options(
+            defaultload(A.bs).joinedload(B.cs))
+        self._run_tests(q, 2)
+
+    def test_joinedload_unaliased_abs_bcs(self):
+        A, B, C = self.classes("A", "B", "C")
+        s = Session()
+        aa = aliased(A)
+        q = s.query(A, aa).filter(
+            aa.id == 2).filter(A.id == 1).options(
+            joinedload(A.bs).joinedload(B.cs))
+        self._run_tests(q, 1)
+
+
 class EntityViaMultiplePathTestThree(fixtures.DeclarativeMappedTest):
     """test for [ticket:3811] continuing on [ticket:3431]"""
 
diff --git a/test/orm/test_options.py b/test/orm/test_options.py
index 556f73c4b8..b7c574e2a5 100644
--- a/test/orm/test_options.py
+++ b/test/orm/test_options.py
@@ -306,6 +306,54 @@ class OptionsTest(PathTest, QueryTest):
         opt = self._option_fixture(Order.items, Item.keywords)
         self._assert_path_result(opt, q, [])
 
+    def test_with_current_nonmatching_entity(self):
+        Item, User, Order = (self.classes.Item,
+                             self.classes.User,
+                             self.classes.Order)
+
+        sess = Session()
+        q = sess.query(Item)._with_current_path(
+            self._make_path_registry(
+                [inspect(aliased(User)), 'orders', Order, 'items'])
+        )
+
+        opt = self._option_fixture(User.orders)
+        self._assert_path_result(opt, q, [])
+
+        opt = self._option_fixture(User.orders, Order.items, Item.keywords)
+        self._assert_path_result(opt, q, [])
+
+        q = sess.query(Item)._with_current_path(
+            self._make_path_registry(
+                [User, 'orders', Order, 'items'])
+        )
+
+        ac = aliased(User)
+
+        opt = self._option_fixture(ac.orders)
+        self._assert_path_result(opt, q, [])
+
+        opt = self._option_fixture(ac.orders, Order.items, Item.keywords)
+        self._assert_path_result(opt, q, [])
+
+    def test_with_current_match_aliased_classes(self):
+        Item, User, Order = (self.classes.Item,
+                             self.classes.User,
+                             self.classes.Order)
+
+        ac = aliased(User)
+        sess = Session()
+        q = sess.query(Item)._with_current_path(
+            self._make_path_registry(
+                [inspect(ac), 'orders', Order, 'items'])
+        )
+
+        opt = self._option_fixture(ac.orders, Order.items, Item.keywords)
+        self._assert_path_result(opt, q, [(Item, "keywords")])
+
+        opt = self._option_fixture(ac.orders, Order.items)
+        self._assert_path_result(opt, q, [])
+
     def test_from_base_to_subclass_attr(self):
         Dingaling, Address = self.classes.Dingaling, self.classes.Address