summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2023-04-02 14:24:32 -0400
committerMike Bayer <mike_mp@zzzcomputing.com>2023-04-02 14:24:32 -0400
commite79ab08165e01dc7af50fcffadb31468ace51b6c (patch)
tree43961bb056e9f0b0c7069bf3e5dea533dccd2c8f
parent0108d1769830c9e4e95f2250d2f382476ee90de6 (diff)
downloadsqlalchemy-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.rst10
-rw-r--r--lib/sqlalchemy/orm/path_registry.py2
-rw-r--r--test/orm/test_ac_relationships.py63
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