From: Mike Bayer Date: Mon, 30 Jan 2023 00:51:39 +0000 (-0500) Subject: apply of_type error message to load.options() as well X-Git-Tag: rel_2_0_1~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=26c014e72d9ded83fc90d1577fb7b5c3ef3977f8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..eb0482748d --- /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 47a7a61228..60f07b83cc 100644 --- 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 cdd1e02c70..6d168d2cee 100644 --- 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 702b3e15f6..41fe972894 100644 --- 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 0f2bb20130..5ce7475f2b 100644 --- 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 e06daa9a4f..1446565ac8 100644 --- 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)