consider propagate_to_loaders at application time

Fixed regression from 1.4 where using :func:`_orm.defaultload` in
conjunction with a non-propagating loader like :func:`_orm.contains_eager`
would nonetheless propagate the :func:`_orm.contains_eager` to a lazy load
operation, causing incorrect queries as this option is only intended to
come from an original load.

Fixes: #11292
Change-Id: I79928afa108970b523f2166c3190f7952eca73ed
(cherry picked from commit 80399cefa1b16a8548ba0c997a1eda94b8e9db01)

diff --git a/doc/build/changelog/unreleased_20/11292.rst b/doc/build/changelog/unreleased_20/11292.rst
new file mode 100644 (file)
index 0000000..65fbdf7
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/11292.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm, regression
+    :tickets: 11292
+
+    Fixed regression from 1.4 where using :func:`_orm.defaultload` in
+    conjunction with a non-propagating loader like :func:`_orm.contains_eager`
+    would nonetheless propagate the :func:`_orm.contains_eager` to a lazy load
+    operation, causing incorrect queries as this option is only intended to
+    come from an original load.
+
+

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 25c6332112ffa190909ae1c0cc10f57d199b5f15..31c3a54e3238c267bedbb2d7fb7112b9ecabfde9 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -98,6 +98,7 @@ class _AbstractLoad(traversals.GenerativeOnTraversal, LoaderOption):
         attr: _AttrType,
         alias: Optional[_FromClauseArgument] = None,
         _is_chain: bool = False,
