Skip to content

Commit

Permalink
Merge "Warn in execute when parameter is an empty list" into main
Browse files Browse the repository at this point in the history
  • Loading branch information
zzzeek authored and Gerrit Code Review committed May 15, 2024
2 parents 93cfb49 + af655e5 commit e69ca16
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 67 deletions.
8 changes: 8 additions & 0 deletions doc/build/changelog/unreleased_21/9647.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.. change::
:tags: change, engine
:tickets: 9647

An empty sequence passed to any ``execute()`` method now
raised a deprecation warning, since such an executemany
is invalid.
Pull request courtesy of Carlos Sousa.
40 changes: 19 additions & 21 deletions lib/sqlalchemy/engine/_util_cy.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
import operator
from typing import Any
from typing import Optional
from typing import Sequence
from typing import Tuple
from typing import TYPE_CHECKING

from sqlalchemy import exc
from .. import exc
from ..util import warn_deprecated

if TYPE_CHECKING:
from .interfaces import _CoreAnyExecuteParams
Expand Down Expand Up @@ -47,7 +47,7 @@ def _is_compiled() -> bool:

@cython.inline
@cython.cfunc
def _is_mapping_or_tuple(value: object) -> cython.bint:
def _is_mapping_or_tuple(value: object, /) -> cython.bint:
return (
isinstance(value, dict)
or isinstance(value, tuple)
Expand All @@ -57,22 +57,7 @@ def _is_mapping_or_tuple(value: object) -> cython.bint:
)


@cython.inline
@cython.cfunc
@cython.exceptval(0)
def _validate_execute_many_item(params: Sequence[Any]) -> cython.bint:
ret: cython.bint = 1
if len(params) > 0:
if not _is_mapping_or_tuple(params[0]):
ret = 0
raise exc.ArgumentError(
"List argument must consist only of tuples or dictionaries"
)
return ret


# _is_mapping_or_tuple and _validate_execute_many_item could be
# inlined if pure python perf is a problem
# _is_mapping_or_tuple could be inlined if pure python perf is a problem
def _distill_params_20(
params: Optional[_CoreAnyExecuteParams],
) -> _CoreMultiExecuteParams:
Expand All @@ -81,7 +66,17 @@ 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__
_validate_execute_many_item(params)
if len(params) == 0:
warn_deprecated(
"Empty parameter sequence passed to execute(). "
"This use is deprecated and will raise an exception in a "
"future SQLAlchemy release",
"2.1",
)
elif not _is_mapping_or_tuple(params[0]):
raise exc.ArgumentError(
"List argument must consist only of tuples or dictionaries"
)
return params
elif isinstance(params, dict) or isinstance(params, Mapping):
# only do immutabledict or abc.__instancecheck__ for Mapping after
Expand All @@ -98,7 +93,10 @@ def _distill_raw_params(
return _Empty_Tuple
elif isinstance(params, list):
# collections_abc.MutableSequence # avoid abc.__instancecheck__
_validate_execute_many_item(params)
if len(params) > 0 and not _is_mapping_or_tuple(params[0]):
raise exc.ArgumentError(
"List argument must consist only of tuples or dictionaries"
)
return params
elif _is_mapping_or_tuple(params):
return [params] # type: ignore[return-value]
Expand Down
2 changes: 1 addition & 1 deletion lib/sqlalchemy/orm/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -2252,7 +2252,7 @@ def _execute_internal(
)
else:
result = conn.execute(
statement, params or {}, execution_options=execution_options
statement, params, execution_options=execution_options
)

if _scalar_result:
Expand Down
2 changes: 1 addition & 1 deletion lib/sqlalchemy/testing/suite/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def insert_data(cls, connection):
],
)

def _assert_result(self, conn, select, result, params=()):
def _assert_result(self, conn, select, result, params=None):
eq_(conn.execute(select, params).fetchall(), result)

def test_plain_union(self, connection):
Expand Down
10 changes: 5 additions & 5 deletions lib/sqlalchemy/testing/suite/test_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def insert_data(cls, connection):
)

def _assert_result(
self, connection, select, result, params=(), set_=False
self, connection, select, result, params=None, set_=False
):
if set_:
query_res = connection.execute(select, params).fetchall()
Expand All @@ -214,7 +214,7 @@ def _assert_result(
else:
eq_(connection.execute(select, params).fetchall(), result)

def _assert_result_str(self, select, result, params=()):
def _assert_result_str(self, select, result, params=None):
with config.db.connect() as conn:
eq_(conn.exec_driver_sql(select, params).fetchall(), result)

Expand Down Expand Up @@ -734,7 +734,7 @@ def test_subquery(self, connection):
class JoinTest(fixtures.TablesTest):
__backend__ = True

def _assert_result(self, select, result, params=()):
def _assert_result(self, select, result, params=None):
with config.db.connect() as conn:
eq_(conn.execute(select, params).fetchall(), result)

Expand Down Expand Up @@ -856,7 +856,7 @@ def insert_data(cls, connection):
],
)

def _assert_result(self, select, result, params=()):
def _assert_result(self, select, result, params=None):
with config.db.connect() as conn:
eq_(conn.execute(select, params).fetchall(), result)

Expand Down Expand Up @@ -1121,7 +1121,7 @@ def insert_data(cls, connection):
],
)

def _assert_result(self, select, result, params=()):
def _assert_result(self, select, result, params=None):
with config.db.connect() as conn:
eq_(conn.execute(select, params).fetchall(), result)

Expand Down
12 changes: 1 addition & 11 deletions test/dialect/oracle/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
from sqlalchemy.dialects.oracle import oracledb
from sqlalchemy.sql import column
from sqlalchemy.sql.sqltypes import NullType
from sqlalchemy.testing import assert_raises_message
from sqlalchemy.testing import AssertsCompiledSQL
from sqlalchemy.testing import eq_
from sqlalchemy.testing import expect_raises_message
Expand Down Expand Up @@ -1047,16 +1046,7 @@ def go():
)
eq_(actual, self.data)

