From 31c3ed715ad2e6007bf6b98ae7670cb1a902731c Mon Sep 17 00:00:00 2001 From: Gord Thompson Date: Sun, 13 Sep 2020 16:38:13 -0600 Subject: [PATCH] Remove silent ignore for skip_locked w/ unsupported backends For SQLAlchemy 1.4: The "skip_locked" keyword used with ``with_for_update()`` will render "SKIP LOCKED" on all MySQL backends, meaning it will fail for MySQL less than version 8 and on current MariaDB backends. This is because those backends do not support "SKIP LOCKED" or any equivalent, so this error should not be silently ignored. This is upgraded from a warning in the 1.3 series. For SQLAlchemy 1.3: The "skip_locked" keyword used with ``with_for_update()`` will emit a warning when used on MariaDB backends, and will then be ignored. This is a deprecated behavior that will raise in SQLAlchemy 1.4, as an application that requests "skip locked" is looking for a non-blocking operation which is not available on those backends. Fixes: #5578 Change-Id: I49ccb6c6ff46eafed12b77f51e1da8e0e397966c --- doc/build/changelog/unreleased_13/5578.rst | 11 +++++++ doc/build/changelog/unreleased_14/5578.rst | 10 +++++++ lib/sqlalchemy/dialects/mysql/base.py | 2 +- test/dialect/mysql/test_compiler.py | 11 ------- test/dialect/mysql/test_for_update.py | 35 ++++++++++++++++++++++ 5 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 doc/build/changelog/unreleased_13/5578.rst create mode 100644 doc/build/changelog/unreleased_14/5578.rst diff --git a/doc/build/changelog/unreleased_13/5578.rst b/doc/build/changelog/unreleased_13/5578.rst new file mode 100644 index 0000000000..28dc955f37 --- /dev/null +++ b/doc/build/changelog/unreleased_13/5578.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, mysql + :tickets: 5568 + + The "skip_locked" keyword used with ``with_for_update()`` will emit a + warning when used on MariaDB backends, and will then be ignored. This is + a deprecated behavior that will raise in SQLAlchemy 1.4, as an application + that requests "skip locked" is looking for a non-blocking operation which + is not available on those backends. + + diff --git a/doc/build/changelog/unreleased_14/5578.rst b/doc/build/changelog/unreleased_14/5578.rst new file mode 100644 index 0000000000..eabe939762 --- /dev/null +++ b/doc/build/changelog/unreleased_14/5578.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, mysql + :tickets: 5568 + + The "skip_locked" keyword used with ``with_for_update()`` will render "SKIP + LOCKED" on all MySQL backends, meaning it will fail for MySQL less than + version 8 and on current MariaDB backends. This is because those backends + do not support "SKIP LOCKED" or any equivalent, so this error should not be + silently ignored. This is upgraded from a warning in the 1.3 series. + diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py index b9067f3840..8fb4c3b4b4 100644 --- a/lib/sqlalchemy/dialects/mysql/base.py +++ b/lib/sqlalchemy/dialects/mysql/base.py @@ -1670,7 +1670,7 @@ class MySQLCompiler(compiler.SQLCompiler): if select._for_update_arg.nowait: tmp += " NOWAIT" - if select._for_update_arg.skip_locked and self.dialect._is_mysql: + if select._for_update_arg.skip_locked: tmp += " SKIP LOCKED" return tmp diff --git a/test/dialect/mysql/test_compiler.py b/test/dialect/mysql/test_compiler.py index 80fc65a0b3..b222431ebd 100644 --- a/test/dialect/mysql/test_compiler.py +++ b/test/dialect/mysql/test_compiler.py @@ -352,7 +352,6 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): self.assert_compile(expr, "concat('x', 'y')", literal_binds=True) def test_mariadb_for_update(self): - table1 = table( "mytable", column("myid"), column("name"), column("description") ) @@ -365,16 +364,6 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): dialect="mariadb", ) - self.assert_compile( - table1.select(table1.c.myid == 7).with_for_update( - skip_locked=True - ), - "SELECT mytable.myid, mytable.name, mytable.description " - "FROM mytable WHERE mytable.myid = %s " - "FOR UPDATE", - dialect="mariadb", - ) - def test_delete_extra_froms(self): t1 = table("t1", column("c1")) t2 = table("t2", column("c1")) diff --git a/test/dialect/mysql/test_for_update.py b/test/dialect/mysql/test_for_update.py index e39a3fcc00..d58c026157 100644 --- a/test/dialect/mysql/test_for_update.py +++ b/test/dialect/mysql/test_for_update.py @@ -9,15 +9,18 @@ from sqlalchemy import Column from sqlalchemy import exc from sqlalchemy import ForeignKey from sqlalchemy import Integer +from sqlalchemy import Table from sqlalchemy import testing from sqlalchemy import update from sqlalchemy.dialects.mysql import base as mysql +from sqlalchemy.exc import ProgrammingError from sqlalchemy.orm import joinedload from sqlalchemy.orm import relationship from sqlalchemy.orm import Session from sqlalchemy.sql import column from sqlalchemy.sql import table from sqlalchemy.testing import AssertsCompiledSQL +from sqlalchemy.testing import expect_raises_message from sqlalchemy.testing import fixtures @@ -363,3 +366,35 @@ class MySQLForUpdateCompileTest(fixtures.TestBase, AssertsCompiledSQL): "LOCK IN SHARE MODE OF mytable", dialect=self.for_update_of_dialect, ) + + +class SkipLockedTest(fixtures.TablesTest): + __only_on__ = ("mysql", "mariadb") + + __backend__ = True + + @classmethod + def define_tables(cls, metadata): + Table( + "stuff", + metadata, + Column("id", Integer, primary_key=True), + Column("value", Integer), + ) + + @testing.only_on("mysql>=8") + def test_skip_locked(self, connection): + stuff = self.tables.stuff + stmt = stuff.select().with_for_update(skip_locked=True) + + connection.execute(stmt).fetchall() + + @testing.only_on(["mysql<8", "mariadb"]) + def test_unsupported_skip_locked(self, connection): + stuff = self.tables.stuff + stmt = stuff.select().with_for_update(skip_locked=True) + + with expect_raises_message( + ProgrammingError, "You have an error in your SQL syntax" + ): + connection.execute(stmt).fetchall() -- 2.47.2