+        _propagate_to_loaders: bool = False,
     ) -> Self:
         r"""Indicate that the given attribute should be eagerly loaded from
         columns stated manually in the query.
@@ -158,7 +159,7 @@ class _AbstractLoad(traversals.GenerativeOnTraversal, LoaderOption):
         cloned = self._set_relationship_strategy(
             attr,
             {"lazy": "joined"},
-            propagate_to_loaders=False,
+            propagate_to_loaders=_propagate_to_loaders,
             opts={"eager_from_alias": coerced_alias},
             _reconcile_to_other=True if _is_chain else None,
         )
@@ -1146,7 +1147,20 @@ class Load(_AbstractLoad):
             mapper_entities, raiseerr
         )
 
+        # if the context has a current path, this is a lazy load
+        has_current_path = bool(compile_state.compile_options._current_path)
+
         for loader in self.context:
+            # issue #11292
+            # historically, propagate_to_loaders was only considered at
+            # object loading time, whether or not to carry along options
+            # onto an object's loaded state where it would be used by lazyload.
+            # however, the defaultload() option needs to propagate in case
+            # its sub-options propagate_to_loaders, but its sub-options
+            # that dont propagate should not be applied for lazy loaders.
+            # so we check again
+            if has_current_path and not loader.propagate_to_loaders:
+                continue
             loader.process_compile_state(
                 self,
                 compile_state,

diff --git a/test/orm/test_default_strategies.py b/test/orm/test_default_strategies.py
index 657875aa9d8c3d4cb5a349a2b13e755b59ab1017..178b03fe6f6f355b5dfde630b4754d822537ac59 100644 (file)
--- a/test/orm/test_default_strategies.py
+++ b/test/orm/test_default_strategies.py
@@ -1,11 +1,18 @@
 import sqlalchemy as sa
+from sqlalchemy import Column
+from sqlalchemy import ForeignKey
+from sqlalchemy import Integer
+from sqlalchemy import select
 from sqlalchemy import testing
 from sqlalchemy import util
+from sqlalchemy.orm import contains_eager
 from sqlalchemy.orm import defaultload
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm import relationship
+from sqlalchemy.orm import Session
 from sqlalchemy.orm import subqueryload
 from sqlalchemy.testing import eq_
+from sqlalchemy.testing import fixtures
 from sqlalchemy.testing.assertions import expect_raises_message
 from sqlalchemy.testing.fixtures import fixture_session
 from test.orm import _fixtures
@@ -738,3 +745,122 @@ class NoLoadTest(_fixtures.FixtureTest):
             eq_(a1.user, None)
 
         self.sql_count_(0, go)
+
+
+class Issue11292Test(fixtures.DeclarativeMappedTest):
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class Parent(Base):
+            __tablename__ = "parent"
+
+            id = Column(Integer, primary_key=True)
+
+            extension = relationship(
+                "Extension", back_populates="parent", uselist=False
+            )
+
+        class Child(Base):
+            __tablename__ = "child"
+
+            id = Column(Integer, primary_key=True)
+
+            extensions = relationship("Extension", back_populates="child")
+
+        class Extension(Base):
+            __tablename__ = "extension"
+
+            id = Column(Integer, primary_key=True)
+            parent_id = Column(Integer, ForeignKey(Parent.id))
+            child_id = Column(Integer, ForeignKey(Child.id))
+
+            parent = relationship("Parent", back_populates="extension")
+            child = relationship("Child", back_populates="extensions")
+
+    @classmethod
+    def insert_data(cls, connection):
+        Parent, Child, Extension = cls.classes("Parent", "Child", "Extension")
+        with Session(connection) as session:
+            for id_ in (1, 2, 3):
+                session.add(Parent(id=id_))
+                session.add(Child(id=id_))
+                session.add(Extension(id=id_, parent_id=id_, child_id=id_))
+            session.commit()
+
+    @testing.variation("load_as_option", [True, False])
+    def test_defaultload_dont_propagate(self, load_as_option):
+        Parent, Child, Extension = self.classes("Parent", "Child", "Extension")
+
+        session = fixture_session()
+
+        # here, we want the defaultload() to go away on subsequent loads,
+        # becuase Parent.extension is propagate_to_loaders=False
+        query = (
+            select(Parent)
+            .join(Extension)
+            .join(Child)
+            .options(
+                contains_eager(Parent.extension),
+                (
+                    defaultload(Parent.extension).options(
+                        contains_eager(Extension.child)
+                    )
+                    if load_as_option
+                    else defaultload(Parent.extension).contains_eager(
+                        Extension.child
+                    )
+                ),
+            )
+        )
+
+        parents = session.scalars(query).all()
+
+        eq_(
+            [(p.id, p.extension.id, p.extension.child.id) for p in parents],
+            [(1, 1, 1), (2, 2, 2), (3, 3, 3)],
+        )
+
+        session.expire_all()
+
+        eq_(
+            [(p.id, p.extension.id, p.extension.child.id) for p in parents],
+            [(1, 1, 1), (2, 2, 2), (3, 3, 3)],
+        )
+
+    @testing.variation("load_as_option", [True, False])
+    def test_defaultload_yes_propagate(self, load_as_option):
+        Parent, Child, Extension = self.classes("Parent", "Child", "Extension")
+
+        session = fixture_session()
+
+        # here, we want the defaultload() to go away on subsequent loads,
+        # becuase Parent.extension is propagate_to_loaders=False
+        query = select(Parent).options(
+            (
+                defaultload(Parent.extension).options(
+                    joinedload(Extension.child)
+                )
+                if load_as_option
+                else defaultload(Parent.extension).joinedload(Extension.child)
+            ),
+        )
+
+        parents = session.scalars(query).all()
+
+        eq_(
+            [(p.id, p.extension.id, p.extension.child.id) for p in parents],
+            [(1, 1, 1), (2, 2, 2), (3, 3, 3)],
+        )
+
+        session.expire_all()
+
+        # this would be 9 without the joinedload
+        with self.assert_statement_count(testing.db, 6):
+            eq_(
+                [
+                    (p.id, p.extension.id, p.extension.child.id)
+                    for p in parents
+                ],
+                [(1, 1, 1), (2, 2, 2), (3, 3, 3)],
+            )

diff --git a/test/orm/test_options.py b/test/orm/test_options.py
index db9b51607c3ab3e0440a1974e1d30ad364e522e1..c6058a80b3b4cacc8a66baeb3b156ab30a2fb48e 100644 (file)
--- a/test/orm/test_options.py
+++ b/test/orm/test_options.py
@@ -419,7 +419,10 @@ class OptionsTest(PathTest, QueryTest):
         # loader option works this way right now; the rest all use
         # defaultload() for the "chain" elements
         return strategy_options._generate_from_keys(
-            strategy_options.Load.contains_eager, arg, True, {}
+            strategy_options.Load.contains_eager,
+            arg,
+            True,
+            dict(_propagate_to_loaders=True),
         )
 
     @testing.combinations(

diff --git a/test/profiles.txt b/test/profiles.txt
index d8226f4a8944ae3fe115f1cd5a83ce3fc5933365..370d895b6275a9eff80ecd474916f670a31da750 100644 (file)
--- a/test/profiles.txt
+++ b/test/profiles.txt
@@ -1,15 +1,15 @@
 # /home/classic/dev/sqlalchemy/test/profiles.txt
 # This file is written out on a per-environment basis.
-# For each test in aaa_profiling, the corresponding function and 
+# For each test in aaa_profiling, the corresponding function and
 # environment is located within this file.  If it doesn't exist,
 # the test is skipped.
-# If a callcount does exist, it is compared to what we received. 
+# If a callcount does exist, it is compared to what we received.
 # assertions are raised if the counts do not match.
-# 
-# To add a new callcount test, apply the function_call_count 
-# decorator and re-run the tests using the --write-profiles 
+#
+# To add a new callcount test, apply the function_call_count
+# decorator and re-run the tests using the --write-profiles
 # option - this file will be rewritten including the new count.
-# 
+#
 
 # TEST: test.aaa_profiling.test_compiler.CompileTest.test_insert
 
@@ -209,15 +209,17 @@ test.aaa_profiling.test_orm.AttributeOverheadTest.test_collection_append_remove
 
 # TEST: test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching
 
-test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_cextensions 128
-test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_nocextensions 128
-test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.12_sqlite_pysqlite_dbapiunicode_cextensions 124
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_cextensions 136
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_nocextensions 136
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.12_sqlite_pysqlite_dbapiunicode_cextensions 132
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_key_bound_branching x86_64_linux_cpython_3.12_sqlite_pysqlite_dbapiunicode_nocextensions 132
 
 # TEST: test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching
 
-test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_cextensions 128
-test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_nocextensions 128
-test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.12_sqlite_pysqlite_dbapiunicode_cextensions 124
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_cextensions 136
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.11_sqlite_pysqlite_dbapiunicode_nocextensions 136
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.12_sqlite_pysqlite_dbapiunicode_cextensions 132
+test.aaa_profiling.test_orm.BranchedOptionTest.test_query_opts_unbound_branching x86_64_linux_cpython_3.12_sqlite_pysqlite_dbapiunicode_nocextensions 132
 
 # TEST: test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline