Commentary; run criteria.params() if statement isn't cached?

Considering adjustment to 56f9c7743e9083add69a10501a503f,
if statement is not cached, skip the relatively expensive
step of re-processing the criteria clause.

However, this causes the overall cache key of the statement
to come out differently which should also be avoided.

Likely we would not merge the actual change, just the
comment here.

References: #6139
Change-Id: Idb555b78d8d7950d084315e004448f64cf59bb5c

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 8fa79bfdb567782eb2a0ebd0085b5783d3333cd6..4827b375243ce0fd5fa45a4e2c7621bfbd5cf986 100644 (file)
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -134,6 +134,19 @@ class Load(Generative, LoaderOption):
         orig_query = context.compile_state.select_statement
         current_query = context.query
 
+        # NOTE: while it seems like we should not do the "apply" operation
+        # here if orig_query is current_query, skipping it in the "optimized"
+        # case causes the query to be different from a cache key perspective,
+        # because we are creating a copy of the criteria which is no longer
+        # the same identity of the _extra_criteria in the loader option
+        # itself.  cache key logic produces a different key for
+        # (A, copy_of_A) vs. (A, A), because in the latter case it shortens
+        # the second part of the key to just indicate on identity.
+
+        # if orig_query is current_query:
+        # not cached yet.   just do the and_()
+        #    return and_(*self._extra_criteria)
+
         k1 = orig_query._generate_cache_key()
         k2 = current_query._generate_cache_key()
 

diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py
index d93c63fb7efebc5ffaf7b4e6e0875a3d1f596017..04afe34779bf91ee27e941efcb8072b197d9746b 100644 (file)
--- a/test/orm/test_relationship_criteria.py
+++ b/test/orm/test_relationship_criteria.py
@@ -1043,7 +1043,12 @@ class RelationshipCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
             result = s.execute(stmt)
             return result
 
-        for value in "ed@wood.com", "ed@lala.com":
+        for value in (
+            "ed@wood.com",
+            "ed@lala.com",
+            "ed@wood.com",
+            "ed@lala.com",
+        ):
             s.close()
             with self.sql_execution_asserter() as asserter:
                 result = go(value)