From: Mike Bayer Date: Wed, 17 Mar 2021 15:04:58 +0000 (-0400) Subject: Use explicit names for mapper _get_clause parameters X-Git-Tag: rel_1_4_1~3^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5f9c45400556f821550e7a39331f1bd5af5a34ce;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 --- diff --git a/doc/build/changelog/unreleased_14/6055.rst b/doc/build/changelog/unreleased_14/6055.rst new file mode 100644 index 0000000000..77ff15eb92 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6055.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 6055 + + 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. + + diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py index 24751bf1d4..4a90caeff7 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 fe425301a0..93f4c87a17 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 51f75baf37..339a5bcf15 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 b26918f2fe..29023c9feb 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 2ffed27888..2b77b8743f 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 51a5310009..3849749deb 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): diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py index 2d4e9848e5..d62e5c0bca 100644 --- a/test/ext/test_baked.py +++ b/test/ext/test_baked.py @@ -348,7 +348,18 @@ class LikeQueryTest(BakedTest): u1 = bq(sess).get(7) eq_(u1.name, "jack") sess.close() - eq_(len(bq._bakery), 4) + + # this went from 4 to 3 as a result of #6055. by giving a name + # to the bind param in mapper._get_clause, while the baked cache + # here grows by one element, the SQL compiled_cache no longer + # changes because the keys of the bindparam() objects are passed + # explicitly as params to the execute() call as a result of + # _load_on_pk_identity() (either the one in baked or the one in + # loading.py), which then puts them + # in column_keys which makes them part of the cache key. These + # were previously anon names, now they are explicit so they + # stay across resets + eq_(len(bq._bakery), 3) class ResultPostCriteriaTest(BakedTest): diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index d145fef79b..8769d2b390 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -1616,8 +1616,8 @@ class PassiveDeletesTest(fixtures.MappedTest): asserter.assert_( CompiledSQL( "SELECT a.id AS a_id, a.type AS a_type " - "FROM a WHERE a.id = :param_1", - [{"param_1": 1}], + "FROM a WHERE a.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL("DELETE FROM a WHERE a.id = :id", [{"id": 1}]), ) @@ -1658,8 +1658,8 @@ class PassiveDeletesTest(fixtures.MappedTest): asserter.assert_( CompiledSQL( "SELECT a.id AS a_id, a.type AS a_type " - "FROM a WHERE a.id = :param_1", - [{"param_1": 1}], + "FROM a WHERE a.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL("DELETE FROM a WHERE a.id = :id", [{"id": 1}]), ) @@ -1696,8 +1696,8 @@ class PassiveDeletesTest(fixtures.MappedTest): asserter.assert_( CompiledSQL( "SELECT a.id AS a_id, a.type AS a_type " - "FROM a WHERE a.id = :param_1", - [{"param_1": 1}], + "FROM a WHERE a.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL("DELETE FROM a WHERE a.id = :id", [{"id": 1}]), ) @@ -2549,8 +2549,8 @@ class OptimizedLoadTest(fixtures.MappedTest): "base.data AS base_data, base.type AS base_type, " "sub.sub AS sub_sub, sub.subcounter2 AS sub_subcounter2 " "FROM base LEFT OUTER JOIN sub ON base.id = sub.id " - "WHERE base.id = :param_1", - {"param_1": sjb_id}, + "WHERE base.id = :pk_1", + {"pk_1": sjb_id}, ), ) @@ -2804,8 +2804,8 @@ class OptimizedLoadTest(fixtures.MappedTest): "SELECT base.counter AS base_counter, " "sub.subcounter AS sub_subcounter, " "sub.subcounter2 AS sub_subcounter2 FROM base JOIN sub " - "ON base.id = sub.id WHERE base.id = :param_1", - lambda ctx: {"param_1": s1.id}, + "ON base.id = sub.id WHERE base.id = :pk_1", + lambda ctx: {"pk_1": s1.id}, ), ) diff --git a/test/orm/test_defaults.py b/test/orm/test_defaults.py index 97743b5de2..abf9b8e031 100644 --- a/test/orm/test_defaults.py +++ b/test/orm/test_defaults.py @@ -311,13 +311,13 @@ class ComputedDefaultsOnUpdateTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test.bar AS test_bar FROM test " - "WHERE test.id = :param_1", - [{"param_1": 1}], + "WHERE test.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test.bar AS test_bar FROM test " - "WHERE test.id = :param_1", - [{"param_1": 2}], + "WHERE test.id = :pk_1", + [{"pk_1": 2}], ), ], ) @@ -385,13 +385,13 @@ class ComputedDefaultsOnUpdateTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test.bar AS test_bar FROM test " - "WHERE test.id = :param_1", - [{"param_1": 1}], + "WHERE test.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test.bar AS test_bar FROM test " - "WHERE test.id = :param_1", - [{"param_1": 2}], + "WHERE test.id = :pk_1", + [{"pk_1": 2}], ), ) else: @@ -402,13 +402,13 @@ class ComputedDefaultsOnUpdateTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test.bar AS test_bar FROM test " - "WHERE test.id = :param_1", - [{"param_1": 1}], + "WHERE test.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test.bar AS test_bar FROM test " - "WHERE test.id = :param_1", - [{"param_1": 2}], + "WHERE test.id = :pk_1", + [{"pk_1": 2}], ), ) diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py index 6d1cd01843..d528cb9355 100644 --- a/test/orm/test_deferred.py +++ b/test/orm/test_deferred.py @@ -76,8 +76,8 @@ class DeferredTest(AssertsCompiledSQL, _fixtures.FixtureTest): ), ( "SELECT orders.description AS orders_description " - "FROM orders WHERE orders.id = :param_1", - {"param_1": 3}, + "FROM orders WHERE orders.id = :pk_1", + {"pk_1": 3}, ), ], ) @@ -256,8 +256,8 @@ class DeferredTest(AssertsCompiledSQL, _fixtures.FixtureTest): "orders.address_id AS orders_address_id, " "orders.description AS orders_description, " "orders.isopen AS orders_isopen " - "FROM orders WHERE orders.id = :param_1", - {"param_1": 3}, + "FROM orders WHERE orders.id = :pk_1", + {"pk_1": 3}, ), ], ) @@ -386,8 +386,8 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest): ), ( "SELECT orders.user_id AS orders_user_id " - "FROM orders WHERE orders.id = :param_1", - {"param_1": 1}, + "FROM orders WHERE orders.id = :pk_1", + {"pk_1": 1}, ), ], ) diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py index 37eed391c0..fc47b91dfb 100644 --- a/test/orm/test_dynamic.py +++ b/test/orm/test_dynamic.py @@ -738,8 +738,8 @@ class UOWTest( sess.flush, CompiledSQL( "SELECT users.id AS users_id, users.name AS users_name " - "FROM users WHERE users.id = :param_1", - lambda ctx: [{"param_1": u1_id}], + "FROM users WHERE users.id = :pk_1", + lambda ctx: [{"pk_1": u1_id}], ), CompiledSQL( "INSERT INTO addresses (user_id, email_address) " @@ -772,8 +772,8 @@ class UOWTest( CompiledSQL( "SELECT addresses.id AS addresses_id, addresses.email_address " "AS addresses_email_address FROM addresses " - "WHERE addresses.id = :param_1", - lambda ctx: [{"param_1": a2_id}], + "WHERE addresses.id = :pk_1", + lambda ctx: [{"pk_1": a2_id}], ), CompiledSQL( "UPDATE addresses SET user_id=:user_id WHERE addresses.id = " @@ -782,8 +782,8 @@ class UOWTest( ), CompiledSQL( "SELECT users.id AS users_id, users.name AS users_name " - "FROM users WHERE users.id = :param_1", - lambda ctx: [{"param_1": u1_id}], + "FROM users WHERE users.id = :pk_1", + lambda ctx: [{"pk_1": u1_id}], ), ) diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index ca0508db4d..f62d3af331 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -6254,8 +6254,8 @@ class SecondaryOptionsTest(fixtures.MappedTest): "SELECT child2.id AS child2_id, base.id AS base_id, " "base.type AS base_type " "FROM base JOIN child2 ON base.id = child2.id " - "WHERE base.id = :param_1", - {"param_1": 4}, + "WHERE base.id = :pk_1", + {"pk_1": 4}, ), ) @@ -6292,8 +6292,8 @@ class SecondaryOptionsTest(fixtures.MappedTest): "SELECT child2.id AS child2_id, base.id AS base_id, " "base.type AS base_type " "FROM base JOIN child2 ON base.id = child2.id " - "WHERE base.id = :param_1", - {"param_1": 4}, + "WHERE base.id = :pk_1", + {"pk_1": 4}, ), ) @@ -6337,7 +6337,7 @@ class SecondaryOptionsTest(fixtures.MappedTest): "related_1.id AS related_1_id FROM base JOIN child2 " "ON base.id = child2.id " "LEFT OUTER JOIN related AS related_1 " - "ON base.id = related_1.id WHERE base.id = :param_1", - {"param_1": 4}, + "ON base.id = related_1.id WHERE base.id = :pk_1", + {"pk_1": 4}, ), ) diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py index 653bf22db2..154e119529 100644 --- a/test/orm/test_lazy_relations.py +++ b/test/orm/test_lazy_relations.py @@ -734,6 +734,33 @@ class LazyTest(_fixtures.FixtureTest): self.assert_sql_count(testing.db, go, 0) sa.orm.clear_mappers() + def test_use_get_lambda_key_wont_go_stale(self): + """test [ticket:6055]""" + Address, addresses, users, User = ( + self.classes.Address, + self.tables.addresses, + self.tables.users, + self.classes.User, + ) + um = mapper(User, users) + am = mapper( + Address, addresses, properties={"user": relationship(User)} + ) + + is_true(am.relationships.user._lazy_strategy.use_get) + + with fixture_session() as sess: + a1 = sess.get(Address, 2) + + eq_(a1.user.id, 8) + + um._reset_memoizations() + + with fixture_session() as sess: + a1 = sess.get(Address, 2) + + eq_(a1.user.id, 8) + def test_uses_get_compatible_types(self): """test the use_get optimization with compatible but non-identical types""" diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py index e0a76c6a0b..22f0cd9ac3 100644 --- a/test/orm/test_merge.py +++ b/test/orm/test_merge.py @@ -1852,13 +1852,13 @@ class DeferredMergeTest(fixtures.MappedTest): [ ( "SELECT book.summary AS book_summary " - "FROM book WHERE book.id = :param_1", - {"param_1": 1}, + "FROM book WHERE book.id = :pk_1", + {"pk_1": 1}, ), ( "SELECT book.excerpt AS book_excerpt " - "FROM book WHERE book.id = :param_1", - {"param_1": 1}, + "FROM book WHERE book.id = :pk_1", + {"pk_1": 1}, ), ], ) @@ -1898,13 +1898,13 @@ class DeferredMergeTest(fixtures.MappedTest): [ ( "SELECT book.summary AS book_summary " - "FROM book WHERE book.id = :param_1", - {"param_1": 1}, + "FROM book WHERE book.id = :pk_1", + {"pk_1": 1}, ), ( "SELECT book.excerpt AS book_excerpt " - "FROM book WHERE book.id = :param_1", - {"param_1": 1}, + "FROM book WHERE book.id = :pk_1", + {"pk_1": 1}, ), ], ) diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 0f0c746984..06ec19e52b 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -359,8 +359,8 @@ class RudimentaryFlushTest(UOWTest): "addresses_user_id, addresses.email_address AS " "addresses_email_address FROM addresses " "WHERE addresses.id = " - ":param_1", - lambda ctx: {"param_1": c1id}, + ":pk_1", + lambda ctx: {"pk_1": c1id}, ), CompiledSQL( "SELECT addresses.id AS addresses_id, " @@ -368,13 +368,13 @@ class RudimentaryFlushTest(UOWTest): "addresses_user_id, addresses.email_address AS " "addresses_email_address FROM addresses " "WHERE addresses.id = " - ":param_1", - lambda ctx: {"param_1": c2id}, + ":pk_1", + lambda ctx: {"pk_1": c2id}, ), CompiledSQL( "SELECT users.id AS users_id, users.name AS users_name " - "FROM users WHERE users.id = :param_1", - lambda ctx: {"param_1": pid}, + "FROM users WHERE users.id = :pk_1", + lambda ctx: {"pk_1": pid}, ), CompiledSQL( "DELETE FROM addresses WHERE addresses.id = :id", @@ -433,8 +433,8 @@ class RudimentaryFlushTest(UOWTest): "addresses_user_id, addresses.email_address AS " "addresses_email_address FROM addresses " "WHERE addresses.id = " - ":param_1", - lambda ctx: {"param_1": c1id}, + ":pk_1", + lambda ctx: {"pk_1": c1id}, ), CompiledSQL( "SELECT addresses.id AS addresses_id, " @@ -442,8 +442,8 @@ class RudimentaryFlushTest(UOWTest): "addresses_user_id, addresses.email_address AS " "addresses_email_address FROM addresses " "WHERE addresses.id = " - ":param_1", - lambda ctx: {"param_1": c2id}, + ":pk_1", + lambda ctx: {"pk_1": c2id}, ), ), CompiledSQL( @@ -497,8 +497,8 @@ class RudimentaryFlushTest(UOWTest): "addresses_user_id, addresses.email_address AS " "addresses_email_address FROM addresses " "WHERE addresses.id = " - ":param_1", - lambda ctx: {"param_1": c1id}, + ":pk_1", + lambda ctx: {"pk_1": c1id}, ), CompiledSQL( "SELECT addresses.id AS addresses_id, " @@ -506,8 +506,8 @@ class RudimentaryFlushTest(UOWTest): "addresses_user_id, addresses.email_address AS " "addresses_email_address FROM addresses " "WHERE addresses.id = " - ":param_1", - lambda ctx: {"param_1": c2id}, + ":pk_1", + lambda ctx: {"pk_1": c2id}, ), ), CompiledSQL( @@ -1265,22 +1265,22 @@ class SingleCycleTest(UOWTest): "SELECT nodes.id AS nodes_id, nodes.parent_id AS " "nodes_parent_id, " "nodes.data AS nodes_data FROM nodes " - "WHERE nodes.id = :param_1", - lambda ctx: {"param_1": pid}, + "WHERE nodes.id = :pk_1", + lambda ctx: {"pk_1": pid}, ), CompiledSQL( "SELECT nodes.id AS nodes_id, nodes.parent_id AS " "nodes_parent_id, " "nodes.data AS nodes_data FROM nodes " - "WHERE nodes.id = :param_1", - lambda ctx: {"param_1": c1id}, + "WHERE nodes.id = :pk_1", + lambda ctx: {"pk_1": c1id}, ), CompiledSQL( "SELECT nodes.id AS nodes_id, nodes.parent_id AS " "nodes_parent_id, " "nodes.data AS nodes_data FROM nodes " - "WHERE nodes.id = :param_1", - lambda ctx: {"param_1": c2id}, + "WHERE nodes.id = :pk_1", + lambda ctx: {"pk_1": c2id}, ), AllOf( CompiledSQL( @@ -2394,13 +2394,13 @@ class EagerDefaultsTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test.foo AS test_foo FROM test " - "WHERE test.id = :param_1", - [{"param_1": 1}], + "WHERE test.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test.foo AS test_foo FROM test " - "WHERE test.id = :param_1", - [{"param_1": 2}], + "WHERE test.id = :pk_1", + [{"pk_1": 2}], ), ) @@ -2457,13 +2457,13 @@ class EagerDefaultsTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test.foo AS test_foo FROM test " - "WHERE test.id = :param_1", - [{"param_1": 1}], + "WHERE test.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test.foo AS test_foo FROM test " - "WHERE test.id = :param_1", - [{"param_1": 2}], + "WHERE test.id = :pk_1", + [{"pk_1": 2}], ), ], ), @@ -2544,13 +2544,13 @@ class EagerDefaultsTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test2.bar AS test2_bar FROM test2 " - "WHERE test2.id = :param_1", - [{"param_1": 1}], + "WHERE test2.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test2.bar AS test2_bar FROM test2 " - "WHERE test2.id = :param_1", - [{"param_1": 3}], + "WHERE test2.id = :pk_1", + [{"pk_1": 3}], ), ], ), @@ -2642,18 +2642,18 @@ class EagerDefaultsTest(fixtures.MappedTest): ), CompiledSQL( "SELECT test2.bar AS test2_bar FROM test2 " - "WHERE test2.id = :param_1", - [{"param_1": 1}], + "WHERE test2.id = :pk_1", + [{"pk_1": 1}], ), CompiledSQL( "SELECT test2.bar AS test2_bar FROM test2 " - "WHERE test2.id = :param_1", - [{"param_1": 3}], + "WHERE test2.id = :pk_1", + [{"pk_1": 3}], ), CompiledSQL( "SELECT test2.bar AS test2_bar FROM test2 " - "WHERE test2.id = :param_1", - [{"param_1": 4}], + "WHERE test2.id = :pk_1", + [{"pk_1": 4}], ), ) diff --git a/test/orm/test_update_delete.py b/test/orm/test_update_delete.py index afb16f2d17..c9248b1189 100644 --- a/test/orm/test_update_delete.py +++ b/test/orm/test_update_delete.py @@ -322,8 +322,8 @@ class UpdateDeleteTest(fixtures.MappedTest): CompiledSQL( "SELECT users.age_int AS users_age_int, " "users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 4}], + "WHERE users.id = :pk_1", + [{"pk_1": 4}], ), CompiledSQL( "UPDATE users " @@ -339,8 +339,8 @@ class UpdateDeleteTest(fixtures.MappedTest): # key CompiledSQL( "SELECT users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 4}], + "WHERE users.id = :pk_1", + [{"pk_1": 4}], ), CompiledSQL( "UPDATE users SET " @@ -368,15 +368,15 @@ class UpdateDeleteTest(fixtures.MappedTest): CompiledSQL( "SELECT users.age_int AS users_age_int, " "users.id AS users_id, users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 1}], + "WHERE users.id = :pk_1", + [{"pk_1": 1}], ), # refresh jill CompiledSQL( "SELECT users.age_int AS users_age_int, " "users.id AS users_id, users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 3}], + "WHERE users.id = :pk_1", + [{"pk_1": 3}], ), ] @@ -386,8 +386,8 @@ class UpdateDeleteTest(fixtures.MappedTest): CompiledSQL( "SELECT users.age_int AS users_age_int, " "users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 4}], + "WHERE users.id = :pk_1", + [{"pk_1": 4}], ) ) asserter.assert_(*to_assert) @@ -443,15 +443,15 @@ class UpdateDeleteTest(fixtures.MappedTest): CompiledSQL( "SELECT users.age_int AS users_age_int, " "users.id AS users_id, users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 1}], + "WHERE users.id = :pk_1", + [{"pk_1": 1}], ), # refresh jill CompiledSQL( "SELECT users.age_int AS users_age_int, " "users.id AS users_id, users.name AS users_name FROM users " - "WHERE users.id = :param_1", - [{"param_1": 3}], + "WHERE users.id = :pk_1", + [{"pk_1": 3}], ), ) diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index c185c59d02..c38434c9c1 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -1413,8 +1413,8 @@ class ServerVersioningTest(fixtures.MappedTest): CompiledSQL( "SELECT version_table.version_id " "AS version_table_version_id " - "FROM version_table WHERE version_table.id = :param_1", - lambda ctx: [{"param_1": 1}], + "FROM version_table WHERE version_table.id = :pk_1", + lambda ctx: [{"pk_1": 1}], ) ) self.assert_sql_execution(testing.db, sess.flush, *statements) @@ -1458,8 +1458,8 @@ class ServerVersioningTest(fixtures.MappedTest): CompiledSQL( "SELECT version_table.version_id " "AS version_table_version_id " - "FROM version_table WHERE version_table.id = :param_1", - lambda ctx: [{"param_1": 1}], + "FROM version_table WHERE version_table.id = :pk_1", + lambda ctx: [{"pk_1": 1}], ) ) with conditional_sane_rowcount_warnings( @@ -1567,20 +1567,20 @@ class ServerVersioningTest(fixtures.MappedTest): CompiledSQL( "SELECT version_table.version_id " "AS version_table_version_id " - "FROM version_table WHERE version_table.id = :param_1", - lambda ctx: [{"param_1": 1}], + "FROM version_table WHERE version_table.id = :pk_1", + lambda ctx: [{"pk_1": 1}], ), CompiledSQL( "SELECT version_table.version_id " "AS version_table_version_id " - "FROM version_table WHERE version_table.id = :param_1", - lambda ctx: [{"param_1": 2}], + "FROM version_table WHERE version_table.id = :pk_1", + lambda ctx: [{"pk_1": 2}], ), CompiledSQL( "SELECT version_table.version_id " "AS version_table_version_id " - "FROM version_table WHERE version_table.id = :param_1", - lambda ctx: [{"param_1": 3}], + "FROM version_table WHERE version_table.id = :pk_1", + lambda ctx: [{"pk_1": 3}], ), ] ) diff --git a/test/sql/test_lambdas.py b/test/sql/test_lambdas.py index e8e4a8d2a7..fcf0241825 100644 --- a/test/sql/test_lambdas.py +++ b/test/sql/test_lambdas.py @@ -23,6 +23,7 @@ from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ from sqlalchemy.testing import ne_ +from sqlalchemy.testing.assertions import expect_raises_message from sqlalchemy.testing.assertsql import CompiledSQL from sqlalchemy.types import Boolean from sqlalchemy.types import Integer @@ -179,6 +180,29 @@ class LambdaElementTest( s5 = go(oldc1, oldc2) self.assert_compile(s5, "SELECT x WHERE y > :y_1") + def test_maintain_required_bindparam(self): + """test that the "required" flag doesn't go away for bound + parameters""" + + def go(): + col_expr = column("x") + stmt = lambdas.lambda_stmt(lambda: select(col_expr)) + stmt += lambda stmt: stmt.where(col_expr == bindparam(None)) + + return stmt + + s1 = go() + + with expect_raises_message( + exc.InvalidRequestError, "A value is required for bind parameter" + ): + s1.compile().construct_params({}) + s2 = go() + with expect_raises_message( + exc.InvalidRequestError, "A value is required for bind parameter" + ): + s2.compile().construct_params({}) + def test_stmt_lambda_w_additional_hascachekey_variants(self): def go(col_expr, q): stmt = lambdas.lambda_stmt(lambda: select(col_expr))