expand "is_unnatural" to include all mapper inheritance paths

Fixed issue in ORM loader strategy logic which further allows for long
chains of :func:`_orm.contains_eager` loader options across complex
inheriting polymorphic / aliased / of_type() relationship chains to take
proper effect in queries.

Fixes: #10006
Change-Id: If803a7709ba8fd341fedb3b9282e0930c95fb5cd

diff --git a/doc/build/changelog/unreleased_20/10006.rst b/doc/build/changelog/unreleased_20/10006.rst
new file mode 100644 (file)
index 0000000..8d62626
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/10006.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 10006
+
+    Fixed issue in ORM loader strategy logic which further allows for long
+    chains of :func:`_orm.contains_eager` loader options across complex
+    inheriting polymorphic / aliased / of_type() relationship chains to take
+    proper effect in queries.

diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py
index e1cbd9313ccc69cd7bcb37a13a5a5bec05cb84c3..2269fb26b51727e782dc279ff60b1b5f2c0e4920 100644 (file)
--- a/lib/sqlalchemy/orm/path_registry.py
+++ b/lib/sqlalchemy/orm/path_registry.py
@@ -514,10 +514,9 @@ class PropRegistry(PathRegistry):
         natural_parent: AbstractEntityRegistry = parent
 
         # inherit "is_unnatural" from the parent
-        if parent.parent.is_unnatural:
-            self.is_unnatural = True
-        else:
-            self.is_unnatural = False
+        self.is_unnatural = parent.parent.is_unnatural or bool(
+            parent.mapper.inherits
+        )
 
         if not insp.is_aliased_class or insp._use_mapper_path:  # type: ignore
             parent = natural_parent = parent.parent[prop.parent]
@@ -665,10 +664,13 @@ class AbstractEntityRegistry(CreatesToken):
         # are to avoid the more expensive conditional logic that follows if we
         # know we don't have to do it.   This conditional can just as well be
         # "if parent.path:", it just is more function calls.
+        #
+        # This is basically the only place that the "is_unnatural" flag
+        # actually changes behavior.
         if parent.path and (self.is_aliased_class or parent.is_unnatural):
             # this is an infrequent code path used only for loader strategies
             # that also make use of of_type().
-            if entity.mapper.isa(parent.natural_path[-1].entity):  # type: ignore # noqa: E501
+            if entity.mapper.isa(parent.natural_path[-1].mapper):  # type: ignore # noqa: E501
                 self.natural_path = parent.natural_path + (entity.mapper,)
             else:
                 self.natural_path = parent.natural_path + (

diff --git a/test/orm/inheritance/test_assorted_poly.py b/test/orm/inheritance/test_assorted_poly.py
index 60322295283ccc24ac1ac5a23e71db99ca374963..41c5cce0e9bd34cb2cf111c5cf15f789e4de8f79 100644 (file)
--- a/test/orm/inheritance/test_assorted_poly.py
+++ b/test/orm/inheritance/test_assorted_poly.py
@@ -2928,3 +2928,223 @@ class AdaptExistsSubqTest(fixtures.DeclarativeMappedTest):
         )
 
         eq_(len(retrieved.children), 1)
