use correct entity in path for aliased class relationship

Fixed bug in "relationship to aliased class" feature introduced at
:ref:`relationship_aliased_class` where it was not possible to create a
loader strategy option targeting an attribute on the target using the
:func:`_orm.aliased` construct directly in a second loader option, such as
``selectinload(A.aliased_bs).joinedload(aliased_b.cs)``, without explicitly
qualifying using :meth:`_orm.PropComparator.of_type` on the preceding
element of the path. Additionally, targeting the non-aliased class directly
would be accepted (inappropriately), but would silently fail, such as
``selectinload(A.aliased_bs).joinedload(B.cs)``; this now raises an error
referring to the typing mismatch.

Fixes: #7224
Change-Id: I40857c7275667dcb64f1d1fd0c8072e48758e678

diff --git a/doc/build/changelog/unreleased_14/7224.rst b/doc/build/changelog/unreleased_14/7224.rst
new file mode 100644 (file)
index 0000000..3f10a60
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/7224.rst
@@ -0,0 +1,15 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 7224
+
+    Fixed bug in "relationship to aliased class" feature introduced at
+    :ref:`relationship_aliased_class` where it was not possible to create a
+    loader strategy option targeting an attribute on the target using the
+    :func:`_orm.aliased` construct directly in a second loader option, such as
+    ``selectinload(A.aliased_bs).joinedload(aliased_b.cs)``, without explicitly
+    qualifying using :meth:`_orm.PropComparator.of_type` on the preceding
+    element of the path. Additionally, targeting the non-aliased class directly
+    would be accepted (inappropriately), but would silently fail, such as
+    ``selectinload(A.aliased_bs).joinedload(B.cs)``; this now raises an error
+    referring to the typing mismatch.
+

diff --git a/lib/sqlalchemy/orm/descriptor_props.py b/lib/sqlalchemy/orm/descriptor_props.py
index 822a0a5a3765973b6aec010f8895c3822626141c..535067d88d0d300bc161cae2489df32c7f3a6c35 100644 (file)
--- a/lib/sqlalchemy/orm/descriptor_props.py
+++ b/lib/sqlalchemy/orm/descriptor_props.py
@@ -32,6 +32,7 @@ class DescriptorProperty(MapperProperty):
     doc = None
 
     uses_objects = False
+    _links_to_entity = False
 
     def instrument_class(self, mapper):
         prop = self

diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 4826354dc2c905812dd284b9f9d35be9c4075de2..903672c80621a5535c8ed6af034e2e2230241321 100644 (file)
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -120,6 +120,15 @@ class MapperProperty(
 
     """
 
+    @property
+    def _links_to_entity(self):
+        """True if this MapperProperty refers to a mapped entity.
+
+        Should only be True for RelationshipProperty, False for all others.
+
+        """
+        raise NotImplementedError()
+
     def _memoized_attr_info(self):
         """Info dictionary associated with the object, allowing user-defined
         data to be associated with this :class:`.InspectionAttr`.

diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py
index 47bec83c9451979e8ac608b8b457b2ae83135b12..6bebbd006e19fe79a66662569f4619f697579c94 100644 (file)
--- a/lib/sqlalchemy/orm/path_registry.py
+++ b/lib/sqlalchemy/orm/path_registry.py
@@ -398,15 +398,15 @@ class PropRegistry(PathRegistry):
 
     @util.memoized_property
     def has_entity(self):
-        return hasattr(self.prop, "mapper")
+        return self.prop._links_to_entity
 
     @util.memoized_property
     def entity(self):
-        return self.prop.mapper
+        return self.prop.entity
 
     @property
     def mapper(self):
-        return self.entity
+        return self.prop.mapper
 
     @property
     def entity_path(self):

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 1a1806c9e9cf065d1288ec4aabef6d0c0d2aeeae..fa230d1093088c1e4faa0be0466e994fe02f1d7e 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -46,6 +46,7 @@ class ColumnProperty(StrategizedProperty):
 
     strategy_wildcard_key = "column"
     inherit_cache = True
+    _links_to_entity = False
 
     __slots__ = (
         "_orig_columns",

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index bc509858036333c8f21cad444f129b8f62135b2e..3916c0a834156de7c0620381b955554fb29e9822 100644 (file)
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -105,6 +105,8 @@ class RelationshipProperty(StrategizedProperty):
     strategy_wildcard_key = "relationship"
     inherit_cache = True
 
+    _links_to_entity = True
+
     _persistence_only = dict(
         passive_deletes=False,
         passive_updates=True,

diff --git a/test/orm/test_ac_relationships.py b/test/orm/test_ac_relationships.py
index 40f099fc84e1e5d32d82d8ffcdb729b97fef4f4c..26dd674800f9ff281089741ce80b11ae48cb7799 100644 (file)
--- a/test/orm/test_ac_relationships.py
+++ b/test/orm/test_ac_relationships.py
@@ -1,5 +1,6 @@
 from sqlalchemy import and_
 from sqlalchemy import Column
+from sqlalchemy import exc
 from sqlalchemy import ForeignKey
 from sqlalchemy import func
 from sqlalchemy import Integer
@@ -14,6 +15,7 @@ from sqlalchemy.orm import selectinload
 from sqlalchemy.orm import Session
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing.assertions import expect_raises_message
 from sqlalchemy.testing.assertsql import CompiledSQL
 from sqlalchemy.testing.fixtures import ComparableEntity
 from sqlalchemy.testing.fixtures import fixture_session
@@ -47,7 +49,7 @@ class PartitionByFixture(fixtures.DeclarativeMappedTest):
             .label("index"),
         ).alias()
 
-        partitioned_b = aliased(B, alias=partition)
+        partitioned_b = cls.partitioned_b = aliased(B, alias=partition)
 
         A.partitioned_bs = relationship(
             partitioned_b,
@@ -139,20 +141,62 @@ class AliasedClassRelationshipTest(
 
         self.assert_sql_count(testing.db, go, 2)
 
-    def test_selectinload_w_joinedload_after(self):
+    @testing.combinations("string", "ac_attribute", "ac_attr_w_of_type")
+    def test_selectinload_w_joinedload_after(self, calling_style):
+        """test has been enhanced to also test #7224"""
+
         A, B, C = self.classes("A", "B", "C")
 
         s = Session(testing.db)
 
+        partitioned_b = self.partitioned_b
+
+        if calling_style == "string":
+            opt = selectinload(A.partitioned_bs).joinedload("cs")
+        elif calling_style == "ac_attribute":
+            opt = selectinload(A.partitioned_bs).joinedload(partitioned_b.cs)
+        elif calling_style == "ac_attr_w_of_type":
+            # this would have been a workaround for people who encountered
+            # #7224. The exception that was raised for "ac_attribute" actually
+            # suggested to use of_type() so we can assume this pattern is
+            # probably being used
+            opt = selectinload(
+                A.partitioned_bs.of_type(partitioned_b)
+            ).joinedload(partitioned_b.cs)
+        else:
+            assert False
+
         def go():
-            for a1 in s.query(A).options(
-                selectinload(A.partitioned_bs).joinedload("cs")
-            ):
+            for a1 in s.query(A).options(opt):
                 for b in a1.partitioned_bs:
                     eq_(len(b.cs), 2)
 
         self.assert_sql_count(testing.db, go, 2)
 
+    @testing.combinations(True, False)
+    def test_selectinload_w_joinedload_after_base_target_fails(
+        self, use_of_type
+    ):
+        A, B, C = self.classes("A", "B", "C")
+
+        s = Session(testing.db)
+        partitioned_b = self.partitioned_b
+
+        if use_of_type:
+            opt = selectinload(
+                A.partitioned_bs.of_type(partitioned_b)
+            ).joinedload(B.cs)
+        else:
+            opt = selectinload(A.partitioned_bs).joinedload(B.cs)
+
+        q = s.query(A).options(opt)
+
+        with expect_raises_message(
+            exc.ArgumentError,
+            r'Attribute "B.cs" does not link from element "aliased\(B\)"',
+        ):
+            q._compile_context()
+
 
 class AltSelectableTest(
     fixtures.DeclarativeMappedTest, testing.AssertsCompiledSQL