]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Use the shallowest path when an object is loaded at multiple paths main
authorMike Bayer <mike_mp@zzzcomputing.com>
Thu, 13 Aug 2026 21:43:13 +0000 (17:43 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Thu, 13 Aug 2026 22:07:18 +0000 (18:07 -0400)
Fixed long-standing issue where an object that was loaded at more than
one path within a single query, such as when a chain of joinedload()
options leads back to an entity that was also loaded at the top level of
the query, would retain the loader options of whichever path the query
happened to see last, which varied with the loader strategy in use.  The
options an object retains are applied to all forms of lazy loading for
that object, so an otherwise identical set of options could behave
differently depending on the loader strategy.  The shallowest path is
now favored, which is deterministic.

Fixes: #13507
Change-Id: I438e6655a9ecf59ca554312a7a87c8b85b4879d2

doc/build/changelog/migration_21.rst
doc/build/changelog/unreleased_21/13507.rst [new file with mode: 0644]
lib/sqlalchemy/orm/loading.py
test/orm/test_loading.py

index 2aff6792c5e8b17abac69b6c17bcae10d3d846ad..9f630725597b4054b4f84e7f87de2c3b6bcdb130 100644 (file)
@@ -1106,6 +1106,58 @@ raise an error, directing users to use :class:`_sql.FrameClause` instead.
 
 :ticket:`12596`
 
+.. _change_13507:
+
+Loader options from a deeper path no longer apply to an object loaded at the top
+---------------------------------------------------------------------------------
+
+The same object can be loaded more than once within a single query.  Given
+``A.b`` referring to ``B``, ``B.a`` referring back to ``A``, and ``A.c``
+referring to ``C``, the query below loads ``A`` twice: once as the entity
+being selected, and again underneath ``A.b -> B.a``::
+
+    stmt = select(A).options(
+        joinedload(A.b).joinedload(B.a).raiseload("*"),
+        joinedload(A.c),
+    )
+
+    a = session.scalars(stmt).unique().one()
+
+SQLAlchemy remembers which of those two paths an object was loaded under, and
+applies the loader options from that path whenever more SQL is emitted for the
+object later on::
+
+    session.expire(a)
+
+    a.value  # refresh, emitting SELECT for the "a" row
+
+    a.c  # 2.0: raises InvalidRequestError; 2.1: loads normally
+
+In 2.0 it was difficult to predict which of the paths would be the one
+remembered, as it varied with the loader strategy in use, so the
+``raiseload("*")`` written for ``A.b -> B.a`` could end up applied to ``a``
+itself.  In 2.1 the shallowest path is favored, a deterministic rule rather
+than one based on which loader strategy within the query happened to see the
+object first.
+
+This applies to all forms of :term:`lazy loading`, such as when expired
+attributes are unexpired, deferred columns are loaded, or unloaded
+relationships are loaded::
+
+    stmt = select(A).options(
+        joinedload(A.b).joinedload(B.a).lazyload(A.c).joinedload(C.d),
+    )
+
+    a = session.scalars(stmt).unique().one()
+
+    a.c  # 2.0: SELECT from "c" with a JOIN to "d"; 2.1: SELECT from "c"
+
+Code that relies on complex interactions of overlapping paths may need
+adjustment, as the behavior should now be consistent across the different
+kinds of loader option.
+
+:ticket:`13507`
+
 
 Core - Behavioral Changes and Improvements
 ==========================================
diff --git a/doc/build/changelog/unreleased_21/13507.rst b/doc/build/changelog/unreleased_21/13507.rst
new file mode 100644 (file)
index 0000000..54773c1
--- /dev/null
@@ -0,0 +1,17 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 13507
+
+    Fixed long-standing issue where an object that was loaded at more than one
+    path within a single query, such as when a chain of :func:`_orm.joinedload`
+    options leads back to an entity that was also loaded at the top level of
+    the query, would retain the loader options of whichever path the query
+    happened to see last, which varied with the loader strategy in use.  The
+    options an object retains are applied to all forms of :term:`lazy loading`
+    for that object, so an otherwise identical set of options could behave
+    differently depending on the loader strategy.  The shallowest path is now
+    favored, which is deterministic.
+
+    .. seealso::
+
+        :ref:`change_13507`
index 7d703357c254393b5c9dc974da3a6ca02d1ef801..ef25362f6e51cd25859a812f15e28b276e6f6f9d 100644 (file)
@@ -1365,8 +1365,27 @@ def _populate_full(
 
     elif load_path != state.load_path:
         # new load path, e.g. object is present in more than one
-        # column position in a series of rows
-        state.load_path = load_path
+        # column position in a series of rows.
+        #
+        # the shallowest path wins.  state.load_path is paired with
+        # state.load_options and the two are replayed together when the
+        # object is later refreshed or unexpired; taking whichever path
+        # happened to be processed last made that replay depend on column
+        # order within the row, row order within the result, and which
+        # eager loader style was in use.  the shallowest path is both
+        # deterministic and the most conservative choice, as a deeper
+        # path matches loader options that were registered for some other
+        # occurrence of this entity.  See #13507.
+        #
+        # only move the path if the current load is the one that stamped
+        # it; the condition here mirrors the one in _instance_processor()
+        # that assigns load_path / load_options together.  when
+        # populate_existing is in effect with no propagated options, the
+        # path in place belongs to a previous load and is left alone.
+        if len(load_path) < len(state.load_path) and (
+            context.propagated_loader_options or not populate_existing
+        ):
+            state.load_path = load_path
 
         # if we have data, and the data isn't in the dict, OK, let's put
         # it in.
index 29397d45bf14a93998962fbe57c9a26fecce6ed4..43e582728c1cee70c2d8f0410a79526669d588af 100644 (file)
@@ -1,6 +1,8 @@
 from sqlalchemy import delete
 from sqlalchemy import exc
+from sqlalchemy import ForeignKey
 from sqlalchemy import insert
+from sqlalchemy import inspect
 from sqlalchemy import Integer
 from sqlalchemy import literal
 from sqlalchemy import literal_column
@@ -10,9 +12,14 @@ from sqlalchemy import testing
 from sqlalchemy import text
 from sqlalchemy import TypeDecorator
 from sqlalchemy import update
+from sqlalchemy.orm import immediateload
+from sqlalchemy.orm import joinedload
+from sqlalchemy.orm import lazyload
 from sqlalchemy.orm import loading
 from sqlalchemy.orm import relationship
+from sqlalchemy.orm import selectinload
 from sqlalchemy.orm import Session
+from sqlalchemy.orm import subqueryload
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_true
 from sqlalchemy.testing import mock
@@ -165,6 +172,171 @@ class InstanceProcessorTest(_fixtures.FixtureTest):
         self.assert_sql_count(testing.db, go, 1)
 
 
+class MultiPathLoadPathTest(_fixtures.FixtureTest):
+    """test #13507.
+
+    When an object occupies more than one column position within a single
+    result, ``state.load_path`` is the shallowest of those paths, rather
+    than whichever position happened to be processed last.  As
+    ``state.load_path`` is what drives the loader options replayed when the
+    object is later refreshed or unexpired, taking the last position made
+    those options depend on the loader strategy in use, on column order
+    within the row and on row order within the result.
+
+    """
+
+    run_inserts = "once"
+    run_deletes = None
+
+    @classmethod
+    def setup_mappers(cls):
+        User, Address, Order = cls.classes("User", "Address", "Order")
+        users, addresses, orders = cls.tables("users", "addresses", "orders")
+
+        cls.mapper_registry.map_imperatively(
+            User,
+            users,
+            properties={
+                "addresses": relationship(Address, backref="user"),
+                "orders": relationship(Order),
+            },
+        )
+        cls.mapper_registry.map_imperatively(Address, addresses)
+        cls.mapper_registry.map_imperatively(Order, orders)
+
+    @testing.combinations(
+        joinedload,
+        selectinload,
+        subqueryload,
+        immediateload,
+        lazyload,
+        argnames="fn",
+        id_="n",
+    )
+    def test_load_path_is_shallowest_path(self, fn):
+        """the root path stays in place even though ``User`` is also
+        loaded underneath ``Address.user``"""
+
+        User, Address = self.classes("User", "Address")
+
+        sess = fixture_session()
+        u1 = (
+            sess.scalars(
+                select(User)
+                .where(User.id == 7)
+                .options(fn(User.addresses).options(fn(Address.user)))
+            )
+            .unique()
+            .one()
+        )
+
+        eq_(inspect(u1).load_path.path, (inspect(User),))
+
+    @testing.combinations(
+        joinedload,
+        selectinload,
+        subqueryload,
+        immediateload,
+        lazyload,
+        argnames="fn",
+        id_="n",
+    )
+    def test_refresh_ignores_cyclic_raiseload(self, fn):
+        """a ``raiseload()`` declared under a path that cycles back to
+        ``User`` is not applied to the ``User`` that was loaded at the
+        root of the query"""
+
+        User, Address = self.classes("User", "Address")
+
+        sess = fixture_session()
+        u1 = (
+            sess.scalars(
+                select(User)
+                .where(User.id == 7)
+                .options(
+                    fn(User.addresses).options(
+                        fn(Address.user).raiseload("*")
+                    ),
+                    fn(User.orders),
+                )
+            )
+            .unique()
+            .one()
+        )
+
+        sess.expire(u1)
+
+        # refresh u1; only the "users" row is fetched
+        eq_(u1.name, "jack")
+
+        eq_(inspect(u1).callables, {})
+
+        eq_(len(u1.orders), 3)
+
+
+class MultiPathShallowestLoadPathTest(fixtures.DeclarativeMappedTest):
+    """test #13507, continued.
+
+    the shallowest path wins regardless of the order in which the paths
+    are encountered, i.e. it does not matter whether the object showed up
+    under a relationship first and as the root entity later, or the
+    reverse.
+
+    """
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class Node(Base):
+            __tablename__ = "node"
+
+            id = Column(Integer, primary_key=True)
+            parent_id = Column(ForeignKey("node.id"))
+            parent = relationship("Node", remote_side=[id])
+            name = Column(String(10))
+
+    @classmethod
+    def insert_data(cls, connection):
+        Node = cls.classes.Node
+
+        with Session(connection) as session:
+            session.add_all(
+                [
+                    Node(id=1, name="n1"),
+                    Node(id=2, parent_id=1, name="n2"),
+                    Node(id=3, parent_id=1, name="n3"),
+                ]
+            )
+            session.commit()
+
+    @testing.combinations("asc", "desc", argnames="direction")
+    def test_shallowest_path_wins(self, direction):
+        Node = self.classes.Node
+
+        sess = fixture_session()
+
+        # descending, Node #1 is seen first under Node.parent in the rows
+        # for #3 and #2 and only afterwards as the root entity in its own
+        # row; ascending, the reverse.  either way the root path is the
+        # one that's kept
+        order_by = Node.id.asc() if direction == "asc" else Node.id.desc()
+
+        nodes = (
+            sess.scalars(
+                select(Node)
+                .order_by(order_by)
+                .options(joinedload(Node.parent))
+            )
+            .unique()
+            .all()
+        )
+
+        n1 = {n.id: n for n in nodes}[1]
+
+        eq_(inspect(n1).load_path.path, (inspect(Node),))
+
+
 class InstancesTest(_fixtures.FixtureTest):
     run_setup_mappers = "once"
     run_inserts = "once"