From: Pablo Nicolas Estevez Date: Mon, 9 Dec 2024 19:44:44 +0000 (-0500) Subject: add delete limit to mysql; ensure int for update/delete limits X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=134ad3bbdc4bcbee13acc043be0a98cc314fcaec;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git add delete limit to mysql; ensure int for update/delete limits Added support for the ``LIMIT`` clause with ``DELETE`` for the MySQL and MariaDB dialects, to complement the already present option for ``UPDATE``. The :meth:`.delete.with_dialect_options` method of the `:func:`.delete` construct accepts parameters for ``mysql_limit`` and ``mariadb_limit``, allowing users to specify a limit on the number of rows deleted. Pull request courtesy of Pablo Nicolás Estevez. Added logic to ensure that the ``mysql_limit`` and ``mariadb_limit`` parameters of :meth:`.update.with_dialect_options` and :meth:`.delete.with_dialect_options` when compiled to string will only compile if the parameter is passed as an integer; a ``ValueError`` is raised otherwise. corrected mysql documentation for update/delete options which must be specified using the ``with_dialect_options()`` method. Fixes: #11764 Closes: #12146 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/12146 Pull-request-sha: e34708374c67e016cda88919109fec5e6462eced Change-Id: I8681ddabaa192b672c7a9b9981c4fe9e4bdc8d03 --- diff --git a/doc/build/changelog/unreleased_20/11764.rst b/doc/build/changelog/unreleased_20/11764.rst new file mode 100644 index 0000000000..499852b6d0 --- /dev/null +++ b/doc/build/changelog/unreleased_20/11764.rst @@ -0,0 +1,20 @@ +.. change:: + :tags: usecase, mysql, mariadb + :tickets: 11764 + + Added support for the ``LIMIT`` clause with ``DELETE`` for the MySQL and + MariaDB dialects, to complement the already present option for + ``UPDATE``. The :meth:`.delete.with_dialect_options` method of the + `:func:`.delete` construct accepts parameters for ``mysql_limit`` and + ``mariadb_limit``, allowing users to specify a limit on the number of rows + deleted. Pull request courtesy of Pablo Nicolás Estevez. + + +.. change:: + :tags: bug, mysql, mariadb + + Added logic to ensure that the ``mysql_limit`` and ``mariadb_limit`` + parameters of :meth:`.update.with_dialect_options` and + :meth:`.delete.with_dialect_options` when compiled to string will only + compile if the parameter is passed as an integer; a ``ValueError`` is + raised otherwise. diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py index efec95cf0d..42e80cf273 100644 --- a/lib/sqlalchemy/dialects/mysql/base.py +++ b/lib/sqlalchemy/dialects/mysql/base.py @@ -488,7 +488,14 @@ available. * UPDATE with LIMIT:: - update(..., mysql_limit=10, mariadb_limit=10) + update(...).with_dialect_options(mysql_limit=10, mariadb_limit=10) + +* DELETE + with LIMIT:: + + delete(...).with_dialect_options(mysql_limit=10, mariadb_limit=10) + + .. versionadded:: 2.0.37 Added delete with limit * optimizer hints, use :meth:`_expression.Select.prefix_with` and :meth:`_query.Query.prefix_with`:: @@ -1713,8 +1720,15 @@ class MySQLCompiler(compiler.SQLCompiler): def update_limit_clause(self, update_stmt): limit = update_stmt.kwargs.get("%s_limit" % self.dialect.name, None) - if limit: - return "LIMIT %s" % limit + if limit is not None: + return f"LIMIT {int(limit)}" + else: + return None + + def delete_limit_clause(self, delete_stmt): + limit = delete_stmt.kwargs.get("%s_limit" % self.dialect.name, None) + if limit is not None: + return f"LIMIT {int(limit)}" else: return None @@ -2538,6 +2552,7 @@ class MySQLDialect(default.DefaultDialect): construct_arguments = [ (sa_schema.Table, {"*": None}), (sql.Update, {"limit": None}), + (sql.Delete, {"limit": None}), (sa_schema.PrimaryKeyConstraint, {"using": None}), ( sa_schema.Index, diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 84bb856d78..257b792132 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -3164,7 +3164,9 @@ class Query( ) def delete( - self, synchronize_session: SynchronizeSessionArgument = "auto" + self, + synchronize_session: SynchronizeSessionArgument = "auto", + delete_args: Optional[Dict[Any, Any]] = None, ) -> int: r"""Perform a DELETE with an arbitrary WHERE clause. @@ -3189,6 +3191,13 @@ class Query( :ref:`orm_expression_update_delete` for a discussion of these strategies. + :param delete_args: Optional dictionary, if present will be passed + to the underlying :func:`_expression.delete` construct as the ``**kw`` + for the object. May be used to pass dialect-specific arguments such + as ``mysql_limit``. + + .. versionadded:: 2.0.37 + :return: the count of rows matched as returned by the database's "row count" feature. @@ -3198,7 +3207,7 @@ class Query( """ # noqa: E501 - bulk_del = BulkDelete(self) + bulk_del = BulkDelete(self, delete_args) if self.dispatch.before_compile_delete: for fn in self.dispatch.before_compile_delete: new_query = fn(bulk_del.query, bulk_del) @@ -3208,6 +3217,10 @@ class Query( self = bulk_del.query delete_ = sql.delete(*self._raw_columns) # type: ignore + + if delete_args: + delete_ = delete_.with_dialect_options(**delete_args) + delete_._where_criteria = self._where_criteria result: CursorResult[Any] = self.session.execute( delete_, @@ -3263,9 +3276,8 @@ class Query( strategies. :param update_args: Optional dictionary, if present will be passed - to the underlying :func:`_expression.update` - construct as the ``**kw`` for - the object. May be used to pass dialect-specific arguments such + to the underlying :func:`_expression.update` construct as the ``**kw`` + for the object. May be used to pass dialect-specific arguments such as ``mysql_limit``, as well as other special arguments such as :paramref:`~sqlalchemy.sql.expression.update.preserve_parameter_order`. @@ -3440,6 +3452,14 @@ class BulkUpdate(BulkUD): class BulkDelete(BulkUD): """BulkUD which handles DELETEs.""" + def __init__( + self, + query: Query[Any], + delete_kwargs: Optional[Dict[Any, Any]], + ): + super().__init__(query) + self.delete_kwargs = delete_kwargs + class RowReturningQuery(Query[Row[Unpack[_Ts]]]): if TYPE_CHECKING: diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index dacbfc38af..21ba058abf 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -6102,6 +6102,10 @@ class SQLCompiler(Compiled): """Provide a hook for MySQL to add LIMIT to the UPDATE""" return None + def delete_limit_clause(self, delete_stmt): + """Provide a hook for MySQL to add LIMIT to the DELETE""" + return None + def update_tables_clause(self, update_stmt, from_table, extra_froms, **kw): """Provide a hook to override the initial table clause in an UPDATE statement. @@ -6394,6 +6398,10 @@ class SQLCompiler(Compiled): if t: text += " WHERE " + t + limit_clause = self.delete_limit_clause(delete_stmt) + if limit_clause: + text += " " + limit_clause + if ( self.implicit_returning or delete_stmt._returning ) and not self.returning_precedes_values: diff --git a/test/dialect/mysql/test_compiler.py b/test/dialect/mysql/test_compiler.py index f0dcb58388..59d604eace 100644 --- a/test/dialect/mysql/test_compiler.py +++ b/test/dialect/mysql/test_compiler.py @@ -55,13 +55,16 @@ from sqlalchemy.dialects.mysql import base as mysql from sqlalchemy.dialects.mysql import insert from sqlalchemy.dialects.mysql import match from sqlalchemy.sql import column +from sqlalchemy.sql import delete from sqlalchemy.sql import table +from sqlalchemy.sql import update from sqlalchemy.sql.expression import bindparam from sqlalchemy.sql.expression import literal_column from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ from sqlalchemy.testing import eq_ignore_whitespace +from sqlalchemy.testing import expect_raises from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures from sqlalchemy.testing import mock @@ -724,6 +727,14 @@ class SQLTest(fixtures.TestBase, AssertsCompiledSQL): .with_dialect_options(mysql_limit=5), "UPDATE t SET col1=%s LIMIT 5", ) + + # does not make sense but we want this to compile + self.assert_compile( + t.update() + .values({"col1": 123}) + .with_dialect_options(mysql_limit=0), + "UPDATE t SET col1=%s LIMIT 0", + ) self.assert_compile( t.update() .values({"col1": 123}) @@ -738,6 +749,39 @@ class SQLTest(fixtures.TestBase, AssertsCompiledSQL): "UPDATE t SET col1=%s WHERE t.col2 = %s LIMIT 1", ) + def test_delete_limit(self): + t = sql.table("t", sql.column("col1"), sql.column("col2")) + + self.assert_compile(t.delete(), "DELETE FROM t") + self.assert_compile( + t.delete().with_dialect_options(mysql_limit=5), + "DELETE FROM t LIMIT 5", + ) + # does not make sense but we want this to compile + self.assert_compile( + t.delete().with_dialect_options(mysql_limit=0), + "DELETE FROM t LIMIT 0", + ) + self.assert_compile( + t.delete().with_dialect_options(mysql_limit=None), + "DELETE FROM t", + ) + self.assert_compile( + t.delete() + .where(t.c.col2 == 456) + .with_dialect_options(mysql_limit=1), + "DELETE FROM t WHERE t.col2 = %s LIMIT 1", + ) + + @testing.combinations((update,), (delete,)) + def test_update_delete_limit_int_only(self, crud_fn): + t = sql.table("t", sql.column("col1"), sql.column("col2")) + + with expect_raises(ValueError): + crud_fn(t).with_dialect_options(mysql_limit="not an int").compile( + dialect=mysql.dialect() + ) + def test_utc_timestamp(self): self.assert_compile(func.utc_timestamp(), "utc_timestamp()") diff --git a/test/orm/dml/test_update_delete_where.py b/test/orm/dml/test_update_delete_where.py index da8efa44fa..7d06a8618c 100644 --- a/test/orm/dml/test_update_delete_where.py +++ b/test/orm/dml/test_update_delete_where.py @@ -2586,7 +2586,7 @@ class UpdateDeleteFromTest(fixtures.MappedTest): ) -class ExpressionUpdateTest(fixtures.MappedTest): +class ExpressionUpdateDeleteTest(fixtures.MappedTest): @classmethod def define_tables(cls, metadata): Table( @@ -2652,6 +2652,27 @@ class ExpressionUpdateTest(fixtures.MappedTest): eq_(update_stmt.dialect_kwargs, update_args) + def test_delete_args(self): + Data = self.classes.Data + session = fixture_session() + delete_args = {"mysql_limit": 1} + + m1 = testing.mock.Mock() + + @event.listens_for(session, "after_bulk_delete") + def do_orm_execute(bulk_ud): + delete_stmt = ( + bulk_ud.result.context.compiled.compile_state.statement + ) + m1(delete_stmt) + + q = session.query(Data) + q.delete(delete_args=delete_args) + + delete_stmt = m1.mock_calls[0][1][0] + + eq_(delete_stmt.dialect_kwargs, delete_args) + class InheritTest(fixtures.DeclarativeMappedTest): run_inserts = "each"