apply of_type error message to load.options() as well

Improved the error reporting when linking strategy options from a base
class to another attribute that's off a subclass, where ``of_type()``
should be used. Previously, when :meth:`.Load.options` is used, the message
would lack informative detail that ``of_type()`` should be used, which was
not the case when linking the options directly. The informative detail now
emits even if :meth:`.Load.options` is used.

Fixes: #9182
Change-Id: Ibc14923d0cbca9114316cb7db2b30f091dc28af8

diff --git a/doc/build/changelog/unreleased_20/9182.rst b/doc/build/changelog/unreleased_20/9182.rst
new file mode 100644 (file)
index 0000000..eb04827
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9182.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 9182
+
+    Improved the error reporting when linking strategy options from a base
+    class to another attribute that's off a subclass, where ``of_type()``
+    should be used. Previously, when :meth:`.Load.options` is used, the message
+    would lack informative detail that ``of_type()`` should be used, which was
+    not the case when linking the options directly. The informative detail now
+    emits even if :meth:`.Load.options` is used.
+
+

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 47a7a61228f43bbe0bc77dadda722d7588746dd2..60f07b83cc72bc18ff18188100b31de9bdf18e6f 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1171,10 +1171,13 @@ class Load(_AbstractLoad):
             cast("_InternalEntityType[Any]", parent.path[-1]),
             cast("_InternalEntityType[Any]", cloned.path[0]),
         ):
-            raise sa_exc.ArgumentError(
-                f'Attribute "{cloned.path[1]}" does not link '
-                f'from element "{parent.path[-1]}".'
-            )
+            if len(cloned.path) > 1:
+                attrname = cloned.path[1]
+                parent_entity = cloned.path[0]
+            else:
+                attrname = cloned.path[0]
+                parent_entity = cloned.path[0]
+            _raise_for_does_not_link(parent.path, attrname, parent_entity)
 
         cloned.path = PathRegistry.coerce(parent.path[0:-1] + cloned.path[:])
 
@@ -1911,33 +1914,8 @@ class _AttributeStrategyLoad(_LoadElement):
                         "loader option to multiple entities in the "
                         "same option. Use separate options per entity."
                     )
-                elif len(path) > 1:
-                    path_is_of_type = (
-                        path[-1].entity is not path[-2].mapper.class_
-                    )
-                    raise sa_exc.ArgumentError(
-                        f'ORM mapped attribute "{attr}" does not '
-                        f'link from relationship "{path[-2]}%s".%s'
-                        % (
-                            f".of_type({path[-1]})" if path_is_of_type else "",
-                            (
-                                "  Did you mean to use "
-                                f'"{path[-2]}'
-                                f'.of_type({attr.class_.__name__})"?'
-                                if not path_is_of_type
-                                and not path[-1].is_aliased_class
-                                and orm_util._entity_corresponds_to(
-                                    path.entity, attr.parent.mapper
-                                )
-                                else ""
-                            ),
-                        )
-                    )
                 else:
-                    raise sa_exc.ArgumentError(
-                        f'ORM mapped attribute "{attr}" does not '
-                        f'link mapped class "{path[-1]}"'
-                    )
+                    _raise_for_does_not_link(path, str(attr), attr.parent)
             else:
                 return None
 
@@ -2500,3 +2478,36 @@ def selectin_polymorphic(
 ) -> _AbstractLoad:
     ul = Load(base_cls)
     return ul.selectin_polymorphic(classes)
+
+
+def _raise_for_does_not_link(path, attrname, parent_entity):
+    if len(path) > 1:
+        path_is_of_type = path[-1].entity is not path[-2].mapper.class_
+        if insp_is_aliased_class(parent_entity):
+            parent_entity_str = str(parent_entity)
+        else:
+            parent_entity_str = parent_entity.class_.__name__
+
+        raise sa_exc.ArgumentError(
+            f'ORM mapped entity or attribute "{attrname}" does not '
+            f'link from relationship "{path[-2]}%s".%s'
+            % (
+                f".of_type({path[-1]})" if path_is_of_type else "",
+                (
+                    "  Did you mean to use "
+                    f'"{path[-2]}'
+                    f'.of_type({parent_entity_str})"?'
+                    if not path_is_of_type
+                    and not path[-1].is_aliased_class
+                    and orm_util._entity_corresponds_to(
+                        path.entity, inspect(parent_entity).mapper
+                    )
+                    else ""
+                ),
+            )
+        )
+    else:
+        raise sa_exc.ArgumentError(
+            f'ORM mapped attribute "{attrname}" does not '
+            f'link mapped class "{path[-1]}"'
+        )

diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py
index cdd1e02c701644412a1115e10e07ee0f0a2cad66..6d168d2cee8f795539b2eabd3e9fe23d0f0106a0 100644 (file)
--- a/test/orm/inheritance/test_relationship.py
+++ b/test/orm/inheritance/test_relationship.py
@@ -2334,8 +2334,8 @@ class JoinedloadOverWPolyAliased(
 
             with expect_raises_message(
                 exc.ArgumentError,
-                r'ORM mapped attribute "Sub1.links" does not link from '
-                r'relationship "Link.child".  Did you mean to use '
+                r'ORM mapped entity or attribute "Sub1.links" does not '
+                r'link from relationship "Link.child".  Did you mean to use '
                 r'"Link.child.of_type\(Sub1\)"\?',
             ):
                 session.query(cls).options(

diff --git a/test/orm/test_ac_relationships.py b/test/orm/test_ac_relationships.py
index 702b3e15f6086617995ad2245b204be20337050a..41fe972894b0cfcc44bd403aff0e5cf64dcd753f 100644 (file)
--- a/test/orm/test_ac_relationships.py
+++ b/test/orm/test_ac_relationships.py
@@ -182,7 +182,7 @@ class AliasedClassRelationshipTest(
 
         with expect_raises_message(
             exc.ArgumentError,
-            r'ORM mapped attribute "B.cs" does not link from '
+            r'ORM mapped entity or attribute "B.cs" does not link from '
             r'relationship "A.partitioned_bs.of_type\(aliased\(B\)\)"',
         ):
             if use_of_type:

diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py
index 0f2bb2013078cd6ab748910ada8d87e01956ad79..5ce7475f2b41d9a4443fa118fbbe6a20fdafd531 100644 (file)
--- a/test/orm/test_deferred.py
+++ b/test/orm/test_deferred.py
@@ -2009,8 +2009,8 @@ class InheritanceTest(_Polymorphic):
 
         with expect_raises_message(
             sa.exc.ArgumentError,
-            'ORM mapped attribute "Manager.status" does not link from '
-            r'relationship "Company.employees.'
+            r'ORM mapped entity or attribute "Manager.status" does not link '
+            r'from relationship "Company.employees.'
             r'of_type\(with_polymorphic\(Person, \[Manager\]\)\)".',
         ):
             s.query(Company).options(

diff --git a/test/orm/test_options.py b/test/orm/test_options.py
index e06daa9a4f794715eb25965009fbe1193d47749d..1446565ac8a2a1bfadbdebe6c16e62b4ac8cd0dd 100644 (file)
--- a/test/orm/test_options.py
+++ b/test/orm/test_options.py
@@ -1010,7 +1010,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
         self._assert_eager_with_entity_exception(
             [User],
             lambda: (joinedload(User.addresses).joinedload(User.orders),),
-            r'ORM mapped attribute "User.orders" does not link '
+            r'ORM mapped entity or attribute "User.orders" does not link '
             r'from relationship "User.addresses"',
         )
 
@@ -1024,7 +1024,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
                     User.orders.of_type(Order)
                 ),
             ),
-            r'ORM mapped attribute "User.orders" does not link '
+            r'ORM mapped entity or attribute "User.orders" does not link '
             r'from relationship "User.addresses"',
         )
 
@@ -1151,7 +1151,8 @@ class OptionsNoPropTestInh(_Polymorphic):
         e1 = with_polymorphic(Person, [Engineer])
         assert_raises_message(
             sa.exc.ArgumentError,
-            r'ORM mapped attribute "Manager.manager_name" does not link from '
+            r'ORM mapped entity or attribute "Manager.manager_name" does '
+            r"not link from "
             r'relationship "Company.employees.'
             r'of_type\(with_polymorphic\(Person, \[Engineer\]\)\)".$',
             lambda: s.query(Company)
@@ -1168,7 +1169,8 @@ class OptionsNoPropTestInh(_Polymorphic):
 
         assert_raises_message(
             sa.exc.ArgumentError,
-            r'ORM mapped attribute "Manager.manager_name" does not link from '
+            r'ORM mapped entity or attribute "Manager.manager_name" does '
+            r"not link from "
             r'relationship "Company.employees.'
             r'of_type\(Mapper\[Engineer\(engineers\)\]\)".$',
             lambda: s.query(Company)
@@ -1187,7 +1189,8 @@ class OptionsNoPropTestInh(_Polymorphic):
         # that doesn't get mixed up here
         assert_raises_message(
             sa.exc.ArgumentError,
-            r'ORM mapped attribute "Manager.status" does not link from '
+            r'ORM mapped entity or attribute "Manager.status" does '
+            r"not link from "
             r'relationship "Company.employees.'
             r'of_type\(Mapper\[Engineer\(engineers\)\]\)".$',
             lambda: s.query(Company)
@@ -1206,7 +1209,8 @@ class OptionsNoPropTestInh(_Polymorphic):
 
         assert_raises_message(
             sa.exc.ArgumentError,
-            r'ORM mapped attribute "Manager.manager_name" does not link from '
+            r'ORM mapped entity or attribute "Manager.manager_name" does '
+            r"not link from "
             r'relationship "Company.employees.'
             r'of_type\(with_polymorphic\(Person, \[Manager\]\)\)".$',
             lambda: s.query(Company)
@@ -1218,30 +1222,28 @@ class OptionsNoPropTestInh(_Polymorphic):
             ._compile_state(),
         )
 
-    def test_missing_attr_is_missing_of_type_for_alias(self):
+    @testing.variation("use_options", [True, False])
+    def test_missing_attr_is_missing_of_type_for_subtype(self, use_options):
         s = fixture_session()
 
-        pa = aliased(Person)
-
-        assert_raises_message(
+        with expect_raises_message(
             sa.exc.ArgumentError,
-            r'ORM mapped attribute "aliased\(Person\).name" does not link '
-            r'from relationship "Company.employees".  Did you mean to use '
-            r'"Company.employees.of_type\(aliased\(Person\)\)\"?',
-            lambda: s.query(Company)
-            .options(joinedload(Company.employees).load_only(pa.name))
-            ._compile_state(),
-        )
+            r"ORM mapped entity or attribute "
+            r'(?:"Mapper\[Engineer\(engineers\)\]"|"Engineer.engineer_name") '
+            r'does not link from relationship "Company.employees".  Did you '
+            r'mean to use "Company.employees.of_type\(Engineer\)"\?',
+        ):
 
-        q = s.query(Company).options(
-            joinedload(Company.employees.of_type(pa)).load_only(pa.name)
-        )
-        orig_path = inspect(Company)._path_registry[
-            Company.employees.property
-        ][inspect(pa)][pa.name.property]
-        key = ("loader", orig_path.natural_path)
-        loader = q._compile_state().attributes[key]
-        eq_(loader.path, orig_path)
+            if use_options:
+                s.query(Company).options(
+                    joinedload(Company.employees).options(
+                        defer(Engineer.engineer_name)
+                    )
+                )._compile_state()
+            else:
+                s.query(Company).options(
+                    joinedload(Company.employees).defer(Engineer.engineer_name)
+                )._compile_state()
 
 
 class PickleTest(fixtures.MappedTest):
@@ -1499,7 +1501,8 @@ class SubOptionsTest(PathTest, QueryTest):
 
         with expect_raises_message(
             sa.exc.ArgumentError,
-            r'ORM mapped attribute "Item.keywords" does not link from '
+            r'ORM mapped entity or attribute "Item.keywords" does '
+            r"not link from "
             r'relationship "User.orders"',
         ):
             [
@@ -1508,8 +1511,9 @@ class SubOptionsTest(PathTest, QueryTest):
             ]
         with expect_raises_message(
             sa.exc.ArgumentError,
-            r'Attribute "Item.keywords" does not link from '
-            r'element "Mapper\[Order\(orders\)\]"',
+            r'ORM mapped entity or attribute "Item.keywords" does '
+            r"not link from "
+            r'relationship "User.orders"',
         ):
             joinedload(User.orders).options(
                 joinedload(Item.keywords), joinedload(Order.items)