don't invoke fromclause.c when creating an annotated

The ``aliased()`` constructor calls upon ``__clause_element__()``,
which internally annotates a ``FromClause``, like a subquery.
This became expensive as ``AnnotatedFromClause`` has for
many years called upon ``element.c`` so that the full ``.c``
collection is transferred to the Annotated.

Taking this out proved to be challenging. A straight remove
seemed to not break any tests except for the one that
tested the exact condition.  Nevertheless this seemed
"spooky" so I instead moved the get of ``.c`` to be in a
memoized proxy method.   However, that then exposed
a recursion issue related to loader_criteria; so the
source of that behavior, which was an accidental behavioral
artifact, is now made into an explcicit option that
loader_criteria uses directly.

The accidental behavioral artifact in question is still
kind of strange since I was not able to fully trace out
how it works, but the end result is that fixing the
artifact to be "correct" causes loader_criteria, within
the particular test for #7491, creates a select/
subquery structure with a cycle in it, so compilation fails
with recursion overflow.
The "solution" is to cause the artifact to occur in this
case, which is that the ``AnnotatedFromClause`` will have a
different ``.c`` collection than its element, which is a
subquery.  It's not totally clear how a cycle is generated
when this is not done.

This is commit one of two, which goes through
some hoops to make essentially a one-line change.

The next commit will rework ColumnCollection to optimize
the corresponding_column() method significantly.

Fixes: #8796
Change-Id: Id58ae6554db62139462c11a8be7313a3677456ad

6 files changed, 63 insertions, 9 deletions

diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 6cd98f5ea..50eba5d4c 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -1317,7 +1317,10 @@ class LoaderCriteriaOption(CriteriaOption):
             crit = self.where_criteria  # type: ignore
         assert isinstance(crit, ColumnElement)
         return sql_util._deep_annotate(
-            crit, {"for_loader_criteria": self}, detect_subquery_cols=True
+            crit,
+            {"for_loader_criteria": self},
+            detect_subquery_cols=True,
+            ind_cols_on_fromclause=True,
         )
 
     def process_compile_state_replaced_entities(
@@ -1416,6 +1419,8 @@ class Bundle(
 
     _propagate_attrs: _PropagateAttrsType = util.immutabledict()
 
+    proxy_set = util.EMPTY_SET  # type: ignore
+
     exprs: List[_ColumnsClauseElement]
 
     def __init__(
diff --git a/lib/sqlalchemy/sql/annotation.py b/lib/sqlalchemy/sql/annotation.py
index 43ca84abb..3ce524447 100644
--- a/lib/sqlalchemy/sql/annotation.py
+++ b/lib/sqlalchemy/sql/annotation.py
@@ -421,6 +421,7 @@ def _deep_annotate(
     annotations: _AnnotationDict,
     exclude: Optional[Sequence[SupportsAnnotations]] = None,
     detect_subquery_cols: bool = False,
+    ind_cols_on_fromclause: bool = False,
 ) -> _SA:
     """Deep copy the given ClauseElement, annotating each element
     with the given annotations dictionary.
@@ -435,6 +436,16 @@ def _deep_annotate(
     cloned_ids: Dict[int, SupportsAnnotations] = {}
 
     def clone(elem: SupportsAnnotations, **kw: Any) -> SupportsAnnotations:
+
+        # ind_cols_on_fromclause means make sure an AnnotatedFromClause
+        # has its own .c collection independent of that which its proxying.
+        # this is used specifically by orm.LoaderCriteriaOption to break
+        # a reference cycle that it's otherwise prone to building,
+        # see test_relationship_criteria->
+        # test_loader_criteria_subquery_w_same_entity.  logic here was
+        # changed for #8796 and made explicit; previously it occurred
+        # by accident
+
         kw["detect_subquery_cols"] = detect_subquery_cols
         id_ = id(elem)
 
@@ -454,7 +465,11 @@ def _deep_annotate(
                 newelem = elem._annotate(annotations)
         else:
             newelem = elem
-        newelem._copy_internals(clone=clone)
+
+        newelem._copy_internals(
+            clone=clone, ind_cols_on_fromclause=ind_cols_on_fromclause
+        )
+
         cloned_ids[id_] = newelem
         return newelem
 
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 488dfe721..3d3aea3f2 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -6752,8 +6752,28 @@ TextAsFrom = TextualSelect
 
 
 class AnnotatedFromClause(Annotated):
-    def __init__(self, element, values):
-        # force FromClause to generate their internal
-        # collections into __dict__
-        element.c
-        Annotated.__init__(self, element, values)
+    def _copy_internals(self, **kw):
+        super()._copy_internals(**kw)
+        if kw.get("ind_cols_on_fromclause", False):
+            ee = self._Annotated__element  # type: ignore
+
+            self.c = ee.__class__.c.fget(self)  # type: ignore
+
+    @util.ro_memoized_property
+    def c(self) -> ReadOnlyColumnCollection[str, KeyedColumnElement[Any]]:
+        """proxy the .c collection of the underlying FromClause.
+
+        Originally implemented in 2008 as a simple load of the .c collection
+        when the annotated construct was created (see d3621ae961a), in modern
+        SQLAlchemy versions this can be expensive for statements constructed
+        with ORM aliases.   So for #8796 SQLAlchemy 2.0 we instead proxy
+        it, which works just as well.
+
+        Two different use cases seem to require the collection either copied
+        from the underlying one, or unique to this AnnotatedFromClause.
+
+        See test_selectable->test_annotated_corresponding_column
+
+        """
+        ee = self._Annotated__element  # type: ignore
+        return ee.c  # type: ignore
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 55c6a35f8..623a3f896 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -1197,7 +1197,9 @@ class ClauseAdapter(visitors.ReplacingExternalTraversal):
         elif self.exclude_fn and self.exclude_fn(col):
             return None
         else:
-            return self._corresponding_column(col, True)  # type: ignore
+            return self._corresponding_column(  # type: ignore
+                col, require_embedded=True
+            )
 
 
 class _ColumnLookup(Protocol):
diff --git a/lib/sqlalchemy/testing/fixtures.py b/lib/sqlalchemy/testing/fixtures.py
index 5fb547cbc..dcee3f18b 100644
--- a/lib/sqlalchemy/testing/fixtures.py
+++ b/lib/sqlalchemy/testing/fixtures.py
@@ -27,6 +27,7 @@ from .util import drop_all_tables_from_metadata
 from .. import event
 from .. import util
 from ..orm import DeclarativeBase
+from ..orm import events as orm_events
 from ..orm import MappedAsDataclass
 from ..orm import registry
 from ..schema import sort_tables_and_constraints
@@ -618,6 +619,17 @@ class RemovesEvents:
             event.remove(*key)
 
 
+class RemoveORMEventsGlobally:
+    @config.fixture(autouse=True)
+    def _remove_listeners(self):
+        yield
+        orm_events.MapperEvents._clear()
+        orm_events.InstanceEvents._clear()
+        orm_events.SessionEvents._clear()
+        orm_events.InstrumentationEvents._clear()
+        orm_events.QueryEvents._clear()
+
+
 _fixture_sessions = set()
 
 
diff --git a/lib/sqlalchemy/util/preloaded.py b/lib/sqlalchemy/util/preloaded.py
index 6332b3c94..ae700b77d 100644
--- a/lib/sqlalchemy/util/preloaded.py
+++ b/lib/sqlalchemy/util/preloaded.py
@@ -35,7 +35,7 @@ if TYPE_CHECKING:
     from sqlalchemy.orm import decl_base as _orm_decl_base
     from sqlalchemy.orm import dependency as _orm_dependency
     from sqlalchemy.orm import descriptor_props as _orm_descriptor_props
-    from sqlalchemy.orm import mapper as _orm_mapper
+    from sqlalchemy.orm import mapperlib as _orm_mapper
     from sqlalchemy.orm import properties as _orm_properties
     from sqlalchemy.orm import relationships as _orm_relationships
     from sqlalchemy.orm import session as _orm_session