]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
consider aliased mappers in cycles also
authorMike Bayer <mike_mp@zzzcomputing.com>
Sun, 2 Apr 2023 18:24:32 +0000 (14:24 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 2 Apr 2023 18:24:32 +0000 (14:24 -0400)
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

doc/build/changelog/unreleased_14/9590.rst [new file with mode: 0644]
lib/sqlalchemy/orm/path_registry.py
test/orm/test_ac_relationships.py

diff --git a/doc/build/changelog/unreleased_14/9590.rst b/doc/build/changelog/unreleased_14/9590.rst
new file mode 100644 (file)
index 0000000..472cfc7
--- /dev/null
@@ -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.
index b117f59f78ba56c0a3260620c183401fd892f57b..e7084fbf676442c937f30af1e58dc3489ade423c 100644 (file)
@@ -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
index 41fe972894b0cfcc44bd403aff0e5cf64dcd753f..a5efd9930d5ba6b3971463be5dd3692eecc2e426 100644 (file)
@@ -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