disable polymorphic adaption in most cases

Improved a fix first made in version 1.4 for :ticket:`8456` which scaled
back the usage of internal "polymorphic adapters", that are used to render
ORM queries when the :paramref:`_orm.Mapper.with_polymorphic` parameter is
used. These adapters, which are very complex and error prone, are now used
only in those cases where an explicit user-supplied subquery is used for
:paramref:`_orm.Mapper.with_polymorphic`, which includes only the use case
of concrete inheritance mappings that use the
:func:`_orm.polymorphic_union` helper, as well as the legacy use case of
using an aliased subquery for joined inheritance mappings, which is not
needed in modern use.

For the most common case of joined inheritance mappings that use the
built-in polymorphic loading scheme, which includes those which make use of
the :paramref:`_orm.Mapper.polymorphic_load` parameter set to ``inline``,
polymorphic adapters are now no longer used. This has both a positive
performance impact on the construction of queries as well as a
substantial simplification of the internal query rendering process.

The specific issue targeted was to allow a :func:`_orm.column_property`
to refer to joined-inheritance classes within a scalar subquery, which now
works as intuitively as is feasible.

ORM context, mapper, strategies now use ORMAdapter in all cases
instead of straight ColumnAdapter; added some more parameters
to ORMAdapter to make this possible.  ORMAdapter now includes a
"trace" enumeration that identifies the use path for the
adapter and can aid in debugging.

implement __slots__ for the ExternalTraversal hierarchy up
to ORMAdapter.  Within this change, we have to change the
ClauseAdapter.wrap() method, which is only used in one polymorphic
codepath, to use copy.copy() instead of
`__dict__` access (apparently `__reduce_ex__` is implemented for
objects with `__slots__`), and we also remove pickling ability,
which should not be needed for adapters (this might have been needed
for 1.3 and earlier in order for Query to be picklable, but none
of that state is present within Query / select() / etc. anymore).

Fixes: #8168
Change-Id: I3f6593eb02ab5e5964807c53a9fa4894c826d017

4 files changed, 203 insertions, 37 deletions

diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index b5b326bca..b3c8e78b3 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -31,9 +31,11 @@ from .interfaces import ORMColumnsClauseRole
 from .path_registry import PathRegistry
 from .util import _entity_corresponds_to
 from .util import _ORMJoin
+from .util import _TraceAdaptRole
 from .util import AliasedClass
 from .util import Bundle
 from .util import ORMAdapter
+from .util import ORMStatementAdapter
 from .. import exc as sa_exc
 from .. import future
 from .. import inspect
@@ -509,7 +511,15 @@ class ORMCompileState(AbstractORMCompileState):
 
     def _create_with_polymorphic_adapter(self, ext_info, selectable):
         """given MapperEntity or ORMColumnEntity, setup polymorphic loading
-        if appropriate
+        if called for by the Mapper.
+
+        As of #8168 in 2.0.0b5, polymorphic adapters, which greatly increase
+        the complexity of the query creation process, are not used at all
+        except in the quasi-legacy cases of with_polymorphic referring to an
+        alias and/or subquery. This would apply to concrete polymorphic
+        loading, and joined inheritance where a subquery is
+        passed to with_polymorphic (which is completely unnecessary in modern
+        use).
 
         """
         if (
@@ -521,7 +531,10 @@ class ORMCompileState(AbstractORMCompileState):
                 self._mapper_loads_polymorphically_with(
                     mp,
                     ORMAdapter(
-                        mp, mp._equivalent_columns, selectable=selectable
+                        _TraceAdaptRole.WITH_POLYMORPHIC_ADAPTER,
+                        mp,
+                        equivalents=mp._equivalent_columns,
+                        selectable=selectable,
                     ),
                 )
 
@@ -727,8 +740,11 @@ class ORMFromStatementCompileState(ORMCompileState):
                 )
                 == "orm"
             )
