dont pass empty sequences to connection.execute()

Fixed internal issue where Alembic would call ``connection.execute()``
sending an empty tuple to indicate "no params".  In SQLAlchemy 2.1 this
case will be deprecated as "empty sequence" is ambiguous as to its intent.

Fixes: #1394
Change-Id: If3105866a13f4e3ffdcd513de3f970257ea48089

diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py
index 2e4f1ae9405eac6c755f3f4f3957efa717ecd8da..bf202a48c62c9fa33c145a407a8b4b6afe06466b 100644 (file)
--- a/alembic/ddl/impl.py
+++ b/alembic/ddl/impl.py
@@ -168,16 +168,15 @@ class DefaultImpl(metaclass=ImplMeta):
     def _exec(
         self,
         construct: Union[Executable, str],
-        execution_options: Optional[dict[str, Any]] = None,
-        multiparams: Sequence[dict] = (),
-        params: Dict[str, Any] = util.immutabledict(),
+        execution_options: Optional[Mapping[str, Any]] = None,
+        multiparams: Optional[Sequence[Mapping[str, Any]]] = None,
+        params: Mapping[str, Any] = util.immutabledict(),
     ) -> Optional[CursorResult]:
         if isinstance(construct, str):
             construct = text(construct)
         if self.as_sql:
-            if multiparams or params:
-                # TODO: coverage
-                raise Exception("Execution arguments not allowed with as_sql")
+            if multiparams is not None or params:
+                raise TypeError("SQL parameters not allowed with as_sql")
 
             compile_kw: dict[str, Any]
             if self.literal_binds and not isinstance(
@@ -200,11 +199,16 @@ class DefaultImpl(metaclass=ImplMeta):
             assert conn is not None
             if execution_options:
                 conn = conn.execution_options(**execution_options)
-            if params:
-                assert isinstance(multiparams, tuple)
-                multiparams += (params,)
 
-            return conn.execute(construct, multiparams)
+            if params and multiparams is not None:
+                raise TypeError(
+                    "Can't send params and multiparams at the same time"
+                )
+
+            if multiparams:
+                return conn.execute(construct, multiparams)
+            else:
+                return conn.execute(construct, params)
 
     def execute(
         self,

diff --git a/alembic/testing/fixtures.py b/alembic/testing/fixtures.py
index 4b83a745f3f5dbafbd0a0c073635122f888e797b..b6cea632e9047db4c51eaefe08eb890081a049ef 100644 (file)
--- a/alembic/testing/fixtures.py
+++ b/alembic/testing/fixtures.py
@@ -49,6 +49,12 @@ class TestBase(SQLAlchemyTestBase):
             connection, opts=dict(transaction_per_migration=True)
         )
 
+    @testing.fixture
+    def as_sql_migration_context(self, connection):
+        return MigrationContext.configure(
+            connection, opts=dict(transaction_per_migration=True, as_sql=True)
+        )
+
     @testing.fixture
     def connection(self):
         with config.db.connect() as conn:

diff --git a/docs/build/unreleased/1394.rst b/docs/build/unreleased/1394.rst
new file mode 100644 (file)
index 0000000..7e59770
--- /dev/null
+++ b/docs/build/unreleased/1394.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, execution
+    :tickets: 1394
+
+    Fixed internal issue where Alembic would call ``connection.execute()``
+    sending an empty tuple to indicate "no params".  In SQLAlchemy 2.1 this
+    case will be deprecated as "empty sequence" is ambiguous as to its intent.
+

diff --git a/tests/test_impl.py b/tests/test_impl.py
index 8a73b87d0791a395db1bb90f22ff107008e57b8c..ffd5f61c08de8ee04599b1283075ec9fe8587d79 100644 (file)
--- a/tests/test_impl.py
+++ b/tests/test_impl.py
@@ -23,6 +23,11 @@ class ImplTest(TablesTest):
         with migration_context.begin_transaction(_per_migration=True):
             yield migration_context.impl
 
+    @testing.fixture
+    def as_sql_impl(self, as_sql_migration_context):
+        with as_sql_migration_context.begin_transaction(_per_migration=True):
+            yield as_sql_migration_context.impl
+
     def test_execute_params(self, impl):
         result = impl._exec(text("select :my_param"), params={"my_param": 5})
         eq_(result.scalar(), 5)
@@ -40,6 +45,28 @@ class ImplTest(TablesTest):
             [(1, 2), (2, 3), (5, 7)],
         )
 
+    def test_dont_send_both(self, impl):
+        with testing.expect_raises_message(
+            TypeError, "Can't send params and multiparams at the same time"
+        ):
+            impl._exec(
+                text("select :my_param"),
+                params={"my_param": 5},
+                multiparams=[],
+            )
+
+    def test_no_params_w_as_sql(self, as_sql_impl):
+        with testing.expect_raises_message(
+            TypeError, "SQL parameters not allowed with as_sql"
+        ):
+            as_sql_impl._exec(text("select :my_param"), params={"my_param": 5})
+
+    def test_no_multiparams_w_as_sql(self, as_sql_impl):
+        with testing.expect_raises_message(
+            TypeError, "SQL parameters not allowed with as_sql"
+        ):
+            as_sql_impl._exec(text("select :my_param"), multiparams=[])
+
 
 class FutureImplTest(FutureEngineMixin, ImplTest):
     pass