diff options
| author | Mike Bayer <mike_mp@zzzcomputing.com> | 2021-10-22 12:31:06 -0400 |
|---|---|---|
| committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2021-10-22 12:36:42 -0400 |
| commit | 0fff45036d2629caf41c771088866beb61e5b284 (patch) | |
| tree | ebb376315646196ec053198bef6ff2b834337712 | |
| parent | ee340d17ea45b25c063d7bfca999b7b8031ff0fe (diff) | |
| download | sqlalchemy-0fff45036d2629caf41c771088866beb61e5b284.tar.gz | |
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
| -rw-r--r-- | doc/build/changelog/unreleased_14/7224.rst | 15 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/descriptor_props.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/interfaces.py | 9 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/path_registry.py | 6 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/properties.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/relationships.py | 2 | ||||
| -rw-r--r-- | test/orm/test_ac_relationships.py | 54 |
7 files changed, 80 insertions, 8 deletions
diff --git a/doc/build/changelog/unreleased_14/7224.rst b/doc/build/changelog/unreleased_14/7224.rst new file mode 100644 index 000000000..3f10a6088 --- /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 822a0a5a3..535067d88 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 4826354dc..903672c80 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 47bec83c9..6bebbd006 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 1a1806c9e..fa230d109 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 bc5098580..3916c0a83 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 40f099fc8..26dd67480 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 |