-            self._from_obj_alias = sql.util.ColumnAdapter(
-                self.statement, adapt_on_names=not statement_is_orm
+
+            self._from_obj_alias = ORMStatementAdapter(
+                _TraceAdaptRole.ADAPT_FROM_STATEMENT,
+                self.statement,
+                adapt_on_names=not statement_is_orm,
             )
 
         return self
@@ -1068,6 +1084,7 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
         self._join_entities = ()
 
         if self.compile_options._set_base_alias:
+            # legacy Query only
             self._set_select_from_alias()
 
         for memoized_entities in query._memoized_select_entities:
@@ -1285,6 +1302,7 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
         return stmt
 
     def _set_select_from_alias(self):
+        """used only for legacy Query cases"""
 
         query = self.select_statement  # query
 
@@ -1297,6 +1315,8 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
             self._from_obj_alias = adapter
 
     def _get_select_from_alias_from_obj(self, from_obj):
+        """used only for legacy Query cases"""
+
         info = from_obj
 
         if "parententity" in info._annotations:
@@ -1313,7 +1333,12 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
 
         elif isinstance(info.selectable, sql.selectable.AliasedReturnsRows):
             equivs = self._all_equivs()
-            return sql_util.ColumnAdapter(info, equivs)
+            assert info is info.selectable
+            return ORMStatementAdapter(
+                _TraceAdaptRole.LEGACY_SELECT_FROM_ALIAS,
+                info.selectable,
+                equivalents=equivs,
+            )
         else:
             return None
 
@@ -1417,7 +1442,9 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
 
         equivs = self._all_equivs()
 
-        self.compound_eager_adapter = sql_util.ColumnAdapter(inner, equivs)
+        self.compound_eager_adapter = ORMStatementAdapter(
+            _TraceAdaptRole.COMPOUND_EAGER_STATEMENT, inner, equivalents=equivs
+        )
 
         statement = future.select(
             *([inner] + self.secondary_columns)  # use_labels=self.labels
@@ -2128,10 +2155,14 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
             )
 
         if need_adapter:
+            # if need_adapter is True, we are in a deprecated case and
+            # a warning has been emitted.
             assert right_mapper
 
             adapter = ORMAdapter(
-                inspect(right), equivalents=right_mapper._equivalent_columns
+                _TraceAdaptRole.DEPRECATED_JOIN_ADAPT_RIGHT_SIDE,
+                inspect(right),
+                equivalents=right_mapper._equivalent_columns,
             )
 
             # if an alias() on the right side was generated,
@@ -2142,11 +2173,7 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
         elif (
             not r_info.is_clause_element
             and not right_is_aliased
-            and right_mapper.with_polymorphic
-            and isinstance(
-                right_mapper._with_polymorphic_selectable,
-                expression.AliasedReturnsRows,
-            )
+            and right_mapper._has_aliased_polymorphic_fromclause
         ):
             # for the case where the target mapper has a with_polymorphic
             # set up, ensure an adapter is set up for criteria that works
@@ -2159,9 +2186,11 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
             # and similar
             self._mapper_loads_polymorphically_with(
                 right_mapper,
-                sql_util.ColumnAdapter(
-                    right_mapper.selectable,
-                    right_mapper._equivalent_columns,
+                ORMAdapter(
+                    _TraceAdaptRole.WITH_POLYMORPHIC_ADAPTER_RIGHT_JOIN,
+                    right_mapper,
+                    selectable=right_mapper.selectable,
+                    equivalents=right_mapper._equivalent_columns,
                 ),
             )
         # if the onclause is a ClauseElement, adapt it with any
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index d15c882c4..7a7524621 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -103,6 +103,7 @@ if TYPE_CHECKING:
     from .properties import ColumnProperty
     from .relationships import RelationshipProperty
     from .state import InstanceState
+    from .util import ORMAdapter
     from ..engine import Row
     from ..engine import RowMapping
     from ..sql._typing import _ColumnExpressionArgument
@@ -112,7 +113,6 @@ if TYPE_CHECKING:
     from ..sql.elements import ColumnElement
     from ..sql.schema import Column
     from ..sql.selectable import FromClause
-    from ..sql.util import ColumnAdapter
     from ..util import OrderedSet
 
 
@@ -2441,6 +2441,20 @@ class Mapper(
             return None
 
     @HasMemoized.memoized_attribute
+    def _has_aliased_polymorphic_fromclause(self):
+        """return True if with_polymorphic[1] is an aliased fromclause,
+        like a subquery.
+
+        As of #8168, polymorphic adaption with ORMAdapter is used only
+        if this is present.
+
+        """
+        return self.with_polymorphic and isinstance(
+            self.with_polymorphic[1],
+            expression.AliasedReturnsRows,
+        )
+
+    @HasMemoized.memoized_attribute
     def _should_select_with_poly_adapter(self):
         """determine if _MapperEntity or _ORMColumnEntity will need to use
         polymorphic adaption when setting up a SELECT as well as fetching
@@ -2456,6 +2470,10 @@ class Mapper(
         # polymorphic selectable, *or* if the base mapper has either of those,
         # we turn on the adaption thing.  if not, we do *no* adaption.
         #
+        # (UPDATE for #8168: the above comment was not accurate, as we were
+        # still saying "do polymorphic" if we were using an auto-generated
+        # flattened JOIN for with_polymorphic.)
+        #
         # this splits the behavior among the "regular" joined inheritance
         # and single inheritance mappers, vs. the "weird / difficult"
         # concrete and joined inh mappings that use a with_polymorphic of
@@ -2467,11 +2485,21 @@ class Mapper(
         # these tests actually adapt the polymorphic selectable (like, the
         # UNION or the SELECT subquery with JOIN in it) to be just the simple
         # subclass table.   Hence even if we are a "plain" inheriting mapper
-        # but our base has a wpoly on it, we turn on adaption.
+        # but our base has a wpoly on it, we turn on adaption.  This is a
+        # legacy case we should probably disable.
+        #
+        #
+        # UPDATE: simplified way more as of #8168.   polymorphic adaption
+        # is turned off even if with_polymorphic is set, as long as there
+        # is no user-defined aliased selectable / subquery configured.
+        # this scales back the use of polymorphic adaption in practice
+        # to basically no cases except for concrete inheritance with a
+        # polymorphic base class.
+        #
         return (
-            self.with_polymorphic
+            self._has_aliased_polymorphic_fromclause
             or self._requires_row_aliasing
-            or self.base_mapper.with_polymorphic
+            or (self.base_mapper._has_aliased_polymorphic_fromclause)
             or self.base_mapper._requires_row_aliasing
         )
 
@@ -2743,10 +2771,14 @@ class Mapper(
         ]
 
     @HasMemoized.memoized_attribute
-    def _polymorphic_adapter(self) -> Optional[sql_util.ColumnAdapter]:
-        if self.with_polymorphic:
-            return sql_util.ColumnAdapter(
-                self.selectable, equivalents=self._equivalent_columns
+    def _polymorphic_adapter(self) -> Optional[orm_util.ORMAdapter]:
+        if self._has_aliased_polymorphic_fromclause:
+            return orm_util.ORMAdapter(
+                orm_util._TraceAdaptRole.MAPPER_POLYMORPHIC_ADAPTER,
+                self,
+                selectable=self.selectable,
+                equivalents=self._equivalent_columns,
+                limit_on_entity=False,
             )
         else:
             return None
@@ -3213,7 +3245,7 @@ class Mapper(
         self,
         row: Optional[Union[Row[Any], RowMapping]],
         identity_token: Optional[Any] = None,
-        adapter: Optional[ColumnAdapter] = None,
+        adapter: Optional[ORMAdapter] = None,
     ) -> _IdentityKeyType[_O]:
         """Return an identity-map key for use in storing/retrieving an
         item from the identity map.
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index efa0dc680..59031171d 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -2229,8 +2229,12 @@ class JoinedLoader(AbstractRelationshipLoader):
         if alias is not None:
             if isinstance(alias, str):
                 alias = prop.target.alias(alias)
-            adapter = sql_util.ColumnAdapter(
-                alias, equivalents=prop.mapper._equivalent_columns
+            adapter = orm_util.ORMAdapter(
+                orm_util._TraceAdaptRole.JOINEDLOAD_USER_DEFINED_ALIAS,
+                prop.mapper,
+                selectable=alias,
+                equivalents=prop.mapper._equivalent_columns,
+                limit_on_entity=False,
             )
         else:
             if path.contains(
@@ -2240,6 +2244,7 @@ class JoinedLoader(AbstractRelationshipLoader):
                     compile_state.attributes, "path_with_polymorphic"
                 )
                 adapter = orm_util.ORMAdapter(
+                    orm_util._TraceAdaptRole.JOINEDLOAD_PATH_WITH_POLYMORPHIC,
                     with_poly_entity,
                     equivalents=prop.mapper._equivalent_columns,
                 )
@@ -2335,9 +2340,11 @@ class JoinedLoader(AbstractRelationshipLoader):
             to_adapt = self._gen_pooled_aliased_class(compile_state)
 
         to_adapt_insp = inspect(to_adapt)
+
         clauses = to_adapt_insp._memo(
             ("joinedloader_ormadapter", self),
             orm_util.ORMAdapter,
+            orm_util._TraceAdaptRole.JOINEDLOAD_MEMOIZED_ADAPTER,
             to_adapt_insp,
             equivalents=self.mapper._equivalent_columns,
             adapt_required=True,
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 6ed8e2272..2a00d98fb 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -8,9 +8,11 @@
 
 from __future__ import annotations
 
+import enum
 import re
 import types
 import typing
+from typing import AbstractSet
 from typing import Any
 from typing import Callable
 from typing import cast
@@ -441,25 +443,121 @@ def identity_key(
         raise sa_exc.ArgumentError("class or instance is required")
 
 
+class _TraceAdaptRole(enum.Enum):
+    """Enumeration of all the use cases for ORMAdapter.
+
+    ORMAdapter remains one of the most complicated aspects of the ORM, as it is
+    used for in-place adaption of column expressions to be applied to a SELECT,
+    replacing :class:`.Table` and other objects that are mapped to classes with
+    aliases of those tables in the case of joined eager loading, or in the case
+    of polymorphic loading as used with concrete mappings or other custom "with
+    polymorphic" parameters, with whole user-defined subqueries. The
+    enumerations provide an overview of all the use cases used by ORMAdapter, a
+    layer of formality as to the introduction of new ORMAdapter use cases (of
+    which none are anticipated), as well as a means to trace the origins of a
+    particular ORMAdapter within runtime debugging.
+
+    SQLAlchemy 2.0 has greatly scaled back ORM features which relied heavily on
+    open-ended statement adaption, including the ``Query.with_polymorphic()``
+    method and the ``Query.select_from_entity()`` methods, favoring
+    user-explicit aliasing schemes using the ``aliased()`` and
+    ``with_polymorphic()`` standalone constructs; these still use adaption,
+    however the adaption is applied in a narrower scope.
+
+    """
+
+    # aliased() use that is used to adapt individual attributes at query
+    # construction time
+    ALIASED_INSP = enum.auto()
+
+    # joinedload cases; typically adapt an ON clause of a relationship
+    # join
+    JOINEDLOAD_USER_DEFINED_ALIAS = enum.auto()
+    JOINEDLOAD_PATH_WITH_POLYMORPHIC = enum.auto()
+    JOINEDLOAD_MEMOIZED_ADAPTER = enum.auto()
+
+    # polymorphic cases - these are complex ones that replace FROM
+    # clauses, replacing tables with subqueries
+    MAPPER_POLYMORPHIC_ADAPTER = enum.auto()
+    WITH_POLYMORPHIC_ADAPTER = enum.auto()
+    WITH_POLYMORPHIC_ADAPTER_RIGHT_JOIN = enum.auto()
+    DEPRECATED_JOIN_ADAPT_RIGHT_SIDE = enum.auto()
+
+    # the from_statement() case, used only to adapt individual attributes
+    # from a given statement to local ORM attributes at result fetching
+    # time.  assigned to ORMCompileState._from_obj_alias
+    ADAPT_FROM_STATEMENT = enum.auto()
+
+    # the joinedload for queries that have LIMIT/OFFSET/DISTINCT case;
+    # the query is placed inside of a subquery with the LIMIT/OFFSET/etc.,
+    # joinedloads are then placed on the outside.
+    # assigned to ORMCompileState.compound_eager_adapter
+    COMPOUND_EAGER_STATEMENT = enum.auto()
+
+    # the legacy Query._set_select_from() case.
+    # this is needed for Query's set operations (i.e. UNION, etc. )
+    # as well as "legacy from_self()", which while removed from 2.0 as
+    # public API, is used for the Query.count() method.  this one
+    # still does full statement traversal
+    # assigned to ORMCompileState._from_obj_alias
+    LEGACY_SELECT_FROM_ALIAS = enum.auto()
+
+
+class ORMStatementAdapter(sql_util.ColumnAdapter):
+    """ColumnAdapter which includes a role attribute."""
+
+    __slots__ = ("role",)
+
+    def __init__(
+        self,
+        role: _TraceAdaptRole,
+        selectable: Selectable,
+        *,
+        equivalents: Optional[_EquivalentColumnMap] = None,
+        adapt_required: bool = False,
+        allow_label_resolve: bool = True,
+        anonymize_labels: bool = False,
+        adapt_on_names: bool = False,
+        adapt_from_selectables: Optional[AbstractSet[FromClause]] = None,
+    ):
+        self.role = role
+        super().__init__(
+            selectable,
+            equivalents=equivalents,
+            adapt_required=adapt_required,
+            allow_label_resolve=allow_label_resolve,
+            anonymize_labels=anonymize_labels,
+            adapt_on_names=adapt_on_names,
+            adapt_from_selectables=adapt_from_selectables,
+        )
+
+
 class ORMAdapter(sql_util.ColumnAdapter):
     """ColumnAdapter subclass which excludes adaptation of entities from
     non-matching mappers.
 
     """
 
+    __slots__ = ("role", "mapper", "is_aliased_class", "aliased_insp")
+
     is_aliased_class: bool
     aliased_insp: Optional[AliasedInsp[Any]]
 
     def __init__(
         self,
+        role: _TraceAdaptRole,
         entity: _InternalEntityType[Any],
+        *,
         equivalents: Optional[_EquivalentColumnMap] = None,
         adapt_required: bool = False,
         allow_label_resolve: bool = True,
         anonymize_labels: bool = False,
         selectable: Optional[Selectable] = None,
+        limit_on_entity: bool = True,
+        adapt_on_names: bool = False,
+        adapt_from_selectables: Optional[AbstractSet[FromClause]] = None,
     ):
-
+        self.role = role
         self.mapper = entity.mapper
         if selectable is None:
             selectable = entity.selectable
@@ -470,21 +568,18 @@ class ORMAdapter(sql_util.ColumnAdapter):
             self.is_aliased_class = False
             self.aliased_insp = None
 
-        sql_util.ColumnAdapter.__init__(
-            self,
+        super().__init__(
             selectable,
             equivalents,
             adapt_required=adapt_required,
             allow_label_resolve=allow_label_resolve,
             anonymize_labels=anonymize_labels,
-            include_fn=self._include_fn,
+            include_fn=self._include_fn if limit_on_entity else None,
+            adapt_on_names=adapt_on_names,
+            adapt_from_selectables=adapt_from_selectables,
         )
 
     def _include_fn(self, elem):
-        # TODO: we still have cases where we should return False here
-        # yet we are not able to reliably detect without false positives.
-        # see issue #8168
-
         entity = elem._annotations.get("parentmapper", None)
 
         return not entity or entity.isa(self.mapper) or self.mapper.isa(entity)
@@ -774,7 +869,7 @@ class AliasedInsp(
 
     mapper: Mapper[_O]
     selectable: FromClause
-    _adapter: sql_util.ColumnAdapter
+    _adapter: ORMAdapter
     with_polymorphic_mappers: Sequence[Mapper[Any]]
     _with_polymorphic_entities: Sequence[AliasedInsp[Any]]
 
@@ -840,8 +935,10 @@ class AliasedInsp(
             self._is_with_polymorphic = False
             self.with_polymorphic_mappers = [mapper]
 
-        self._adapter = sql_util.ColumnAdapter(
-            selectable,
+        self._adapter = ORMAdapter(
+            _TraceAdaptRole.ALIASED_INSP,
+            mapper,
+            selectable=selectable,
             equivalents=mapper._equivalent_columns,
             adapt_on_names=adapt_on_names,
             anonymize_labels=True,
@@ -853,6 +950,7 @@ class AliasedInsp(
                 for m in self.with_polymorphic_mappers
                 if not adapt_on_names
             },
+            limit_on_entity=False,
         )
 
         if nest_adapters: