summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2021-10-22 12:31:06 -0400
committerMike Bayer <mike_mp@zzzcomputing.com>2021-10-22 12:36:42 -0400
commit0fff45036d2629caf41c771088866beb61e5b284 (patch)
treeebb376315646196ec053198bef6ff2b834337712
parentee340d17ea45b25c063d7bfca999b7b8031ff0fe (diff)
downloadsqlalchemy-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.rst15
-rw-r--r--lib/sqlalchemy/orm/descriptor_props.py1
-rw-r--r--lib/sqlalchemy/orm/interfaces.py9
-rw-r--r--lib/sqlalchemy/orm/path_registry.py6
-rw-r--r--lib/sqlalchemy/orm/properties.py1
-rw-r--r--lib/sqlalchemy/orm/relationships.py2
-rw-r--r--test/orm/test_ac_relationships.py54
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