From: Gord Thompson <gord@gordthompson.com>
Date: Sun, 13 Sep 2020 22:38:13 +0000 (-0600)
Subject: Add warning for skip_locked with MariaDB backend.
X-Git-Tag: rel_1_3_20~21
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f14fb98c7b2f66cde35edd24b9140e919a4e2181;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Add warning for skip_locked with MariaDB backend.

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
(cherry picked with major changes from commit b25d0cda90fe906fda2fe8401a810c4da0bf7268)
---

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/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py
index 181faf8fee..c44f2ac21e 100644
--- a/lib/sqlalchemy/dialects/mysql/base.py
+++ b/lib/sqlalchemy/dialects/mysql/base.py
@@ -1590,8 +1590,14 @@ class MySQLCompiler(compiler.SQLCompiler):
         if select._for_update_arg.nowait:
             tmp += " NOWAIT"
 
-        if select._for_update_arg.skip_locked and self.dialect._is_mysql:
-            tmp += " SKIP LOCKED"
+        if select._for_update_arg.skip_locked:
+            if self.dialect._is_mysql:
+                tmp += " SKIP LOCKED"
+            else:
+                util.warn(
+                    "SKIP LOCKED ignored on non-supporting MariaDB backend. "
+                    "This will raise an error in SQLAlchemy 1.4."
+                )
 
         return tmp
 
diff --git a/test/dialect/mysql/test_compiler.py b/test/dialect/mysql/test_compiler.py
index ad556902dd..8de027dadb 100644
--- a/test/dialect/mysql/test_compiler.py
+++ b/test/dialect/mysql/test_compiler.py
@@ -369,15 +369,16 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL):
             dialect=dialect,
         )
 
-        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=dialect,
-        )
+        with testing.expect_warnings("SKIP LOCKED ignored on non-supporting"):
+            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=dialect,
+            )
 
     def test_delete_extra_froms(self):
         t1 = table("t1", column("c1"))
diff --git a/test/dialect/mysql/test_for_update.py b/test/dialect/mysql/test_for_update.py
index 2d672cb3dd..604065c0ba 100644
--- a/test/dialect/mysql/test_for_update.py
+++ b/test/dialect/mysql/test_for_update.py
@@ -9,15 +9,19 @@ 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 assert_raises_message
 from sqlalchemy.testing import AssertsCompiledSQL
+from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 
 
@@ -357,3 +361,56 @@ class MySQLForUpdateCompileTest(fixtures.TestBase, AssertsCompiledSQL):
             "LOCK IN SHARE MODE OF mytable",
             dialect=self.for_update_of_dialect,
         )
+
+
+class SkipLockedTest(fixtures.TablesTest):
+    __only_on__ = ("mysql",)
+    __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")
+    @testing.skip_if(lambda config: testing.db.dialect._is_mariadb)
+    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(lambda config: testing.db.dialect._is_mariadb)
+    def test_warning_skip_locked(self, connection):
+
+        stuff = self.tables.stuff
+
+        stmt = stuff.select().with_for_update(skip_locked=True)
+
+        with expect_warnings(
+            "SKIP LOCKED ignored on non-supporting MariaDB backend. "
+            "This will raise an error in SQLAlchemy 1.4."
+        ):
+
+            connection.execute(stmt).fetchall()
+
+    @testing.only_on("mysql<8")
+    def test_unsupported_skip_locked(self, connection):
+
+        stuff = self.tables.stuff
+
+        stmt = stuff.select().with_for_update(skip_locked=True)
+
+        assert_raises_message(
+            ProgrammingError,
+            "You have an error in your SQL syntax",
+            connection.execute,
+            stmt,
+        )