+
+
+@testing.combinations(
+    ("single",),
+    ("joined",),
+    id_="s",
+    argnames="inheritance_type",
+)
+class MultiOfTypeContainsEagerTest(fixtures.DeclarativeMappedTest):
+    """test for #10006"""
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        employee_m2m = Table(
+            "employee_m2m",
+            Base.metadata,
+            Column(
+                "left", Integer, ForeignKey("employee.id"), primary_key=True
+            ),
+            Column(
+                "right", Integer, ForeignKey("employee.id"), primary_key=True
+            ),
+        )
+
+        class Property(ComparableEntity, Base):
+            __tablename__ = "property"
+            id: Mapped[int] = mapped_column(primary_key=True)
+            value: Mapped[str] = mapped_column(name="value")
+            user_id: Mapped[int] = mapped_column(ForeignKey("employee.id"))
+
+        class Employee(ComparableEntity, Base):
+            __tablename__ = "employee"
+            id: Mapped[int] = mapped_column(primary_key=True)
+            name: Mapped[str]
+            type: Mapped[str]
+            prop1 = relationship(Property, lazy="raise", uselist=False)
+
+            colleagues = relationship(
+                "Employee",
+                secondary=employee_m2m,
+                primaryjoin=lambda: Employee.id == employee_m2m.c.left,
+                secondaryjoin=lambda: Employee.id == employee_m2m.c.right,
+                lazy="raise",
+                collection_class=set,
+            )
+
+            __mapper_args__ = {
+                "polymorphic_on": "type",
+                "polymorphic_identity": "employee",
+            }
+
+        class Manager(Employee):
+            if cls.inheritance_type == "joined":
+                __tablename__ = "manager"
+                id: Mapped[int] = mapped_column(  # noqa: A001
+                    ForeignKey("employee.id"), primary_key=True
+                )
+            __mapper_args__ = {"polymorphic_identity": "manager"}
+
+        class Engineer(Employee):
+            if cls.inheritance_type == "joined":
+                __tablename__ = "engineer"
+                id: Mapped[int] = mapped_column(  # noqa: A001
+                    ForeignKey("employee.id"), primary_key=True
+                )
+            __mapper_args__ = {"polymorphic_identity": "engineer"}
+
+        class Clerk(Employee):
+            if cls.inheritance_type == "joined":
+                __tablename__ = "clerk"
+                id: Mapped[int] = mapped_column(  # noqa: A001
+                    ForeignKey("employee.id"), primary_key=True
+                )
+            __mapper_args__ = {"polymorphic_identity": "clerk"}
+
+        class UnitHead(Employee):
+            if cls.inheritance_type == "joined":
+                __tablename__ = "unithead"
+                id: Mapped[int] = mapped_column(  # noqa: A001
+                    ForeignKey("employee.id"), primary_key=True
+                )
+            managers = relationship(
+                "Manager",
+                secondary=employee_m2m,
+                primaryjoin=lambda: Employee.id == employee_m2m.c.left,
+                secondaryjoin=lambda: (
+                    and_(
+                        Employee.id == employee_m2m.c.right,
+                        Employee.type == "manager",
+                    )
+                ),
+                viewonly=True,
+                lazy="raise",
+                collection_class=set,
+            )
+            __mapper_args__ = {"polymorphic_identity": "unithead"}
+
+    @classmethod
+    def insert_data(cls, connection):
+        UnitHead, Manager, Engineer, Clerk, Property = cls.classes(
+            "UnitHead", "Manager", "Engineer", "Clerk", "Property"
+        )
+
+        with Session(connection) as sess:
+            unithead = UnitHead(
+                type="unithead",
+                name="unithead1",
+                prop1=Property(value="val unithead"),
+            )
+            manager = Manager(
+                type="manager",
+                name="manager1",
+                prop1=Property(value="val manager"),
+            )
+            other_manager = Manager(
+                type="manager",
+                name="manager2",
+                prop1=Property(value="val other manager"),
+            )
+            engineer = Engineer(
+                type="engineer",
+                name="engineer1",
+                prop1=Property(value="val engineer"),
+            )
+            clerk = Clerk(
+                type="clerk", name="clerk1", prop1=Property(value="val clerk")
+            )
+            unithead.colleagues.update([manager, other_manager])
+            manager.colleagues.update([engineer, clerk])
+            sess.add_all([unithead, manager, other_manager, engineer, clerk])
+            sess.commit()
+
+    @testing.variation("query_type", ["joinedload", "contains_eager"])
+    @testing.variation("use_criteria", [True, False])
+    def test_big_query(self, query_type, use_criteria):
+        Employee, UnitHead, Manager, Engineer, Clerk, Property = self.classes(
+            "Employee", "UnitHead", "Manager", "Engineer", "Clerk", "Property"
+        )
+
+        if query_type.contains_eager:
+            mgr = aliased(Manager)
+            clg = aliased(Employee)
+            clgs_prop1 = aliased(Property, name="clgs_prop1")
+
+            query = (
+                select(UnitHead)
+                .options(
+                    contains_eager(UnitHead.managers.of_type(mgr))
+                    .contains_eager(mgr.colleagues.of_type(clg))
+                    .contains_eager(clg.prop1.of_type(clgs_prop1)),
+                )
+                .outerjoin(UnitHead.managers.of_type(mgr))
+                .outerjoin(mgr.colleagues.of_type(clg))
+                .outerjoin(clg.prop1.of_type(clgs_prop1))
+            )
+            if use_criteria:
+                ma_prop1 = aliased(Property)
+                uhead_prop1 = aliased(Property)
+                query = (
+                    query.outerjoin(UnitHead.prop1.of_type(uhead_prop1))
+                    .outerjoin(mgr.prop1.of_type(ma_prop1))
+                    .where(
+                        uhead_prop1.value == "val unithead",
+                        ma_prop1.value == "val manager",
+                        clgs_prop1.value == "val engineer",
+                    )
+                )
+        elif query_type.joinedload:
+            if use_criteria:
+                query = (
+                    select(UnitHead)
+                    .options(
+                        joinedload(
+                            UnitHead.managers.and_(
+                                Manager.prop1.has(value="val manager")
+                            )
+                        )
+                        .joinedload(
+                            Manager.colleagues.and_(
+                                Employee.prop1.has(value="val engineer")
+                            )
+                        )
+                        .joinedload(Employee.prop1),
+                    )
+                    .where(UnitHead.prop1.has(value="val unithead"))
+                )
+            else:
+                query = select(UnitHead).options(
+                    joinedload(UnitHead.managers)
+                    .joinedload(Manager.colleagues)
+                    .joinedload(Employee.prop1),
+                )
+
+        session = fixture_session()
+        head = session.scalars(query).unique().one()
+
+        if use_criteria:
+            expected_managers = {
+                Manager(
+                    name="manager1",
+                    colleagues={Engineer(name="engineer1", prop1=Property())},
+                )
+            }
+        else:
+            expected_managers = {
+                Manager(
+                    name="manager1",
+                    colleagues={
+                        Engineer(name="engineer1", prop1=Property()),
+                        Clerk(name="clerk1"),
+                    },
+                ),
+                Manager(name="manager2"),
+            }
+        eq_(
+            head,
+            UnitHead(managers=expected_managers),
+        )