From: Mike Bayer Date: Fri, 22 Oct 2021 16:31:06 +0000 (-0400) Subject: use correct entity in path for aliased class relationship X-Git-Tag: rel_1_4_27~40^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0fff45036d2629caf41c771088866beb61e5b284;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..3f10a60883 --- /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 822a0a5a37..535067d88d 100644 --- 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 4826354dc2..903672c806 100644 --- 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 47bec83c94..6bebbd006e 100644 --- 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 1a1806c9e9..fa230d1093 100644 --- 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 bc50985803..3916c0a834 100644 --- 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 40f099fc84..26dd674800 100644 --- 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