diff options
| author | Mike Bayer <mike_mp@zzzcomputing.com> | 2022-11-13 11:49:43 -0500 |
|---|---|---|
| committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2022-11-15 13:46:02 -0500 |
| commit | 93dc7ea1502c37793011b094447641361aff5aba (patch) | |
| tree | 2a443b0b902777771e6b18a499e6527de31ee729 /test/sql | |
| parent | 5fc22d0e645cd560db43fb7fd5072ecbab06128b (diff) | |
| download | sqlalchemy-93dc7ea1502c37793011b094447641361aff5aba.tar.gz | |
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
Diffstat (limited to 'test/sql')
| -rw-r--r-- | test/sql/test_selectable.py | 64 |
1 files changed, 53 insertions, 11 deletions
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")) |
