Fix boolean check in new path comparison logic

Fixed regression where a new error message that was supposed to raise when
attempting to link a relationship option to an AliasedClass without using
:meth:`.PropComparator.of_type` would instead raise an ``AttributeError``.
Note that in 1.3, it is no longer valid to create an option path from a
plain mapper relationship to an :class:`.AliasedClass` without using
:meth:`.PropComparator.of_type`.

Fixes: #4566
Change-Id: Ic547a1c8408e41aec66ef9644aac7f76f50dd064

diff --git a/doc/build/changelog/unreleased_13/4566.rst b/doc/build/changelog/unreleased_13/4566.rst
new file mode 100644 (file)
index 0000000..3fb1a49
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4566.rst
@@ -0,0 +1,10 @@
+.. change::
+   :tags: bug, orm
+   :tickets: 4566
+
+   Fixed regression where a new error message that was supposed to raise when
+   attempting to link a relationship option to an AliasedClass without using
+   :meth:`.PropComparator.of_type` would instead raise an ``AttributeError``.
+   Note that in 1.3, it is no longer valid to create an option path from a
+   plain mapper relationship to an :class:`.AliasedClass` without using
+   :meth:`.PropComparator.of_type`.

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 8a34771c75a02451fff4282bf1b804fd82681660..912b6b5503d27afc250eef8e329bfab5f68c5570 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -227,7 +227,7 @@ class Load(Generative, MapperOption):
                 if raiseerr:
                     raise sa_exc.ArgumentError(
                         'Can\'t find property named "%s" on '
-                        "%s in this Query. " % (attr, ent)
+                        "%s in this Query." % (attr, ent)
                     )
                 else:
                     return None
@@ -236,6 +236,9 @@ class Load(Generative, MapperOption):
 
             path = path[attr]
         elif _is_mapped_class(attr):
+            # TODO: this does not appear to be a valid codepath.  "attr"
+            # would never be a mapper.  This block is present in 1.2
+            # as well howver does not seem to be accessed in any tests.
             if not orm_util._entity_corresponds_to_use_path_impl(
                 attr.parent, path[-1]
             ):
@@ -255,7 +258,20 @@ class Load(Generative, MapperOption):
                 if raiseerr:
                     raise sa_exc.ArgumentError(
                         'Attribute "%s" does not '
-                        'link from element "%s"' % (attr, path.entity)
+                        'link from element "%s".%s'
+                        % (
+                            attr,
+                            path.entity,
+                            (
+                                "  Did you mean to use "
+                                "%s.of_type(%s)?"
+                                % (path[-2], attr.class_.__name__)
+                                if len(path) > 1
+                                and path.entity.is_mapper
+                                and attr.parent.is_aliased_class
+                                else ""
+                            ),
+                        )
                     )
                 else:
                     return None

diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index f9258895d3e00786fdff239942d4fac84708cf33..7b8edd34933a4106d77b09ea605ca97037fe79d6 100644 (file)
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -1265,8 +1265,10 @@ def _entity_corresponds_to_use_path_impl(given, entity):
         return (
             entity.is_aliased_class
             and not entity._use_mapper_path
-            and given is entity
-            or given in entity._with_polymorphic_entities
+            and (
+                given is entity
+                or given in entity._with_polymorphic_entities
+            )
         )
     elif not entity.is_aliased_class:
         return given.common_parent(entity.mapper)

diff --git a/test/orm/test_options.py b/test/orm/test_options.py
index e382a80971972fec769fb2d5476302265fb4633d..8597305dde4472ccc46da3579d5d536c947486a6 100644 (file)
--- a/test/orm/test_options.py
+++ b/test/orm/test_options.py
@@ -1266,7 +1266,7 @@ class OptionsNoPropTestInh(_Polymorphic):
         assert_raises_message(
             sa.exc.ArgumentError,
             r'Attribute "Manager.manager_name" does not link from element '
-            r'"with_polymorphic\(Person, \[Engineer\]\)"',
+            r'"with_polymorphic\(Person, \[Engineer\]\)".$',
             s.query(Company).options,
             joinedload(Company.employees.of_type(Engineer)).load_only(
                 Manager.manager_name
@@ -1281,7 +1281,7 @@ class OptionsNoPropTestInh(_Polymorphic):
         assert_raises_message(
             sa.exc.ArgumentError,
             r'Attribute "Manager.status" does not link from element '
-            r'"with_polymorphic\(Person, \[Engineer\]\)"',
+            r'"with_polymorphic\(Person, \[Engineer\]\)".$',
             s.query(Company).options,
             joinedload(Company.employees.of_type(Engineer)).load_only(
                 Manager.status
@@ -1296,7 +1296,7 @@ class OptionsNoPropTestInh(_Polymorphic):
         assert_raises_message(
             sa.exc.ArgumentError,
             r'Can\'t find property named "manager_name" on '
-            "mapped class Engineer->engineers in this Query.",
+            r"mapped class Engineer->engineers in this Query.$",
             s.query(Company).options,
             joinedload(Company.employees.of_type(Engineer)).load_only(
                 "manager_name"
@@ -1311,13 +1311,37 @@ class OptionsNoPropTestInh(_Polymorphic):
         assert_raises_message(
             sa.exc.ArgumentError,
             r'Attribute "Manager.manager_name" does not link from '
-            r'element "with_polymorphic\(Person, \[Manager\]\)"',
+            r'element "with_polymorphic\(Person, \[Manager\]\)".$',
             s.query(Company).options,
             joinedload(Company.employees.of_type(wp)).load_only(
                 Manager.manager_name
             ),
         )
 
+    def test_missing_attr_is_missing_of_type_for_alias(self):
+        s = Session()
+
+        pa = aliased(Person)
+
+        assert_raises_message(
+            sa.exc.ArgumentError,
+            r'Attribute "AliasedClass_Person.name" does not link from '
+            r'element "mapped class Person->people".  Did you mean to use '
+            r"Company.employees.of_type\(AliasedClass_Person\)\?",
+            s.query(Company).options,
+            joinedload(Company.employees).load_only(pa.name),
+        )
+
+        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._attributes[key]
+        eq_(loader.path, orig_path)
+
 
 class PickleTest(PathTest, QueryTest):
     def _option_fixture(self, *arg):