Assemble "don't joinedload other side" rule using query._current_path

Discovered during testing for [ticket:3822], the rule added
for [ticket:1495] will fail if the source object has propagated
options set up, which add elements to query._current_path.

Fixes: #3824
Change-Id: I3d96c96fee5f9b247f739d2136d18681ac61f2fe

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index b90b932b55402a7c1a8715d79a16625968bf4bbf..127227ded9e03f657cb70d9de7e2102ef3892a17 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -31,6 +31,14 @@
         was a ``functools.partial`` or other object that doesn't have a
         ``__module__`` attribute.
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3824
+
+        Fixed bug involving the rule to disable a joined collection eager
+        loader on the other side of a many-to-one lazy loader, first added
+        in [ticket:1495], where the rule would fail if the parent object
+        had some other lazyloader-bound query options associated with it.
 
 .. changelog::
     :version: 1.1.1

diff --git a/lib/sqlalchemy/ext/baked.py b/lib/sqlalchemy/ext/baked.py
index 3ca94925eba69875f7586f7bfbcabb10b5645559..2f658edf3ab2e5ebeabae84d90f96d7b55115bb2 100644 (file)
--- a/lib/sqlalchemy/ext/baked.py
+++ b/lib/sqlalchemy/ext/baked.py
@@ -467,11 +467,15 @@ class BakedLazyLoader(strategies.LazyLoader):
             if rev.direction is interfaces.MANYTOONE and \
                 rev._use_get and \
                     not isinstance(rev.strategy, strategies.LazyLoader):
+
                 q.add_criteria(
                     lambda q:
                     q.options(
-                        strategy_options.Load(
-                            rev.parent).baked_lazyload(rev.key)))
+                        strategy_options.Load.for_existing_path(
+                            q._current_path[rev.parent]
+                        ).baked_lazyload(rev.key)
+                    )
+                )
 
         lazy_clause, params = self._generate_lazy_clause(state, passive)
 

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index b2cd5b5ec89910069c905662667a1034e7373915..5f5ab10690c7ceb1db530e251d61b2db9a55161d 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -615,7 +615,10 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
                 rev._use_get and \
                     not isinstance(rev.strategy, LazyLoader):
                 q = q.options(
-                    strategy_options.Load(rev.parent).lazyload(rev.key))
+                    strategy_options.Load.for_existing_path(
+                        q._current_path[rev.parent]
+                    ).lazyload(rev.key)
+                )
 
         lazy_clause, params = self._generate_lazy_clause(
             state, passive=passive)

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 2fb13f3cfaec26fe887e9314a072172371bc885a..0c1ebf404e2773d3c9491be7c08b11fbc7f079be 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -85,6 +85,14 @@ class Load(Generative, MapperOption):
         self.context = {}
         self.local_opts = {}
 
+    @classmethod
+    def for_existing_path(cls, path):
+        load = cls.__new__(cls)
+        load.path = path
+        load.context = {}
+        load.local_opts = {}
+        return load
+
     def _generate(self):
         cloned = super(Load, self)._generate()
         cloned.local_opts = {}

diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py
index 2f501eb6c03dbf2df4bad0bb45db0b8f6dec2cb3..818019de2b245b710d2be3bed54225a1ffe23a9c 100644 (file)
--- a/test/ext/test_baked.py
+++ b/test/ext/test_baked.py
@@ -1,5 +1,5 @@
 from sqlalchemy.orm import Session, subqueryload, \
-    mapper, relationship, lazyload, clear_mappers
+    mapper, relationship, lazyload, clear_mappers, backref
 from sqlalchemy.testing import eq_, is_, is_not_
 from sqlalchemy.testing import assert_raises, assert_raises_message
 from sqlalchemy import testing
@@ -628,7 +628,9 @@ class ResultTest(BakedTest):
                 sess.close()
 
 
