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

diff --git a/doc/build/changelog/unreleased_13/5578.rst b/doc/build/changelog/unreleased_13/5578.rst
new file mode 100644 (file)
index 0000000..28dc955
--- /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 (file)
index 0000000..eabe939
--- /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 b9067f38409a14125e1dd35134f0c280c3e62fc0..8fb4c3b4b43d5ee3c00684f096b33f3c37e9e385 100644 (file)
--- 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 80fc65a0b3dd3a6670123b5f7a1e2e7ddfa4e66e..b222431ebde558361ede8c630d9e2445b5d83065 100644 (file)
--- 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 e39a3fcc0014a62b3f3c847c70b43c619d7d4b7f..d58c026157825db64eee3101f29ab816dbfa202b 100644 (file)
--- 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()