diff options
| -rw-r--r-- | doc/build/changelog/unreleased_13/4566.rst | 10 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/strategy_options.py | 20 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/util.py | 6 | ||||
| -rw-r--r-- | test/orm/test_options.py | 32 |
4 files changed, 60 insertions, 8 deletions
diff --git a/doc/build/changelog/unreleased_13/4566.rst b/doc/build/changelog/unreleased_13/4566.rst new file mode 100644 index 000000000..3fb1a49e8 --- /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 8a34771c7..912b6b550 100644 --- 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 f9258895d..7b8edd349 100644 --- 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 e382a8097..8597305dd 100644 --- 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): |