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

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/engine/_util_cy.py b/lib/sqlalchemy/engine/_util_cy.py
index 6c45b22ef67e176755f62c0970d12f9e3d765940..dd56c65d2a81f1dbf7df3728db1bfb41256faf84 100644 (file)
--- a/lib/sqlalchemy/engine/_util_cy.py
+++ b/lib/sqlalchemy/engine/_util_cy.py
@@ -57,7 +57,17 @@ def _is_mapping_or_tuple(value: object, /) -> cython.bint:
     )
 
 
-# _is_mapping_or_tuple could be inlined if pure python perf is a problem
+@cython.inline
+@cython.cfunc
+def _is_mapping(value: object, /) -> cython.bint:
+    return (
+        isinstance(value, dict)
+        or isinstance(value, Mapping)
+        # only do immutabledict or abc.__instancecheck__ for Mapping after
+        # we've checked for plain dictionaries and would otherwise raise
+    )
+
+
 def _distill_params_20(
     params: Optional[_CoreAnyExecuteParams],
 ) -> _CoreMultiExecuteParams:
@@ -73,19 +83,18 @@ def _distill_params_20(
                 "future SQLAlchemy release",
                 "2.1",
             )
-        elif not _is_mapping_or_tuple(params[0]):
+        elif not _is_mapping(params[0]):
             raise exc.ArgumentError(
-                "List argument must consist only of tuples or dictionaries"
+                "List argument must consist only of dictionaries"
             )
         return params
-    elif isinstance(params, dict) or isinstance(params, Mapping):
-        # only do immutabledict or abc.__instancecheck__ for Mapping after
-        # we've checked for plain dictionaries and would otherwise raise
-        return [params]
+    elif _is_mapping(params):
+        return [params]  # type: ignore[list-item]
     else:
         raise exc.ArgumentError("mapping or list expected for parameters")
 
 
+# _is_mapping_or_tuple could be inlined if pure python perf is a problem
 def _distill_raw_params(
     params: Optional[_DBAPIAnyExecuteParams],
 ) -> _DBAPIMultiExecuteParams:

diff --git a/test/engine/test_processors.py b/test/engine/test_processors.py
index cdb518c969b8e09a6049c440ed74d1d6323fb1ad..e43f642b4468f44df4c93c79dc74754d067b97c9 100644 (file)
--- a/test/engine/test_processors.py
+++ b/test/engine/test_processors.py
@@ -153,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_(
@@ -171,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/misc.py b/test/perf/compiled_extensions/misc.py
index d051cca0b78636d2bcc55499bc6ab064b8a87bdc..2ab6782e8694b5218f66b22eaff4d4026a9423c1 100644 (file)
--- a/test/perf/compiled_extensions/misc.py
+++ b/test/perf/compiled_extensions/misc.py
@@ -138,26 +138,22 @@ class DistillParam(Case):
     def none_20(self):
         self.impl._distill_params_20(None)
 
-    @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)
@@ -177,16 +173,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 AnonMap(Case):
     NUMBER = 5_000_000