From: Mike Bayer Date: Thu, 19 Oct 2023 16:14:14 +0000 (-0400) Subject: manufacture empty result for DELETE..RETURNING w/ no description X-Git-Tag: rel_2_0_23~20^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c6270f4cfb0616b7b8676aac9092b8d959f96df7;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git manufacture empty result for DELETE..RETURNING w/ no description Established a workaround for what seems to be an intrinsic issue across MySQL/MariaDB drivers where a RETURNING result for DELETE DML which returns no rows using SQLAlchemy's "empty IN" criteria fails to provide a cursor.description, which then yields result that returns no rows, leading to regressions for the ORM that in the 2.0 series uses RETURNING for bulk DELETE statements for the "synchronize session" feature. To resolve, when the specific case of "no description when RETURNING was given" is detected, an "empty result" with a correct cursor description is generated and used in place of the non-working cursor. Fixes: #10505 Change-Id: Ib56f1ec5746e2b3212e563169353bc023db84099 --- diff --git a/doc/build/changelog/unreleased_20/10505.rst b/doc/build/changelog/unreleased_20/10505.rst new file mode 100644 index 0000000000..b9ea6b4e5b --- /dev/null +++ b/doc/build/changelog/unreleased_20/10505.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, mariadb, regression + :tickets: 10505 + + Established a workaround for what seems to be an intrinsic issue across + MySQL/MariaDB drivers where a RETURNING result for DELETE DML which returns + no rows using SQLAlchemy's "empty IN" criteria fails to provide a + cursor.description, which then yields result that returns no rows, + leading to regressions for the ORM that in the 2.0 series uses RETURNING + for bulk DELETE statements for the "synchronize session" feature. To + resolve, when the specific case of "no description when RETURNING was + given" is detected, an "empty result" with a correct cursor description is + generated and used in place of the non-working cursor. diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py index 465f3597cb..92f90774fb 100644 --- a/lib/sqlalchemy/dialects/mysql/base.py +++ b/lib/sqlalchemy/dialects/mysql/base.py @@ -999,14 +999,14 @@ output:: ) """ # noqa +from __future__ import annotations from array import array as _array from collections import defaultdict from itertools import compress import re +from typing import cast -from sqlalchemy import literal_column -from sqlalchemy.sql import visitors from . import reflection as _reflection from .enumerated import ENUM from .enumerated import SET @@ -1047,10 +1047,12 @@ from .types import TINYTEXT from .types import VARCHAR from .types import YEAR from ... import exc +from ... import literal_column from ... import log from ... import schema as sa_schema from ... import sql from ... import util +from ...engine import cursor as _cursor from ...engine import default from ...engine import reflection from ...engine.reflection import ReflectionDefaults @@ -1062,7 +1064,9 @@ from ...sql import operators from ...sql import roles from ...sql import sqltypes from ...sql import util as sql_util +from ...sql import visitors from ...sql.compiler import InsertmanyvaluesSentinelOpts +from ...sql.compiler import SQLCompiler from ...sql.schema import SchemaConst from ...types import BINARY from ...types import BLOB @@ -1166,6 +1170,32 @@ ischema_names = { class MySQLExecutionContext(default.DefaultExecutionContext): + def post_exec(self): + if ( + self.isdelete + and cast(SQLCompiler, self.compiled).effective_returning + and not self.cursor.description + ): + # All MySQL/mariadb drivers appear to not include + # cursor.description for DELETE..RETURNING with no rows if the + # WHERE criteria is a straight "false" condition such as our EMPTY + # IN condition. manufacture an empty result in this case (issue + # #10505) + # + # taken from cx_Oracle implementation + self.cursor_fetch_strategy = ( + _cursor.FullyBufferedCursorFetchStrategy( + self.cursor, + [ + (entry.keyname, None) + for entry in cast( + SQLCompiler, self.compiled + )._result_columns + ], + [], + ) + ) + def create_server_side_cursor(self): if self.dialect.supports_server_side_cursors: return self._dbapi_connection.cursor(self.dialect._sscursor) diff --git a/lib/sqlalchemy/dialects/mysql/mariadbconnector.py b/lib/sqlalchemy/dialects/mysql/mariadbconnector.py index df9e84ad51..9730c9b4da 100644 --- a/lib/sqlalchemy/dialects/mysql/mariadbconnector.py +++ b/lib/sqlalchemy/dialects/mysql/mariadbconnector.py @@ -80,6 +80,8 @@ class MySQLExecutionContext_mariadbconnector(MySQLExecutionContext): return self._dbapi_connection.cursor(buffered=True) def post_exec(self): + super().post_exec() + self._rowcount = self.cursor.rowcount if self.isinsert and self.compiled.postfetch_lastrowid: diff --git a/lib/sqlalchemy/testing/suite/test_update_delete.py b/lib/sqlalchemy/testing/suite/test_update_delete.py index 62776ea6b3..2d13bda34a 100644 --- a/lib/sqlalchemy/testing/suite/test_update_delete.py +++ b/lib/sqlalchemy/testing/suite/test_update_delete.py @@ -6,6 +6,7 @@ from ..schema import Column from ..schema import Table from ... import Integer from ... import String +from ... import testing class SimpleUpdateDeleteTest(fixtures.TablesTest): @@ -58,5 +59,71 @@ class SimpleUpdateDeleteTest(fixtures.TablesTest): [(1, "d1"), (3, "d3")], ) + @testing.variation("criteria", ["rows", "norows", "emptyin"]) + @testing.requires.update_returning + def test_update_returning(self, connection, criteria): + t = self.tables.plain_pk + + stmt = t.update().returning(t.c.id, t.c.data) + + if criteria.norows: + stmt = stmt.where(t.c.id == 10) + elif criteria.rows: + stmt = stmt.where(t.c.id == 2) + elif criteria.emptyin: + stmt = stmt.where(t.c.id.in_([])) + else: + criteria.fail() + + r = connection.execute(stmt, dict(data="d2_new")) + assert not r.is_insert + assert r.returns_rows + eq_(r.keys(), ["id", "data"]) + + if criteria.rows: + eq_(r.all(), [(2, "d2_new")]) + else: + eq_(r.all(), []) + + eq_( + connection.execute(t.select().order_by(t.c.id)).fetchall(), + [(1, "d1"), (2, "d2_new"), (3, "d3")] + if criteria.rows + else [(1, "d1"), (2, "d2"), (3, "d3")], + ) + + @testing.variation("criteria", ["rows", "norows", "emptyin"]) + @testing.requires.delete_returning + def test_delete_returning(self, connection, criteria): + t = self.tables.plain_pk + + stmt = t.delete().returning(t.c.id, t.c.data) + + if criteria.norows: + stmt = stmt.where(t.c.id == 10) + elif criteria.rows: + stmt = stmt.where(t.c.id == 2) + elif criteria.emptyin: + stmt = stmt.where(t.c.id.in_([])) + else: + criteria.fail() + + r = connection.execute(stmt) + assert not r.is_insert + assert r.returns_rows + eq_(r.keys(), ["id", "data"]) + + if criteria.rows: + eq_(r.all(), [(2, "d2")]) + else: + eq_(r.all(), []) + + eq_( + connection.execute(t.select().order_by(t.c.id)).fetchall(), + [(1, "d1"), (3, "d3")] + if criteria.rows + else [(1, "d1"), (2, "d2"), (3, "d3")], + ) + __all__ = ("SimpleUpdateDeleteTest",)