-class LazyLoaderTest(BakedTest):
+from sqlalchemy.testing.assertsql import CompiledSQL
+
+class LazyLoaderTest(testing.AssertsCompiledSQL, BakedTest):
     run_setup_mappers = 'each'
 
     def _o2m_fixture(self, lazy="select", **kw):
@@ -920,6 +922,72 @@ class LazyLoaderTest(BakedTest):
 
                 sess.close()
 
+    def test_useget_cancels_eager(self):
+        """test that a one to many lazyload cancels the unnecessary
+        eager many-to-one join on the other side."""
+
+        User = self.classes.User
+        Address = self.classes.Address
+
+        mapper(User, self.tables.users)
+        mapper(Address, self.tables.addresses, properties={
+            'user': relationship(
+                User, lazy='joined',
+                backref=backref('addresses', lazy='baked_select')
+            )
+        })
+
+        sess = Session()
+        u1 = sess.query(User).filter(User.id == 8).one()
+
+        def go():
+            eq_(u1.addresses[0].user, u1)
+        self.assert_sql_execution(
+            testing.db, go,
+            CompiledSQL(
+                "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                "addresses_user_id, addresses.email_address AS "
+                "addresses_email_address FROM addresses WHERE :param_1 = "
+                "addresses.user_id",
+                {'param_1': 8})
+            )
+
+    def test_useget_cancels_eager_propagated_present(self):
+        """test that a one to many lazyload cancels the unnecessary
+        eager many-to-one join on the other side, even when a propagated
+        option is present."""
+
+        User = self.classes.User
+        Address = self.classes.Address
+
+        mapper(User, self.tables.users)
+        mapper(Address, self.tables.addresses, properties={
+            'user': relationship(
+                User, lazy='joined',
+                backref=backref('addresses', lazy='baked_select')
+            )
+        })
+
+        from sqlalchemy.orm.interfaces import MapperOption
+
+        class MyBogusOption(MapperOption):
+            propagate_to_loaders = True
+
+        sess = Session()
+        u1 = sess.query(User).options(MyBogusOption()).filter(User.id == 8).one()
+
+        def go():
+            eq_(u1.addresses[0].user, u1)
+        self.assert_sql_execution(
+            testing.db, go,
+            CompiledSQL(
+                "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                "addresses_user_id, addresses.email_address AS "
+                "addresses_email_address FROM addresses WHERE :param_1 = "
+                "addresses.user_id",
+                {'param_1': 8})
+            )
+
     # additional tests:
     # 1. m2m w lazyload
     # 2. o2m lazyload where m2o backrefs have an eager load, test

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 759d4c5ba20c24f862812556a88248ce8d0c4cc6..e57e2b3c2aec95f9d21065d55679bab5028b2faa 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -898,6 +898,43 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
                 {'param_1': 8})
             )
 
+    def test_useget_cancels_eager_propagated_present(self):
+        """test that a one to many lazyload cancels the unnecessary
+        eager many-to-one join on the other side, even when a propagated
+        option is present."""
+
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User)
+
+        mapper(User, users)
+        mapper(Address, addresses, properties={
+            'user': relationship(User, lazy='joined', backref='addresses')
+        })
+
+        from sqlalchemy.orm.interfaces import MapperOption
+
+        class MyBogusOption(MapperOption):
+            propagate_to_loaders = True
+
+        sess = create_session()
+        u1 = sess.query(User).options(MyBogusOption()).filter(User.id == 8).one()
+
+        def go():
+            eq_(u1.addresses[0].user, u1)
+        self.assert_sql_execution(
+            testing.db, go,
+            CompiledSQL(
+                "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                "addresses_user_id, addresses.email_address AS "
+                "addresses_email_address FROM addresses WHERE :param_1 = "
+                "addresses.user_id",
+                {'param_1': 8})
+            )
+
+
     def test_manytoone_limit(self):
         """test that the subquery wrapping only occurs with
         limit/offset and m2m or o2m joins present."""