From 5f9c45400556f821550e7a39331f1bd5af5a34ce Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 17 Mar 2021 11:04:58 -0400 Subject: Use explicit names for mapper _get_clause parameters Fixed a critical regression in the relationship lazy loader where the SQL criteria used to fetch a related many-to-one object could go stale in relation to other memoized structures within the loader if the mapper had configuration changes, such as can occur when mappers are late configured or configured on demand, producing a comparison to None and returning no object. Huge thanks to Alan Hamlett for their help tracking this down late into the night. The primary change is that mapper._get_clause() uses a fixed name for its bound parameters, which is memoized under a lambda statement in the case of many-to-one lazy loading. This has implications for some other logic namely the .compare() used by loader strategies to determine use_get needed to be adjusted. This change also repairs the lambda module's behavior of removing the "required" flag from bound parameters, which caused this issue to also fail silently rather than issuing an error for a required bind parameter. Fixes: #6055 Change-Id: I19e1aba9207a049873e0f13c19bad7541e223cfd --- lib/sqlalchemy/sql/elements.py | 6 +++--- lib/sqlalchemy/sql/lambdas.py | 4 +++- lib/sqlalchemy/sql/traversals.py | 11 +++++++++-- 3 files changed, 15 insertions(+), 6 deletions(-) (limited to 'lib/sqlalchemy/sql') 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): -- cgit v1.2.1