Merge "Use explicit names for mapper _get_clause parameters"

6 files changed, 30 insertions, 8 deletions

diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 24751bf1d..4a90caeff 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -436,6 +436,14 @@ def load_on_pk_identity(
                 lambda q: q.where(
                     sql_util._deep_annotate(_get_clause, {"_orm_adapt": True})
                 ),
+                # this track_on will allow the lambda to refresh if
+                # _get_clause goes stale due to reconfigured mapper.
+                # however, it's not needed as the lambda otherwise tracks
+                # on the SQL cache key of the expression.  the main thing
+                # is that the bindparam.key stays the same if the cache key
+                # stays the same, as we are referring to the .key explicitly
+                # in the params.
+                # track_on=[id(_get_clause)]
             )
         else:
             q._where_criteria = (
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index fe425301a..93f4c87a1 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2569,8 +2569,11 @@ class Mapper(
 
         """
         params = [
-            (primary_key, sql.bindparam(None, type_=primary_key.type))
-            for primary_key in self.primary_key
+            (
+                primary_key,
+                sql.bindparam("pk_%d" % idx, type_=primary_key.type),
+            )
+            for idx, primary_key in enumerate(self.primary_key, 1)
         ]
         return (
             sql.and_(*[k == v for (k, v) in params]),
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 51f75baf3..339a5bcf1 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -672,6 +672,7 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
             and self.entity._get_clause[0].compare(
                 self._lazywhere,
                 use_proxies=True,
+                compare_keys=False,
                 equivalents=self.mapper._equivalent_columns,
             )
         )
@@ -2535,6 +2536,7 @@ class SelectInLoader(PostLoader, util.MemoizedSlots):
                 self.omit_join = self.parent._get_clause[0].compare(
                     lazyloader._rev_lazywhere,
                     use_proxies=True,
+                    compare_keys=False,
                     equivalents=self.parent._equivalent_columns,
                 )
 
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index b26918f2f..29023c9fe 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -1400,14 +1400,14 @@ class BindParameter(roles.InElementRole, ColumnElement):
         else:
             self.type = type_
 
-    def _with_value(self, value, maintain_key=False):
+    def _with_value(self, value, maintain_key=False, required=NO_ARG):
         """Return a copy of this :class:`.BindParameter` with the given value
         set.
         """
         cloned = self._clone(maintain_key=maintain_key)
         cloned.value = value
         cloned.callable = None
-        cloned.required = False
+        cloned.required = required if required is not NO_ARG else self.required
         if cloned.type is type_api.NULLTYPE:
             cloned.type = type_api._resolve_value_to_type(value)
         return cloned
@@ -1826,7 +1826,7 @@ class TextClause(
                     replace_context=err,
                 )
             else:
-                new_params[key] = existing._with_value(value)
+                new_params[key] = existing._with_value(value, required=False)
 
     @util.preload_module("sqlalchemy.sql.selectable")
     def columns(self, *cols, **types):
diff --git a/lib/sqlalchemy/sql/lambdas.py b/lib/sqlalchemy/sql/lambdas.py
index 2ffed2788..2b77b8743 100644
--- a/lib/sqlalchemy/sql/lambdas.py
+++ b/lib/sqlalchemy/sql/lambdas.py
@@ -1170,7 +1170,9 @@ class PyWrapper(ColumnOperators):
         to_evaluate = object.__getattribute__(self, "_to_evaluate")
         if param is None:
             name = object.__getattribute__(self, "_name")
-            self._param = param = elements.BindParameter(name, unique=True)
+            self._param = param = elements.BindParameter(
+                name, required=False, unique=True
+            )
             self._has_param = True
             param.type = type_api._resolve_value_to_type(to_evaluate)
         return param._with_value(to_evaluate, maintain_key=True)
diff --git a/lib/sqlalchemy/sql/traversals.py b/lib/sqlalchemy/sql/traversals.py
index 51a531000..3849749de 100644
--- a/lib/sqlalchemy/sql/traversals.py
+++ b/lib/sqlalchemy/sql/traversals.py
@@ -1370,12 +1370,19 @@ class TraversalComparatorStrategy(InternalTraversal, util.MemoizedSlots):
             return COMPARE_FAILED
 
     def compare_bindparam(self, left, right, **kw):
+        compare_keys = kw.pop("compare_keys", True)
         compare_values = kw.pop("compare_values", True)
+
         if compare_values:
-            return []
+            omit = []
         else:
             # this means, "skip these, we already compared"
-            return ["callable", "value"]
+            omit = ["callable", "value"]
+
+        if not compare_keys:
+            omit.append("key")
+
+        return omit
 
 
 class ColIdentityComparatorStrategy(TraversalComparatorStrategy):