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 (file)
index 0000000..c276b85
--- /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 cb17acd69c08eb0c4d18bcb22a2979443fb2c170..68e4f9f436be3b39dbad20e1f23484698cf3eea2 100644 (file)
--- 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 50badea2a9482232462bfbdbf0a6f0204e1dcde6..c717660735200600fd3b745c6ea8264f41ca2a34 100644 (file)
--- 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 5f28e3ea0ef59ee36ec8ee2fd47cee46713d8f0d..4f06f772add5c001b5347c05b11b100c53334880 100644 (file)
--- 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 0982d96ea7bde02a08f8d7964a61df02ba0caaed..8f8f4cf1e006767d95b79468bafcb97f016aa4be 100644 (file)
--- 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