# this comes from cx_Oracle because these are raw
# cx_Oracle.Variable objects
if testing.requires.oracle5x.enabled:
assert_raises_message(
testing.db.dialect.dbapi.ProgrammingError,
"LOB variable no longer valid after subsequent fetch",
go,
)
else:
go()
go()

def test_lobs_with_convert_many_rows(self):
# even with low arraysize, lobs are fine in autoconvert
Expand Down
36 changes: 30 additions & 6 deletions test/engine/test_execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
from sqlalchemy.testing import is_false
from sqlalchemy.testing import is_not
from sqlalchemy.testing import is_true
from sqlalchemy.testing.assertions import expect_deprecated
from sqlalchemy.testing.assertsql import CompiledSQL
from sqlalchemy.testing.provision import normalize_sequence
from sqlalchemy.testing.schema import Column
Expand Down Expand Up @@ -637,14 +638,37 @@ def _go(conn):
conn.close()

def test_empty_insert(self, connection):
"""test that execute() interprets [] as a list with no params"""
"""test that execute() interprets [] as a list with no params and
warns since it has nothing to do with such an executemany.
"""
users_autoinc = self.tables.users_autoinc

connection.execute(
users_autoinc.insert().values(user_name=bindparam("name", None)),
[],
)
eq_(connection.execute(users_autoinc.select()).fetchall(), [(1, None)])
with expect_deprecated(
r"Empty parameter sequence passed to execute\(\). "
"This use is deprecated and will raise an exception in a "
"future SQLAlchemy release"
):
connection.execute(
users_autoinc.insert().values(
user_name=bindparam("name", None)
),
[],
)

eq_(len(connection.execute(users_autoinc.select()).all()), 1)

@testing.only_on("sqlite")
def test_raw_insert_with_empty_list(self, connection):
"""exec_driver_sql instead does not raise if an empty list is passed.
Let the driver do that if it wants to.
"""
conn = connection
with expect_raises_message(
tsa.exc.ProgrammingError, "Incorrect number of bindings supplied"
):
conn.exec_driver_sql(
"insert into users (user_id, user_name) values (?, ?)", []
)

@testing.only_on("sqlite")
def test_execute_compiled_favors_compiled_paramstyle(self):
Expand Down
10 changes: 8 additions & 2 deletions test/engine/test_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from sqlalchemy.testing import expect_raises_message
from sqlalchemy.testing import fixtures
from sqlalchemy.testing import is_none
from sqlalchemy.testing.assertions import expect_deprecated
from sqlalchemy.util import immutabledict


Expand Down Expand Up @@ -144,8 +145,13 @@ def test_distill_20_none(self):
eq_(self.module._distill_params_20(None), ())

def test_distill_20_empty_sequence(self):
eq_(self.module._distill_params_20(()), ())
eq_(self.module._distill_params_20([]), [])
with expect_deprecated(
r"Empty parameter sequence passed to execute\(\). "
"This use is deprecated and will raise an exception in a "
"future SQLAlchemy release"
):
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),))
Expand Down
18 changes: 18 additions & 0 deletions test/orm/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,24 @@ def test_no_string_execute(self, connection):
):
sess.scalar("select id from users where id=:id", {"id": 7})

@testing.skip_if(
"oracle", "missing SELECT keyword [SQL: INSERT INTO tbl () VALUES ()]"
)
def test_empty_list_execute(self, metadata, connection):
t = Table("tbl", metadata, Column("col", sa.Integer))
t.create(connection)
sess = Session(bind=connection)
sess.execute(t.insert(), {"col": 42})

with assertions.expect_deprecated(
r"Empty parameter sequence passed to execute\(\). "
"This use is deprecated and will raise an exception in a "
"future SQLAlchemy release"
):
sess.execute(t.insert(), [])

eq_(len(sess.execute(sa.select(t.c.col)).all()), 2)


class TransScopingTest(_fixtures.FixtureTest):
run_inserts = None
Expand Down
5 changes: 0 additions & 5 deletions test/perf/compiled_extensions/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,6 @@ def update_results(cls, results):
def none_20(self):
self.impl._distill_params_20(None)

@test_case
def empty_sequence_20(self):
self.impl._distill_params_20(())
self.impl._distill_params_20([])

@test_case
def list_20(self):
self.impl._distill_params_20(self.list_tup)
Expand Down
7 changes: 0 additions & 7 deletions test/perf/compiled_extensions/row.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ def python():
assert not py_util._is_compiled()
return py_util.tuplegetter

@staticmethod
def c():
from sqlalchemy import cresultproxy

return cresultproxy.tuplegetter

@staticmethod
def cython():
from sqlalchemy.engine import _util_cy
Expand All @@ -29,7 +23,6 @@ def cython():

IMPLEMENTATIONS = {
"python": python.__func__,
"c": c.__func__,
"cython": cython.__func__,
}

Expand Down
9 changes: 1 addition & 8 deletions test/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ def computed_columns_on_update_returning(self):

@property
def returning_star(self):
"""backend supports RETURNING *"""
"""backend supports ``RETURNING *``"""

return skip_if(["oracle", "mssql"])

Expand Down Expand Up @@ -1870,13 +1870,6 @@ def go(config):

return only_if(go)

@property
def oracle5x(self):
return only_if(
lambda config: against(config, "oracle+cx_oracle")
and config.db.dialect.cx_oracle_ver < (6,)
)

@property
def fail_on_oracledb_thin(self):
def go(config):
Expand Down

0 comments on commit e69ca16

Please sign in to comment.