diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2023-04-02 14:24:32 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2023-04-02 14:27:58 -0400 |
commit | 1aef8e75a69319469d3b447422b8cdee2a1cf894 (patch) | |
tree | 3442ffe10828ac12c99ab766b96fbe9e4b19e128 | |
parent | 1a7f56a1bd2c583577159343327b04a49e8f57fb (diff) | |
download | sqlalchemy-1aef8e75a69319469d3b447422b8cdee2a1cf894.tar.gz |
consider aliased mappers in cycles also
Fixed endless loop which could occur when using "relationship to aliased
class" feature and also indicating a recursive eager loader such as
``lazy="selectinload"`` in the loader, in combination with another eager
loader on the opposite side. The check for cycles has been fixed to include
aliased class relationships.
Fixes: #9590
Change-Id: I8d340882f040ff9289c209bedd8fbdfd7186f944
(cherry picked from commit e79ab08165e01dc7af50fcffadb31468ace51b6c)
-rw-r--r-- | doc/build/changelog/unreleased_14/9590.rst | 10 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/path_registry.py | 2 | ||||
-rw-r--r-- | test/orm/test_ac_relationships.py | 63 |
3 files changed, 74 insertions, 1 deletions
diff --git a/doc/build/changelog/unreleased_14/9590.rst b/doc/build/changelog/unreleased_14/9590.rst new file mode 100644 index 000000000..472cfc70e --- /dev/null +++ b/doc/build/changelog/unreleased_14/9590.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 9590 + :versions: 2.0.9 + + Fixed endless loop which could occur when using "relationship to aliased + class" feature and also indicating a recursive eager loader such as + ``lazy="selectinload"`` in the loader, in combination with another eager + loader on the opposite side. The check for cycles has been fixed to include + aliased class relationships. diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py index 4deb96b1f..dad1fd46c 100644 --- a/lib/sqlalchemy/orm/path_registry.py +++ b/lib/sqlalchemy/orm/path_registry.py @@ -120,7 +120,7 @@ class PathRegistry(HasCacheKey): def contains_mapper(self, mapper): for path_mapper in [self.path[i] for i in range(0, len(self.path), 2)]: - if path_mapper.is_mapper and path_mapper.isa(mapper): + if path_mapper.mapper.isa(mapper): return True else: return False diff --git a/test/orm/test_ac_relationships.py b/test/orm/test_ac_relationships.py index f59d704f3..57e2b25e9 100644 --- a/test/orm/test_ac_relationships.py +++ b/test/orm/test_ac_relationships.py @@ -14,6 +14,7 @@ from sqlalchemy.orm import relationship from sqlalchemy.orm import selectinload from sqlalchemy.orm import Session from sqlalchemy.testing import eq_ +from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures from sqlalchemy.testing.assertions import expect_raises_message from sqlalchemy.testing.assertsql import CompiledSQL @@ -331,3 +332,65 @@ class AltSelectableTest( "FROM a JOIN (b JOIN d ON d.b_id = b.id " "JOIN c ON c.id = d.c_id) ON a.b_id = b.id", ) + + +class StructuralEagerLoadCycleTest(fixtures.DeclarativeMappedTest): + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class A(Base): + __tablename__ = "a" + id = Column(Integer, primary_key=True) + + bs = relationship(lambda: B, back_populates="a") + + class B(Base): + __tablename__ = "b" + id = Column(Integer, primary_key=True) + a_id = Column(ForeignKey("a.id")) + + a = relationship(A, lazy="joined", back_populates="bs") + + partitioned_b = aliased(B) + + A.partitioned_bs = relationship( + partitioned_b, lazy="selectin", viewonly=True + ) + + @classmethod + def insert_data(cls, connection): + A, B = cls.classes("A", "B") + + s = Session(connection) + a = A() + a.bs = [B() for _ in range(5)] + s.add(a) + + s.commit() + + @testing.variation("ensure_no_warning", [True, False]) + def test_no_endless_loop(self, ensure_no_warning): + """test #9590""" + + A = self.classes.A + + sess = fixture_session() + + results = sess.scalars(select(A)) + + # the correct behavior is 1. no warnings and 2. no endless loop. + # however when the failure mode is occurring, it correctly warns, + # but then we don't get to see the endless loop happen. + # so test it both ways even though when things are "working", there's + # no problem + if ensure_no_warning: + + a = results.first() + else: + with expect_warnings( + "Loader depth for query is excessively deep", assert_=False + ): + a = results.first() + + a.bs |