rewrite _extra_criteria in selectinload; propagate correctly to Load

Fixed issue in :func:`_orm.selectinload` where use of the new
:meth:`_orm.PropComparator.and_` feature within options that were nested
more than one level deep would fail to update bound parameter values that
were in the nested criteria, as a side effect of SQL statement caching.

Implementation adds a new step that rewrites the parameters
inside of all _extra_criteria when invoking selectinload
as well as subqueryload.  Additionally, changed how Load()
gets "extra_criteria", in that it pulls it from
UnboundLoad._extra_criteria instead of re-fetching it from the
path elements, which are not updated by this new step.

This patch also builds upon the removal of lambda queries
for use in loader strategies in #6889.  lambdas made this issue
much more difficult to diagnose.  An attempt to reintroduce
lambdas here after finally identifying the "extra_criteria"
issue above showed that lambdas still impact the
assertsql fixture, meaning we have a statement structure that
upon calling .compile() still delivers stale data due to lambdas,
even if caching is turned off, and the non-cached test was still
failing due to stale data within the lambdas.

This is basically the complexity that #6889 fixes and as there's
no real performance gain to using lambdas in these strategies
on top of the existing statement caching that does most of the
work, it should be much less likely going forward to have as many
deeply confusing issues as we've had within selectinload/lazyload
in the 1.4 series.

Fixes: #6881
Change-Id: I919c079d2ed06125def5f8d6d81f3f305e158c04

1 files changed, 21 insertions, 3 deletions

diff --git a/lib/sqlalchemy/testing/assertsql.py b/lib/sqlalchemy/testing/assertsql.py
index 2d41d6dab..4ee4c5844 100644
--- a/lib/sqlalchemy/testing/assertsql.py
+++ b/lib/sqlalchemy/testing/assertsql.py
@@ -96,6 +96,15 @@ class CompiledSQL(SQLMatchRule):
         context = execute_observed.context
         compare_dialect = self._compile_dialect(execute_observed)
 
+        # received_statement runs a full compile().  we should not need to
+        # consider extracted_parameters; if we do this indicates some state
+        # is being sent from a previous cached query, which some misbehaviors
+        # in the ORM can cause, see #6881
+        cache_key = None  # execute_observed.context.compiled.cache_key
+        extracted_parameters = (
+            None  # execute_observed.context.extracted_parameters
+        )
+
         if "schema_translate_map" in context.execution_options:
             map_ = context.execution_options["schema_translate_map"]
         else:
@@ -104,10 +113,12 @@ class CompiledSQL(SQLMatchRule):
         if isinstance(execute_observed.clauseelement, _DDLCompiles):
 
             compiled = execute_observed.clauseelement.compile(
-                dialect=compare_dialect, schema_translate_map=map_
+                dialect=compare_dialect,
+                schema_translate_map=map_,
             )
         else:
             compiled = execute_observed.clauseelement.compile(
+                cache_key=cache_key,
                 dialect=compare_dialect,
                 column_keys=context.compiled.column_keys,
                 for_executemany=context.compiled.for_executemany,
@@ -117,10 +128,17 @@ class CompiledSQL(SQLMatchRule):
         parameters = execute_observed.parameters
 
         if not parameters:
-            _received_parameters = [compiled.construct_params()]
+            _received_parameters = [
+                compiled.construct_params(
+                    extracted_parameters=extracted_parameters
+                )
+            ]
         else:
             _received_parameters = [
-                compiled.construct_params(m) for m in parameters
+                compiled.construct_params(
+                    m, extracted_parameters=extracted_parameters
+                )
+                for m in parameters
             ]
 
         return _received_statement, _received_parameters