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:24:32 -0400 |
commit | e79ab08165e01dc7af50fcffadb31468ace51b6c (patch) | |
tree | 43961bb056e9f0b0c7069bf3e5dea533dccd2c8f | |
parent | 0108d1769830c9e4e95f2250d2f382476ee90de6 (diff) | |
download | sqlalchemy-e79ab08165e01dc7af50fcffadb31468ace51b6c.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
-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 b117f59f7..e7084fbf6 100644 --- a/lib/sqlalchemy/orm/path_registry.py +++ b/lib/sqlalchemy/orm/path_registry.py @@ -231,7 +231,7 @@ class PathRegistry(HasCacheKey): def contains_mapper(self, mapper: Mapper[Any]) -> bool: _m_path = cast(_OddPathRepresentation, self.path) for path_mapper in [_m_path[i] for i in range(0, len(_m_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 41fe97289..a5efd9930 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 @@ -332,3 +333,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 |