From: Mike Bayer Date: Tue, 25 Jul 2023 15:26:03 +0000 (-0400) Subject: implement join_depth for immediateload, fix expunged loading X-Git-Tag: rel_2_0_20~28^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9f9ad6c0c9d71330c16dc9bcc74a84eba024e00d;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git implement join_depth for immediateload, fix expunged loading Fixed issue where the ``lazy="immediateload"`` loader strategy would place an internal loading token into the ORM mapped attribute under circumstances where the load should not occur, such as in a recursive self-referential load. As part of this change, the ``lazy="immediateload"`` strategy now honors the :paramref:`_orm.relationship.join_depth` parameter for self-referential eager loads in the same way as that of other eager loaders, where leaving it unset or set at zero will lead to a self-referential immediateload not occurring, setting it to a value of one or greater will immediateload up until that given depth. Fixes: #10139 Change-Id: I319d8cd86d7dd57f8ba61a9d483dc84505e02c72 --- diff --git a/doc/build/changelog/unreleased_20/10139.rst b/doc/build/changelog/unreleased_20/10139.rst new file mode 100644 index 0000000000..e3b706ab00 --- /dev/null +++ b/doc/build/changelog/unreleased_20/10139.rst @@ -0,0 +1,14 @@ +.. change:: + :tags: orm, bug + :tickets: 10139 + + Fixed issue where the ``lazy="immediateload"`` loader strategy would place + an internal loading token into the ORM mapped attribute under circumstances + where the load should not occur, such as in a recursive self-referential + load. As part of this change, the ``lazy="immediateload"`` strategy now + honors the :paramref:`_orm.relationship.join_depth` parameter for + self-referential eager loads in the same way as that of other eager + loaders, where leaving it unset or set at zero will lead to a + self-referential immediateload not occurring, setting it to a value of one + or greater will immediateload up until that given depth. + diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 2ce313a43b..4e677410dc 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -1348,7 +1348,11 @@ class PostLoader(AbstractRelationshipLoader): @relationships.RelationshipProperty.strategy_for(lazy="immediate") class ImmediateLoader(PostLoader): - __slots__ = () + __slots__ = ("join_depth",) + + def __init__(self, parent, strategy_key): + super().__init__(parent, strategy_key) + self.join_depth = self.parent_property.join_depth def init_class_attribute(self, mapper): self.parent_property._get_strategy( @@ -1371,7 +1375,7 @@ class ImmediateLoader(PostLoader): run_loader, execution_options, recursion_depth, - ) = self._setup_for_recursion(context, path, loadopt) + ) = self._setup_for_recursion(context, path, loadopt, self.join_depth) if not run_loader: # this will not emit SQL and will only emit for a many-to-one # "use get" load. the "_RELATED" part means it may return @@ -1430,7 +1434,10 @@ class ImmediateLoader(PostLoader): alternate_effective_path=alternate_effective_path, execution_options=execution_options, ) - if value is not ATTR_WAS_SET: + if value not in ( + ATTR_WAS_SET, + LoaderCallableStatus.PASSIVE_NO_RESULT, + ): state.get_impl(key).set_committed_value( state, dict_, value ) diff --git a/test/orm/_fixtures.py b/test/orm/_fixtures.py index 094568a376..783bfa14ab 100644 --- a/test/orm/_fixtures.py +++ b/test/orm/_fixtures.py @@ -279,7 +279,11 @@ class FixtureTest(fixtures.MappedTest): Column( "id", Integer, primary_key=True, test_needs_autoincrement=True ), - Column("parent_id", Integer, ForeignKey("nodes.id")), + Column( + "parent_id", + Integer, + ForeignKey("nodes.id"), + ), Column("data", String(30)), test_needs_acid=True, test_needs_fk=True, diff --git a/test/orm/test_immediate_load.py b/test/orm/test_immediate_load.py index 376157bf7f..c2f1e3bf39 100644 --- a/test/orm/test_immediate_load.py +++ b/test/orm/test_immediate_load.py @@ -1,9 +1,12 @@ """basic tests of lazy loaded attributes""" +from sqlalchemy import select from sqlalchemy import testing from sqlalchemy.orm import immediateload from sqlalchemy.orm import relationship +from sqlalchemy.orm import Session from sqlalchemy.testing import eq_ +from sqlalchemy.testing import is_ from sqlalchemy.testing.fixtures import fixture_session from test.orm import _fixtures @@ -201,3 +204,101 @@ class ImmediateTest(_fixtures.FixtureTest): # aren't fired off. This is because the "lazyload" strategy # does not invoke eager loaders. assert "user" not in u1.addresses[0].__dict__ + + +class SelfReferentialTest(_fixtures.FixtureTest): + run_inserts = None + run_deletes = "each" + + @testing.fixture + def node_fixture(self): + Node = self.classes.Node + nodes = self.tables.nodes + + def go(join_depth): + self.mapper_registry.map_imperatively( + Node, + nodes, + properties={ + "parent": relationship( + Node, + remote_side=nodes.c.id, + lazy="immediate", + join_depth=join_depth, + ) + }, + ) + + return Node + + yield go + + # 1. the fixture uses InnoDB, so foreign keys are enforced + # 2. "delete from nodes" in InnoDB is not smart enough to delete + # all rows in the correct order automatically + # 3. "DELETE..ORDER BY" is mysql specific. we want the TablesTest + # fixture to be generic + # 4. Can't add "ON DELETE CASCADE" to that fixture because SQL Server + # rejects it + # 5. so until we have a "delete from all tables taking FKs into + # account" routine, we need a custom teardown here for MySQL/MariaDB + # 6. A similar fixture in test_recursive_loaders is cheating since it's + # hardcoding to using MyISAM for MySQL + with Session(testing.db) as sess: + for node in sess.scalars(select(Node)): + sess.delete(node) + + sess.commit() + + @testing.variation("persistence", ["expunge", "keep", "reload"]) + @testing.combinations((None,), (1,), (2,), argnames="join_depth") + def test_self_referential_recursive( + self, persistence, join_depth, node_fixture + ): + """test #10139""" + + Node = node_fixture(join_depth) + + sess = fixture_session() + n0 = Node(data="n0") + n1 = Node(data="n1") + n2 = Node(data="n2") + n1.parent = n0 + n2.parent = n1 + sess.add_all([n0, n1, n2]) + sess.commit() + if persistence.expunge or persistence.reload: + sess.close() + + if persistence.reload: + sess.add(n1) + sess.add(n0) + + n2 = sess.query(Node).filter(Node.data == "n2").one() + + if persistence.expunge and (join_depth is None or join_depth < 1): + expected_count = 1 + else: + expected_count = 0 + + with self.assert_statement_count(testing.db, expected_count): + if persistence.keep or persistence.reload: + is_(n2.parent, n1) + else: + eq_(n2.parent, Node(data="n1")) + + n1 = n2.parent + + # ensure n1.parent_id is unexpired + n1.parent_id + + if persistence.expunge and (join_depth is None or join_depth < 2): + expected_count = 1 + else: + expected_count = 0 + + with self.assert_statement_count(testing.db, expected_count): + if persistence.keep or persistence.reload: + is_(n1.parent, n0) + else: + eq_(n1.parent, Node(data="n0"))