Skip to content

Commit

Permalink
dont pass empty sequences to connection.execute()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zzzeek committed Jan 10, 2024
1 parent abc8002 commit f24a644
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 10 deletions.
24 changes: 14 additions & 10 deletions alembic/ddl/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,15 @@ def bind(self) -> Optional[Connection]:
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(
Expand All @@ -200,11 +199,16 @@ def _exec(
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,
Expand Down
6 changes: 6 additions & 0 deletions alembic/testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ def migration_context(self, connection):
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:
Expand Down
8 changes: 8 additions & 0 deletions docs/build/unreleased/1394.rst
Original file line number Diff line number Diff line change
@@ -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.

27 changes: 27 additions & 0 deletions tests/test_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ def impl(self, migration_context):
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)
Expand All @@ -40,6 +45,28 @@ def test_execute_multiparams(self, impl):
[(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

0 comments on commit f24a644

Please sign in to comment.