From: Federico Caselli Date: Thu, 5 Jun 2025 18:09:32 +0000 (+0200) Subject: remove support for list of tuples in the normal execute X-Git-Tag: rel_2_0_42~23 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d079ab21e496d654c76da16348c72eef3d258393;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git remove support for list of tuples in the normal execute The function that validates the arguments in the normal execute flow allowed by mistake list of tuples, that are not supported by the code since the 2.0 series. Change-Id: Ia401b0e19e72ed33b7d3d5718578cbed0d214c2a (cherry picked from commit 7a96d2792dd7a65b8cc3a8af72d423c2c382b11d) --- diff --git a/doc/build/changelog/unreleased_20/no_tuples.rst b/doc/build/changelog/unreleased_20/no_tuples.rst new file mode 100644 index 0000000000..c276b858df --- /dev/null +++ b/doc/build/changelog/unreleased_20/no_tuples.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: engine + + Improved validation of execution parameters passed to the + :meth:`_engine.Connection.execute` and similar methods to + provided a better error when tuples are passed in. + Previously the execution would fail with a difficult to + understand error message. diff --git a/lib/sqlalchemy/cyextension/util.pyx b/lib/sqlalchemy/cyextension/util.pyx index cb17acd69c..68e4f9f436 100644 --- a/lib/sqlalchemy/cyextension/util.pyx +++ b/lib/sqlalchemy/cyextension/util.pyx @@ -10,28 +10,24 @@ from sqlalchemy import exc cdef tuple _Empty_Tuple = () -cdef inline bint _mapping_or_tuple(object value): +cdef inline bint _is_mapping_or_tuple(object value): return isinstance(value, dict) or isinstance(value, tuple) or isinstance(value, Mapping) -cdef inline bint _check_item(object params) except 0: - cdef object item - cdef bint ret = 1 - if params: - item = params[0] - if not _mapping_or_tuple(item): - ret = 0 - raise exc.ArgumentError( - "List argument must consist only of tuples or dictionaries" - ) - return ret + +cdef inline bint _is_mapping(object value): + return isinstance(value, dict) or isinstance(value, Mapping) + def _distill_params_20(object params): if params is None: return _Empty_Tuple elif isinstance(params, list) or isinstance(params, tuple): - _check_item(params) + if params and not _is_mapping(params[0]): + raise exc.ArgumentError( + "List argument must consist only of dictionaries" + ) return params - elif isinstance(params, dict) or isinstance(params, Mapping): + elif _is_mapping(params): return [params] else: raise exc.ArgumentError("mapping or list expected for parameters") @@ -41,9 +37,12 @@ def _distill_raw_params(object params): if params is None: return _Empty_Tuple elif isinstance(params, list): - _check_item(params) + if params and not _is_mapping_or_tuple(params[0]): + raise exc.ArgumentError( + "List argument must consist only of tuples or dictionaries" + ) return params - elif _mapping_or_tuple(params): + elif _is_mapping_or_tuple(params): return [params] else: raise exc.ArgumentError("mapping or sequence expected for parameters") diff --git a/lib/sqlalchemy/engine/_py_util.py b/lib/sqlalchemy/engine/_py_util.py index 50badea2a9..c717660735 100644 --- a/lib/sqlalchemy/engine/_py_util.py +++ b/lib/sqlalchemy/engine/_py_util.py @@ -32,9 +32,9 @@ def _distill_params_20( # Assume list is more likely than tuple elif isinstance(params, list) or isinstance(params, tuple): # collections_abc.MutableSequence): # avoid abc.__instancecheck__ - if params and not isinstance(params[0], (tuple, Mapping)): + if params and not isinstance(params[0], Mapping): raise exc.ArgumentError( - "List argument must consist only of tuples or dictionaries" + "List argument must consist only of dictionaries" ) return params diff --git a/test/engine/test_processors.py b/test/engine/test_processors.py index 5f28e3ea0e..4f06f772ad 100644 --- a/test/engine/test_processors.py +++ b/test/engine/test_processors.py @@ -5,6 +5,7 @@ from types import MappingProxyType from sqlalchemy import exc from sqlalchemy.engine import processors from sqlalchemy.testing import assert_raises_message +from sqlalchemy.testing import combinations from sqlalchemy.testing import eq_ from sqlalchemy.testing import expect_raises_message from sqlalchemy.testing import fixtures @@ -152,13 +153,6 @@ class _DistillArgsTest(fixtures.TestBase): eq_(self.module._distill_params_20(()), ()) eq_(self.module._distill_params_20([]), []) - def test_distill_20_sequence_sequence(self): - eq_(self.module._distill_params_20(((1, 2, 3),)), ((1, 2, 3),)) - eq_(self.module._distill_params_20([(1, 2, 3)]), [(1, 2, 3)]) - - eq_(self.module._distill_params_20(((1, 2), (2, 3))), ((1, 2), (2, 3))) - eq_(self.module._distill_params_20([(1, 2), (2, 3)]), [(1, 2), (2, 3)]) - def test_distill_20_sequence_dict(self): eq_(self.module._distill_params_20(({"a": 1},)), ({"a": 1},)) eq_( @@ -170,27 +164,23 @@ class _DistillArgsTest(fixtures.TestBase): (MappingProxyType({"a": 1}),), ) - def test_distill_20_sequence_error(self): - with expect_raises_message( - exc.ArgumentError, - "List argument must consist only of tuples or dictionaries", - ): - self.module._distill_params_20((1, 2, 3)) + @combinations( + [(1, 2, 3)], + [([1, 2, 3],)], + [[1, 2, 3]], + [["a", "b"]], + [((1, 2, 3),)], + [[(1, 2, 3)]], + [((1, 2), (2, 3))], + [[(1, 2), (2, 3)]], + argnames="arg", + ) + def test_distill_20_sequence_error(self, arg): with expect_raises_message( exc.ArgumentError, - "List argument must consist only of tuples or dictionaries", - ): - self.module._distill_params_20(([1, 2, 3],)) - with expect_raises_message( - exc.ArgumentError, - "List argument must consist only of tuples or dictionaries", - ): - self.module._distill_params_20([1, 2, 3]) - with expect_raises_message( - exc.ArgumentError, - "List argument must consist only of tuples or dictionaries", + "List argument must consist only of dictionaries", ): - self.module._distill_params_20(["a", "b"]) + self.module._distill_params_20(arg) def test_distill_20_dict(self): eq_(self.module._distill_params_20({"foo": "bar"}), [{"foo": "bar"}]) diff --git a/test/perf/compiled_extensions.py b/test/perf/compiled_extensions.py index 0982d96ea7..8f8f4cf1e0 100644 --- a/test/perf/compiled_extensions.py +++ b/test/perf/compiled_extensions.py @@ -401,26 +401,22 @@ class DistillParam(Case): self.impl._distill_params_20(()) self.impl._distill_params_20([]) - @test_case - def list_20(self): - self.impl._distill_params_20(self.list_tup) - - @test_case - def tuple_20(self): - self.impl._distill_params_20(self.tup_tup) - @test_case def list_dict_20(self): - self.impl._distill_params_20(self.list_tup) + self.impl._distill_params_20(self.list_dic) @test_case def tuple_dict_20(self): - self.impl._distill_params_20(self.dict) + self.impl._distill_params_20(self.tup_dic) @test_case def mapping_20(self): self.impl._distill_params_20(self.mapping) + @test_case + def dict_20(self): + self.impl._distill_params_20(self.dict) + @test_case def raw_none(self): self.impl._distill_raw_params(None) @@ -440,16 +436,20 @@ class DistillParam(Case): @test_case def raw_list_dict(self): - self.impl._distill_raw_params(self.list_tup) + self.impl._distill_raw_params(self.list_dic) @test_case def raw_tuple_dict(self): - self.impl._distill_raw_params(self.dict) + self.impl._distill_raw_params(self.tup_dic) @test_case def raw_mapping(self): self.impl._distill_raw_params(self.mapping) + @test_case + def raw_dict(self): + self.impl._distill_raw_params(self.mapping) + class IdentitySet(Case): @staticmethod