From 111a05a35b9fb4e6683bd18d3064634e002233f6 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 13 Aug 2026 17:43:13 -0400 Subject: [PATCH] Use the shallowest path when an object is loaded at multiple paths 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 | 52 ++++++ doc/build/changelog/unreleased_21/13507.rst | 17 ++ lib/sqlalchemy/orm/loading.py | 23 ++- test/orm/test_loading.py | 172 ++++++++++++++++++++ 4 files changed, 262 insertions(+), 2 deletions(-) create mode 100644 doc/build/changelog/unreleased_21/13507.rst diff --git a/doc/build/changelog/migration_21.rst b/doc/build/changelog/migration_21.rst index 2aff6792c5..9f63072559 100644 --- a/doc/build/changelog/migration_21.rst +++ b/doc/build/changelog/migration_21.rst @@ -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 index 0000000000..54773c10ac --- /dev/null +++ b/doc/build/changelog/unreleased_21/13507.rst @@ -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` diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py index 7d703357c2..ef25362f6e 100644 --- a/lib/sqlalchemy/orm/loading.py +++ b/lib/sqlalchemy/orm/loading.py @@ -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. diff --git a/test/orm/test_loading.py b/test/orm/test_loading.py index 29397d45bf..43e582728c 100644 --- a/test/orm/test_loading.py +++ b/test/orm/test_loading.py @@ -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" -- 2.47.3