From 93dc7ea1502c37793011b094447641361aff5aba Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sun, 13 Nov 2022 11:49:43 -0500
Subject: 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
---
 test/sql/test_selectable.py | 64 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 11 deletions(-)

(limited to 'test/sql')

diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 05847b1d0..897d6c5ff 100644
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -3167,33 +3167,75 @@ class AnnotationsTest(fixtures.TestBase):
         binary_2 = col_anno == 5
         eq_(binary_2.left._annotations, {"foo": "bar"})
 
-    def test_annotated_corresponding_column(self):
+    @testing.combinations(
+        ("plain",),
+        ("annotated",),
+        ("deep_annotated",),
+        ("deep_annotated_w_ind_col",),
+        argnames="testcase",
+    )
+    def test_annotated_corresponding_column(self, testcase):
+        """ensures the require_embedded case remains when an inner statement
+        was copied out for annotations.
+
+        First implemented in 2008 in d3621ae961a, the implementation is
+        updated for #8796 as a performance improvement as well as to
+        establish a discovered implicit behavior where clone() would break
+        the contract of corresponding_column() into an explicit option,
+        fixing the implicit behavior.
+
+        """
         table1 = table("table1", column("col1"))
 
         s1 = select(table1.c.col1).subquery()
-        t1 = s1._annotate({})
-        t2 = s1
+
+        expect_same = True
+
+        if testcase == "plain":
+            t1 = s1
+        elif testcase == "annotated":
+            t1 = s1._annotate({})
+        elif testcase == "deep_annotated":
+            # was failing prior to #8796
+            t1 = sql_util._deep_annotate(s1, {"foo": "bar"})
+        elif testcase == "deep_annotated_w_ind_col":
+            # was implicit behavior w/ annotate prior to #8796
+            t1 = sql_util._deep_annotate(
+                s1, {"foo": "bar"}, ind_cols_on_fromclause=True
+            )
+            expect_same = False
+        else:
+            assert False
 
         # t1 needs to share the same _make_proxy() columns as t2, even
         # though it's annotated.  otherwise paths will diverge once they
         # are corresponded against "inner" below.
 
-        assert t1.c is t2.c
-        assert t1.c.col1 is t2.c.col1
+        if expect_same:
+            assert t1.c is s1.c
+            assert t1.c.col1 is s1.c.col1
+        else:
+            assert t1.c is not s1.c
+            assert t1.c.col1 is not s1.c.col1
 
         inner = select(s1).subquery()
 
-        assert (
-            inner.corresponding_column(t2.c.col1, require_embedded=False)
-            is inner.corresponding_column(t2.c.col1, require_embedded=True)
-            is inner.c.col1
-        )
         assert (
             inner.corresponding_column(t1.c.col1, require_embedded=False)
-            is inner.corresponding_column(t1.c.col1, require_embedded=True)
             is inner.c.col1
         )
 
+        if expect_same:
+            assert (
+                inner.corresponding_column(t1.c.col1, require_embedded=True)
+                is inner.c.col1
+            )
+        else:
+            assert (
+                inner.corresponding_column(t1.c.col1, require_embedded=True)
+                is not inner.c.col1
+            )
+
     def test_annotated_visit(self):
         table1 = table("table1", column("col1"), column("col2"))
 
-- 
cgit v1.2.1