From: Jan Vollmer Date: Wed, 15 Jul 2026 12:56:19 +0000 (-0400) Subject: Improve ORM loader strategy error messages to use user-friendly representations X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ada9d7fcd18d10ff101bf2e3b57e32bb06301605;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Improve ORM loader strategy error messages to use user-friendly representations Improved error messages raised when ORM loader strategy options cannot be applied to a query. Messages now render the offending option in a user-friendly form such as ``joinedload(User.orders)`` rather than exposing internal class and path representations, and the "does not apply to root entities" message now includes the option that triggered the error. The same user-friendly rendering is also applied to the "conflicting loader strategy" message and to the ``of_type()`` representation in "does not link" messages. Originating pull request courtesy Jan Vollmer. Co-authored-by: Mike Bayer Fixes: #12398 Closes: #12401 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/12401 Pull-request-sha: 4de2e161924d2d61ee7ad8402315da754e7995f4 Change-Id: I41c4f7fd077bd7fdb6e6ac9675370a37f7e7d7b4 --- diff --git a/doc/build/changelog/unreleased_21/12398.rst b/doc/build/changelog/unreleased_21/12398.rst new file mode 100644 index 0000000000..6b92a9a904 --- /dev/null +++ b/doc/build/changelog/unreleased_21/12398.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: usecase, orm + :tickets: 12398 + + Improved error messages raised when ORM loader strategy options cannot be + applied to a query. Messages now render the offending option in a + user-friendly form such as ``joinedload(User.orders)`` rather than exposing + internal class and path representations, and the "does not apply to root + entities" message now includes the option that triggered the error. The + same user-friendly rendering is also applied to the "conflicting loader + strategy" message and to the ``of_type()`` representation in "does not + link" messages. Originating pull request courtesy Jan Vollmer. diff --git a/lib/sqlalchemy/orm/_typing.py b/lib/sqlalchemy/orm/_typing.py index ab9518beb4..2c9953345a 100644 --- a/lib/sqlalchemy/orm/_typing.py +++ b/lib/sqlalchemy/orm/_typing.py @@ -98,6 +98,10 @@ OrmExecuteOptionsParameter = Union[ ] +class _HasPathString(Protocol): + def path_string(self) -> str: ... + + class _ORMAdapterProto(Protocol): """protocol for the :class:`.AliasedInsp._orm_adapt_element` method which is a synonym for :class:`.AliasedInsp._adapt_element`. diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py index 5f4268f705..8a26e2686f 100644 --- a/lib/sqlalchemy/orm/base.py +++ b/lib/sqlalchemy/orm/base.py @@ -14,6 +14,7 @@ import operator import typing from typing import Any from typing import Callable +from typing import cast from typing import Dict from typing import Generic from typing import Literal @@ -27,6 +28,7 @@ from typing import TypeVar from typing import Union from . import exc +from ._typing import _HasPathString from ._typing import _O from ._typing import insp_is_mapper from .. import exc as sa_exc @@ -385,6 +387,19 @@ def state_attribute_str(state: InstanceState[Any], attribute: str) -> str: return state_str(state) + "." + attribute +def entity_str(entity: Any) -> str: + """Return a user-facing string for a mapped entity, such as a mapped + class, :class:`_orm.Mapper`, or :func:`_orm.aliased` construct. + + Also accepts a :class:`_orm.PathRegistry` directly, which is itself + ``inspect()``-able and implements ``path_string()``; in that case + this is equivalent to calling + :meth:`_orm.PathRegistry.path_string` directly. + + """ + return cast(_HasPathString, inspection.inspect(entity)).path_string() + + def object_mapper(instance: _T) -> Mapper[_T]: """Given an object, return the primary Mapper associated with the object instance. diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 9e8ec5dae2..5f638386cd 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -758,6 +758,13 @@ class MapperProperty( getattr(self, "key", "no key"), ) + def path_string(self) -> str: + """Return a user-facing name for this :class:`.MapperProperty`, + for use in a :class:`_orm.PathRegistry` string representation. + + """ + return f"{self.parent.class_.__name__}.{self.key}" + @inspection._self_inspects class PropComparator(SQLORMOperations[_T_co], Generic[_T_co], ColumnOperators): diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index faf0b45a32..58842eee05 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -2422,6 +2422,13 @@ class Mapper( ), ) + def path_string(self) -> str: + """Return a user-facing name for this :class:`_orm.Mapper`, + for use in a :class:`_orm.PathRegistry` string representation. + + """ + return self.class_.__name__ + def _is_orphan(self, state: InstanceState[_O]) -> bool: orphan_possible = False for mapper in self.iterate_to_root(): diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py index 855b58b3e4..8e30962d94 100644 --- a/lib/sqlalchemy/orm/path_registry.py +++ b/lib/sqlalchemy/orm/path_registry.py @@ -27,6 +27,7 @@ from typing import Union from . import base as orm_base from ._typing import insp_is_mapper_property from .. import exc +from .. import inspection from .. import util from ..sql import visitors from ..sql.cache_key import HasCacheKey @@ -86,6 +87,7 @@ _WILDCARD_TOKEN: _LiteralStar = "*" _DEFAULT_TOKEN = "_sa_default" +@inspection._self_inspects class PathRegistry(HasCacheKey): """Represent query load paths and registry functions. @@ -351,6 +353,39 @@ class PathRegistry(HasCacheKey): def __repr__(self) -> str: return f"{self.__class__.__name__}({self.path!r})" + def path_string(self) -> str: + """Return a user-facing string representation of this path, + e.g. ``"User.orders -> Order.items"``. + + """ + + raw = self.path + parts = [] + lraw = len(raw) + for i in range(0, lraw - 1, 2): + entity = raw[i] + prop = raw[i + 1] + prop_key = getattr(prop, "key", str(prop)) + + if ( + i < lraw - 2 + and cast( + "_InternalEntityType[Any]", raw[i + 2] + ).is_aliased_class + ): + parts.append( + f"{orm_base.entity_str(entity)}.{prop_key}." + f"of_type({orm_base.entity_str(raw[i + 2])})" + ) + else: + parts.append(f"{orm_base.entity_str(entity)}.{prop_key}") + + return ( + " -> ".join(parts) + if parts + else orm_base.entity_str(self.path[0]) if self.path else "" + ) + class _CreatesToken(PathRegistry): __slots__ = () @@ -539,6 +574,7 @@ class _PropRegistry(PathRegistry): prop: StrategizedProperty[Any] mapper: Optional[Mapper[Any]] entity: Optional[_InternalEntityType[Any]] + parent: _AbstractEntityRegistry def __init__( self, parent: _AbstractEntityRegistry, prop: StrategizedProperty[Any] diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index c9bd03b67a..7932388239 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -27,11 +27,11 @@ from typing import TypeVar from typing import Union from . import util as orm_util -from ._typing import insp_is_aliased_class from ._typing import insp_is_attribute from ._typing import insp_is_mapper from ._typing import insp_is_mapper_property from .attributes import QueryableAttribute +from .base import entity_str from .base import InspectionAttr from .interfaces import LoaderOption from .path_registry import _AbstractEntityRegistry @@ -84,6 +84,10 @@ _StrategySpec = Dict[str, Any] _OptsType = Dict[str, Any] _AttrGroupType = Tuple[_AttrType, ...] +# maps _StrategyKey tuples to the user-facing loader function name, +# populated by the @_strategy_labels() decorator below +_STRATEGY_FN_LABELS: Dict[Any, str] = {} + class _AbstractLoad(traversals.GenerativeOnTraversal, LoaderOption): __slots__ = ("propagate_to_loaders",) @@ -1019,7 +1023,7 @@ class Load(_AbstractLoad): self.additional_source_entities = () def __str__(self) -> str: - return f"Load({self.path[0]})" + return f"Load({entity_str(self.path[0])})" @classmethod def _construct_for_existing_path( @@ -1619,16 +1623,17 @@ class _LoadElement( if not found_entities: raise sa_exc.ArgumentError( "Query has only expression-based entities; " - f"attribute loader options for {path[0]} can't " - "be applied here." + f"attribute loader option {self._to_option_method_string()} " + "can't be applied here." ) else: raise sa_exc.ArgumentError( - f"Mapped class {path[0]} does not apply to any of the " - f"root entities in this query, e.g. " + f"Mapped class {entity_str(path[0])} referenced in " + f"option {self._to_option_method_string()} does not apply " + f"to any of the root entities in this query, e.g. " f"""{ ", ".join( - str(x.entity_zero) + entity_str(x.entity_zero) for x in mapper_entities if x.entity_zero )}. Please """ "specify the full path " @@ -1801,7 +1806,7 @@ class _LoadElement( ): raise sa_exc.ArgumentError( f'Attribute "{self.path[1]}" does not link ' - f'from element "{parent.path[-1]}".' + f'from element "{entity_str(parent.path[-1])}".' ) return self._prepend_path(parent.path) @@ -1871,9 +1876,34 @@ class _LoadElement( return replacement raise sa_exc.InvalidRequestError( - f"Loader strategies for {replacement.path} conflict" + f"Loader strategy replacement " + f"{replacement._to_option_method_string()} is in conflict " + f"with existing strategy {existing._to_option_method_string()}" ) + def _to_option_method_string(self) -> str: + """Return a string representation of this :class:`._LoadElement` + as it would be written as a loader option method call, e.g. + ``"joinedload(User.orders)"``. + + """ + assert ( + self.strategy is not None + ), "to_option_method_string() requires a strategy to be set" + + for opt_key in self.local_opts: + fn = _STRATEGY_FN_LABELS.get((self.strategy, opt_key)) + if fn: + break + else: + fn = _STRATEGY_FN_LABELS.get((self.strategy, None)) + + assert ( + fn is not None + ), f"No _STRATEGY_FN_LABELS entry for strategy {self.strategy!r}" + + return f"{fn}({self.path.path_string()})" + class _AttributeStrategyLoad(_LoadElement): """Loader strategies against specific relationship or column paths. @@ -2394,6 +2424,30 @@ def _parse_attr_argument( return insp, lead_entity, prop +def _strategy_labels( + *strategy_keys: "Any", + discriminating_opt: Optional[str] = None, +) -> Callable[[_FN], _FN]: + """Decorator that registers strategy key(s) -> function name in + ``_STRATEGY_FN_LABELS``. Apply below ``@loader_unbound_fn`` so that + ``fn.__name__`` is still the original function name when the decorator + runs. + + :param discriminating_opt: optional local_opts key that distinguishes + this function from another function with the same strategy key. + When set, the entry is stored under ``(strategy_key, opt_key)`` + instead of ``(strategy_key, None)``, and ``__str__`` will use this + function name when that opt is present in ``local_opts``. + """ + + def decorator(fn: _FN) -> _FN: + for key in strategy_keys: + _STRATEGY_FN_LABELS[(key, discriminating_opt)] = fn.__name__ + return fn + + return decorator + + def loader_unbound_fn(fn: _FN) -> _FN: """decorator that applies docstrings between standalone loader functions and the loader methods on :class:`._AbstractLoad`. @@ -2434,6 +2488,7 @@ def _expand_column_strategy_attrs( @loader_unbound_fn +@_strategy_labels((("lazy", "joined"),), discriminating_opt="eager_from_alias") def contains_eager(*keys: _AttrType, **kw: Any) -> _AbstractLoad: return _generate_from_keys(Load.contains_eager, keys, True, kw) @@ -2448,16 +2503,19 @@ def load_only(*attrs: _AttrType, raiseload: bool = False) -> _AbstractLoad: @loader_unbound_fn +@_strategy_labels((("lazy", "joined"),)) def joinedload(*keys: _AttrType, **kw: Any) -> _AbstractLoad: return _generate_from_keys(Load.joinedload, keys, False, kw) @loader_unbound_fn +@_strategy_labels((("lazy", "subquery"),)) def subqueryload(*keys: _AttrType) -> _AbstractLoad: return _generate_from_keys(Load.subqueryload, keys, False, {}) @loader_unbound_fn +@_strategy_labels((("lazy", "selectin"),)) def selectinload( *keys: _AttrType, recursion_depth: Optional[int] = None, @@ -2472,11 +2530,13 @@ def selectinload( @loader_unbound_fn +@_strategy_labels((("lazy", "select"),)) def lazyload(*keys: _AttrType) -> _AbstractLoad: return _generate_from_keys(Load.lazyload, keys, False, {}) @loader_unbound_fn +@_strategy_labels((("lazy", "immediate"),)) def immediateload( *keys: _AttrType, recursion_depth: Optional[int] = None ) -> _AbstractLoad: @@ -2486,11 +2546,13 @@ def immediateload( @loader_unbound_fn +@_strategy_labels((("lazy", "noload"),)) def noload(*keys: _AttrType) -> _AbstractLoad: return _generate_from_keys(Load.noload, keys, False, {}) @loader_unbound_fn +@_strategy_labels((("lazy", "raise"),), (("lazy", "raise_on_sql"),)) def raiseload(*keys: _AttrType, **kw: Any) -> _AbstractLoad: return _generate_from_keys(Load.raiseload, keys, False, kw) @@ -2501,6 +2563,10 @@ def defaultload(*keys: _AttrType) -> _AbstractLoad: @loader_unbound_fn +@_strategy_labels( + (("deferred", True), ("instrument", True)), + (("deferred", True), ("instrument", True), ("raiseload", True)), +) def defer(key: _AttrType, *, raiseload: bool = False) -> _AbstractLoad: if raiseload: kw = {"raiseload": raiseload} @@ -2511,6 +2577,7 @@ def defer(key: _AttrType, *, raiseload: bool = False) -> _AbstractLoad: @loader_unbound_fn +@_strategy_labels((("deferred", False), ("instrument", True))) def undefer(key: _AttrType) -> _AbstractLoad: return _generate_from_keys(Load.undefer, (key,), False, {}) @@ -2522,6 +2589,7 @@ def undefer_group(name: str) -> _AbstractLoad: @loader_unbound_fn +@_strategy_labels((("query_expression", True),)) def with_expression( key: _AttrType, expression: _ColumnExpressionArgument[Any] ) -> _AbstractLoad: @@ -2531,6 +2599,7 @@ def with_expression( @loader_unbound_fn +@_strategy_labels((("selectinload_polymorphic", True),)) def selectin_polymorphic( base_cls: _EntityType[Any], classes: Iterable[Type[Any]] ) -> _AbstractLoad: @@ -2541,22 +2610,23 @@ def selectin_polymorphic( 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'link from relationship "{entity_str(path[-2])}%s".%s' % ( - f".of_type({path[-1]})" if path_is_of_type else "", + ( + f".of_type({entity_str(path[-1])})" + if path_is_of_type + else "" + ), ( " Did you mean to use " - f'"{path[-2]}' - f'.of_type({parent_entity_str})" or "loadopt.options(' + f'"{entity_str(path[-2])}' + f'.of_type({entity_str(parent_entity)})" or ' + '"loadopt.options(' f"selectin_polymorphic({path[-2].mapper.class_.__name__}, " - f'[{parent_entity_str}]), ...)" ?' + f'[{entity_str(parent_entity)}]), ...)" ?' if not path_is_of_type and not path[-1].is_aliased_class and orm_util._entity_corresponds_to( @@ -2569,5 +2639,5 @@ def _raise_for_does_not_link(path, attrname, parent_entity): else: raise sa_exc.ArgumentError( f'ORM mapped attribute "{attrname}" does not ' - f'link mapped class "{path[-1]}"' + f'link mapped class "{entity_str(path[-1])}"' ) diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index f333802622..3d5fb5b5bf 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -1302,6 +1302,13 @@ class AliasedInsp( ) def __str__(self): + return self.path_string() + + def path_string(self) -> str: + """Return a user-facing name for this :class:`.AliasedInsp`, + for use in a :class:`_orm.PathRegistry` string representation. + + """ if self._is_with_polymorphic: return "with_polymorphic(%s, [%s])" % ( self._target.__name__, diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py index 95c362d8fa..b827533665 100644 --- a/test/orm/test_deferred.py +++ b/test/orm/test_deferred.py @@ -534,8 +534,8 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest): q2 = q.options(undefer(Order.user_id)) with expect_raises_message( sa.exc.InvalidRequestError, - r"Loader strategies for ORM Path\[Mapper\[Order\(orders\)\] -> " - r"Order.user_id\] conflict", + r"Loader strategy replacement undefer\(Order.user_id\) " + r"is in conflict with existing strategy defer\(Order.user_id\)", ): q2.all() @@ -2116,7 +2116,8 @@ class InheritanceTest(_Polymorphic): assert_raises_message( sa.exc.ArgumentError, - r"Mapped class Mapper\[Manager\(managers\)\] does not apply to " + r"Mapped class Manager referenced in option " + r"undefer\(Manager.status\) does not apply to " "any of the root entities in this query, e.g. " r"with_polymorphic\(Person, \[Manager\]\).", s.query(wp).options(load_only(Manager.status))._compile_context, diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index c644645631..428d8ffc05 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -6822,9 +6822,10 @@ class DeepOptionsTest(_fixtures.FixtureTest): assert_raises_message( sa.exc.ArgumentError, - r"Mapped class Mapper\[Order\(orders\)\] does not apply to any of " - "the " - r"root entities in this query, e.g. Mapper\[User\(users\)\]. " + r"Mapped class Order referenced in option " + r"joinedload\(Order.items\) " + r"does not apply to any of the root entities in this query, " + r"e.g. User. " "Please specify the full path from one of the root entities " "to the target attribute.", sess.query(User) diff --git a/test/orm/test_expire.py b/test/orm/test_expire.py index 84511cb226..f85ceaff16 100644 --- a/test/orm/test_expire.py +++ b/test/orm/test_expire.py @@ -855,7 +855,8 @@ class ExpireTest(_fixtures.FixtureTest): # for up front with expect_raises_message( sa.exc.ArgumentError, - r"Mapped class Mapper\[User\(users\)\] does not apply to " + r"Mapped class User referenced in option " + r"joinedload\(User.addresses\) does not apply to " "any of the root entities in this query", ): row = sess.execute( diff --git a/test/orm/test_options.py b/test/orm/test_options.py index c6058a80b3..fb7800a022 100644 --- a/test/orm/test_options.py +++ b/test/orm/test_options.py @@ -16,16 +16,23 @@ from sqlalchemy.orm import contains_eager from sqlalchemy.orm import defaultload from sqlalchemy.orm import defer from sqlalchemy.orm import exc as orm_exc +from sqlalchemy.orm import immediateload from sqlalchemy.orm import joinedload +from sqlalchemy.orm import lazyload from sqlalchemy.orm import Load from sqlalchemy.orm import load_only from sqlalchemy.orm import loading +from sqlalchemy.orm import noload +from sqlalchemy.orm import query_expression +from sqlalchemy.orm import raiseload from sqlalchemy.orm import relationship +from sqlalchemy.orm import selectinload from sqlalchemy.orm import strategy_options from sqlalchemy.orm import subqueryload from sqlalchemy.orm import synonym from sqlalchemy.orm import undefer from sqlalchemy.orm import util as orm_util +from sqlalchemy.orm import with_expression from sqlalchemy.orm import with_polymorphic from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_not @@ -115,7 +122,7 @@ class LoadTest(PathTest, QueryTest): result = Load(User) eq_( str(result), - "Load(Mapper[User(users)])", + "Load(User)", ) result = Load(aliased(User)) @@ -478,7 +485,8 @@ class OptionsTest(PathTest, QueryTest): with expect_raises_message( sa.exc.ArgumentError, - r"Mapped class Mapper\[User\(users\)\] does not apply to any " + r"Mapped class User referenced in option " + r"contains_eager\(User.addresses\) does not apply to any " "of the root entities in this query", ): self._assert_path_result(opt, q, []) @@ -902,7 +910,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest): Item.id, Item.keywords, r"Query has only expression-based entities; attribute loader " - r"options for Mapper\[Item\(items\)\] can't be applied here.", + r"option joinedload\(Item.keywords\) can't be applied here.", ) def test_option_against_nonexistent_PropComparator(self): @@ -911,10 +919,10 @@ class OptionsNoPropTest(_fixtures.FixtureTest): self._assert_eager_with_entity_exception( [Keyword], (joinedload(Item.keywords),), - r"Mapped class Mapper\[Item\(items\)\] does not apply to any of " - "the root entities in this query, e.g. " - r"Mapper\[Keyword\(keywords\)\]. " - "Please specify the full path from one of " + r"Mapped class Item referenced in option " + r"joinedload\(Item.keywords\) " + "does not apply to any of the root entities in this query, e.g. " + r"Keyword. Please specify the full path from one of " "the root entities to the target attribute. ", ) @@ -969,8 +977,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest): [Keyword.id, Item.id], lambda: (joinedload(Keyword.keywords),), r"Query has only expression-based entities; attribute loader " - r"options for Mapper\[Keyword\(keywords\)\] can't be applied " - "here.", + r"option joinedload\(Keyword.keywords\) can't be applied here.", ) @testing.combinations(True, False, argnames="first_element") @@ -1150,9 +1157,9 @@ class OptionsNoPropTestInh(_Polymorphic): assert_raises_message( sa.exc.ArgumentError, - r"Mapped class Mapper\[Manager\(managers\)\] does not apply to " - "any of " - r"the root entities in this query, e.g. " + r"Mapped class Manager referenced in option " + r"undefer\(Manager.status\) does not apply to " + r"any of the root entities in this query, e.g. " r"with_polymorphic\(Person, \[Manager\]\).", s.query(wp).options(load_only(Manager.status))._compile_state, ) @@ -1183,8 +1190,7 @@ class OptionsNoPropTestInh(_Polymorphic): sa.exc.ArgumentError, r'ORM mapped entity or attribute "Manager.manager_name" does ' r"not link from " - r'relationship "Company.employees.' - r'of_type\(Mapper\[Engineer\(engineers\)\]\)".$', + r'relationship "Company.employees.of_type\(Engineer\)".$', lambda: s.query(Company) .options( joinedload(Company.employees.of_type(Engineer)).load_only( @@ -1203,8 +1209,7 @@ class OptionsNoPropTestInh(_Polymorphic): sa.exc.ArgumentError, r'ORM mapped entity or attribute "Manager.status" does ' r"not link from " - r'relationship "Company.employees.' - r'of_type\(Mapper\[Engineer\(engineers\)\]\)".$', + r'relationship "Company.employees.of_type\(Engineer\)".$', lambda: s.query(Company) .options( joinedload(Company.employees.of_type(Engineer)).load_only( @@ -1908,8 +1913,9 @@ class MapperOptionsTest(_fixtures.FixtureTest): joinedload(User.orders), contains_eager(User.orders), ), - r"Loader strategies for ORM Path\[Mapper\[User\(users\)\] -> " - r"User.orders -> Mapper\[Order\(orders\)\]\] conflict", + r"Loader strategy replacement contains_eager\(User.orders\) " + r"is in conflict with existing strategy " + r"joinedload\(User.orders\)", ), ( lambda User, Order: ( @@ -1925,13 +1931,15 @@ class MapperOptionsTest(_fixtures.FixtureTest): Order.items ), ), - r"Loader strategies for ORM Path\[Mapper\[User\(users\)\] -> " - r"User.orders -> Mapper\[Order\(orders\)\]\] conflict", + r"Loader strategy replacement " + r"joinedload\(User.orders\) " + r"is in conflict with existing strategy " + r"joinedload\(User.orders\)", ), ( lambda User: (defer(User.name), undefer(User.name)), - r"Loader strategies for ORM Path\[Mapper\[User\(users\)\] -> " - r"User.name\] conflict", + r"Loader strategy replacement undefer\(User.name\) " + r"is in conflict with existing strategy defer\(User.name\)", ), ) def test_conflicts(self, make_opt, errmsg): @@ -1972,3 +1980,90 @@ class MapperOptionsTest(_fixtures.FixtureTest): sess.query(User).options(opt)._compile_context() else: sess.query(User).options(opt)._compile_context() + + @testing.combinations( + ( + lambda User: joinedload(User.addresses), + "joinedload(User.addresses)", + ), + ( + lambda User: contains_eager(User.addresses), + "contains_eager(User.addresses)", + ), + ( + lambda User: subqueryload(User.addresses), + "subqueryload(User.addresses)", + ), + ( + lambda User: selectinload(User.addresses), + "selectinload(User.addresses)", + ), + ( + # rendering of of_type() is also tested in + # test_utils.py::PathRegistryTest but do a spot check here too + lambda User: selectinload( + User.addresses.of_type(aliased(Address)) + ), + "selectinload(User.addresses.of_type(aliased(Address)))", + ), + ( + lambda User: lazyload(User.addresses), + "lazyload(User.addresses)", + ), + ( + lambda User: immediateload(User.addresses), + "immediateload(User.addresses)", + ), + ( + lambda User: noload(User.addresses), + "noload(User.addresses)", + ), + ( + lambda User: raiseload(User.addresses), + "raiseload(User.addresses)", + ), + ( + lambda User: raiseload(User.addresses, sql_only=True), + "raiseload(User.addresses)", + ), + ( + lambda User: defer(User.name), + "defer(User.name)", + ), + ( + lambda User: defer(User.name, raiseload=True), + "defer(User.name)", + ), + ( + lambda User: undefer(User.name), + "undefer(User.name)", + ), + ( + lambda User: with_expression(User.expr_col, User.name), + "with_expression(User.expr_col)", + ), + ) + @testing.emits_warning + def test_strategy_labels_str(self, make_opt, expected_str): + users, addresses, User, Address = ( + self.tables.users, + self.tables.addresses, + self.classes.User, + self.classes.Address, + ) + + self.mapper_registry.map_imperatively( + User, + users, + properties={ + "addresses": relationship( + self.mapper_registry.map_imperatively(Address, addresses) + ), + "expr_col": query_expression(), + }, + ) + + opt = testing.resolve_lambda(make_opt, User=User) + + elements = [el for el in opt.context if el.strategy is not None] + eq_(str(elements[0]), expected_str) diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py index a685274d09..de70195042 100644 --- a/test/orm/test_utils.py +++ b/test/orm/test_utils.py @@ -1043,6 +1043,113 @@ class PathRegistryTest(_fixtures.FixtureTest): ) eq_(PathRegistry.deserialize([(User, "addresses")]), p3) + @testing.combinations( + ( + lambda umapper: PathRegistry.coerce( + (umapper, umapper.attrs.addresses) + ), + "User.addresses", + ), + ( + lambda umapper: PathRegistry.coerce((umapper,)), + "User", + ), + ( + lambda ualias: PathRegistry.coerce((ualias,)), + "aliased(User)", + ), + ( + lambda umapper, amapper: PathRegistry.coerce( + (umapper, umapper.attrs.addresses, amapper) + ), + "User.addresses", + ), + ( + lambda ualias, umapper, amapper: PathRegistry.coerce( + (ualias, umapper.attrs.addresses, amapper) + ), + "aliased(User).addresses", + ), + ( + lambda umapper, aalias: PathRegistry.coerce( + (umapper, umapper.attrs.addresses, aalias) + ), + "User.addresses.of_type(aliased(Address))", + ), + ( + lambda: RootRegistry(), + "", + ), + ( + lambda umapper, omapper, imapper: PathRegistry.coerce( + ( + umapper, + umapper.attrs.orders, + omapper, + omapper.attrs["items"], + imapper, + ) + ), + "User.orders -> Order.items", + ), + ( + lambda umapper, omapper, oalias, imapper: PathRegistry.coerce( + ( + umapper, + umapper.attrs.orders, + oalias, + omapper.attrs["items"], + imapper, + ) + ), + "User.orders.of_type(aliased(Order)) -> aliased(Order).items", + ), + ( + lambda umapper, omapper, owpoly, imapper: PathRegistry.coerce( + ( + umapper, + umapper.attrs.orders, + owpoly, + omapper.attrs["items"], + imapper, + ) + ), + "User.orders.of_type(with_polymorphic(Order, []))" + " -> with_polymorphic(Order, []).items", + ), + argnames="path_reg,expected", + ) + def test_path_string(self, path_reg, expected): + umapper = inspect(self.classes.User) + amapper = inspect(self.classes.Address) + omapper = inspect(self.classes.Order) + imapper = inspect(self.classes.Item) + auser = aliased(self.classes.User) + ualias = inspect(auser) + aaddress = aliased(self.classes.Address) + aorder = aliased(self.classes.Order) + aalias = inspect(aaddress) + oalias = inspect(aorder) + + # this is a "fake" with_polymorphic but we expect it to trip the + # _is_with_polymorphic flag. if this changes, then we need to bring + # in the full polymorphic fixture + wpolyorder = with_polymorphic(self.classes.Order, [self.classes.Order]) + owpoly = inspect(wpolyorder) + + p1 = testing.resolve_lambda( + path_reg, + umapper=umapper, + amapper=amapper, + ualias=ualias, + aalias=aalias, + oalias=oalias, + owpoly=owpoly, + omapper=omapper, + imapper=imapper, + ) + eq_(p1.path_string(), expected) + class PathRegistryInhTest(_poly_fixtures._Polymorphic): run_setup_mappers = "once"