From: Mike Bayer Date: Thu, 9 Jun 2022 01:35:02 +0000 (-0400) Subject: restore parameter escaping for public methods X-Git-Tag: rel_2_0_0b1~251^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab8b4f25eebfbd2ef5819ff98dd372d5392c4b6b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git restore parameter escaping for public methods Adjusted the fix made for :ticket:`8056` which adjusted the escaping of bound parameter names with special characters such that the escaped names were translated after the SQL compilation step, which broke a published recipe on the FAQ illustrating how to merge parameter names into the string output of a compiled SQL string. The change restores the escaped names that come from ``compiled.params`` and adds a conditional parameter to :meth:`.SQLCompiler.construct_params` named ``escape_names`` that defaults to ``True``, restoring the old behavior by default. Fixes: #8113 Change-Id: I9cbedb1080bc06d51f287fd2cbf26aaab1c74653 --- diff --git a/doc/build/changelog/unreleased_14/8113.rst b/doc/build/changelog/unreleased_14/8113.rst new file mode 100644 index 0000000000..100f9a731f --- /dev/null +++ b/doc/build/changelog/unreleased_14/8113.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, sql + :tickets: 8113 + + Adjusted the fix made for :ticket:`8056` which adjusted the escaping of + bound parameter names with special characters such that the escaped names + were translated after the SQL compilation step, which broke a published + recipe on the FAQ illustrating how to merge parameter names into the string + output of a compiled SQL string. The change restores the escaped names that + come from ``compiled.params`` and adds a conditional parameter to + :meth:`.SQLCompiler.construct_params` named ``escape_names`` that defaults + to ``True``, restoring the old behavior by default. diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index 6b76601ffe..df35e7128c 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -943,13 +943,15 @@ class DefaultExecutionContext(ExecutionContext): if not parameters: self.compiled_parameters = [ compiled.construct_params( - extracted_parameters=extracted_parameters + extracted_parameters=extracted_parameters, + escape_names=False, ) ] else: self.compiled_parameters = [ compiled.construct_params( m, + escape_names=False, _group_number=grp, extracted_parameters=extracted_parameters, ) diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 78c6af38ba..8ce0c65e42 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -633,6 +633,7 @@ class Compiled: self, params: Optional[_CoreSingleExecuteParams] = None, extracted_parameters: Optional[Sequence[BindParameter[Any]]] = None, + escape_names: bool = True, ) -> Optional[_MutableCoreSingleExecuteParams]: """Return the bind params for this compiled object. @@ -1176,11 +1177,14 @@ class SQLCompiler(Compiled): self, params: Optional[_CoreSingleExecuteParams] = None, extracted_parameters: Optional[Sequence[BindParameter[Any]]] = None, + escape_names: bool = True, _group_number: Optional[int] = None, _check: bool = True, ) -> _MutableCoreSingleExecuteParams: """return a dictionary of bind parameter keys and values""" + has_escaped_names = escape_names and bool(self.escaped_bind_names) + if extracted_parameters: # related the bound parameters collected in the original cache key # to those collected in the incoming cache key. They will not have @@ -1210,10 +1214,16 @@ class SQLCompiler(Compiled): if params: pd = {} for bindparam, name in self.bind_names.items(): + escaped_name = ( + self.escaped_bind_names.get(name, name) + if has_escaped_names + else name + ) + if bindparam.key in params: - pd[name] = params[bindparam.key] + pd[escaped_name] = params[bindparam.key] elif name in params: - pd[name] = params[name] + pd[escaped_name] = params[name] elif _check and bindparam.required: if _group_number: @@ -1238,13 +1248,19 @@ class SQLCompiler(Compiled): value_param = bindparam if bindparam.callable: - pd[name] = value_param.effective_value + pd[escaped_name] = value_param.effective_value else: - pd[name] = value_param.value + pd[escaped_name] = value_param.value return pd else: pd = {} for bindparam, name in self.bind_names.items(): + escaped_name = ( + self.escaped_bind_names.get(name, name) + if has_escaped_names + else name + ) + if _check and bindparam.required: if _group_number: raise exc.InvalidRequestError( @@ -1266,9 +1282,9 @@ class SQLCompiler(Compiled): value_param = bindparam if bindparam.callable: - pd[name] = value_param.effective_value + pd[escaped_name] = value_param.effective_value else: - pd[name] = value_param.value + pd[escaped_name] = value_param.value return pd @util.memoized_instancemethod @@ -1342,7 +1358,7 @@ class SQLCompiler(Compiled): """ if parameters is None: - parameters = self.construct_params() + parameters = self.construct_params(escape_names=False) expanded_parameters = {} positiontup: Optional[List[str]] @@ -4895,6 +4911,7 @@ class DDLCompiler(Compiled): self, params: Optional[_CoreSingleExecuteParams] = None, extracted_parameters: Optional[Sequence[BindParameter[Any]]] = None, + escape_names: bool = True, ) -> Optional[_MutableCoreSingleExecuteParams]: return None diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index 94c38548f7..930f32b7bf 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -3752,10 +3752,14 @@ class BindParameterTest(AssertsCompiledSQL, fixtures.TestBase): """general bind param escape unit tests added as a result of #8053. - However, note that the final application of an escaped param name + The final application of an escaped param name was moved out of compiler and into DefaultExecutionContext in related issue #8056. + However in #8113 we made this conditional to suit usage recipes + posted in the FAQ. + + """ SomeEnum = pep435_enum("SomeEnum") @@ -3788,14 +3792,33 @@ class BindParameterTest(AssertsCompiledSQL, fixtures.TestBase): compiled = t.insert().compile( dialect=dialect, compile_kwargs=dict(compile_keys=("_id", "_data")) ) - params = compiled.construct_params({"_id": 1, "_data": one}) + # not escaped + params = compiled.construct_params( + {"_id": 1, "_data": one}, escape_names=False + ) eq_(params, {"_id": 1, "_data": one}) + + # escaped by default + params = compiled.construct_params({"_id": 1, "_data": one}) + eq_(params, {'"_id"': 1, '"_data"': one}) + + # escaped here as well + eq_(compiled.params, {'"_data"': None, '"_id"': None}) + + # bind processors aren't part of this eq_(compiled._bind_processors, {"_data": mock.ANY}) - # previously, this was: - # eq_(params, {'"_id"': 1, '"_data"': one}) - # eq_(compiled._bind_processors, {'"_data"': mock.ANY}) + dialect.paramstyle = "pyformat" + compiled = t.insert().compile( + dialect=dialect, compile_kwargs=dict(compile_keys=("_id", "_data")) + ) + + # FAQ recipe works + eq_( + compiled.string % compiled.params, + "INSERT INTO t (_id, _data) VALUES (None, None)", + ) def test_expanding_non_expanding_conflict(self): """test #8018"""