From ddfa5565c1559a3727aa2f3012f27ac229abf912 Mon Sep 17 00:00:00 2001
From: Felix Scheinost <31761492+felixscheinost@users.noreply.github.com>
Date: Wed, 20 Nov 2024 19:07:41 +0100
Subject: [PATCH] feat: fix lock-and-fetch for MySQL (#537)

## Brief, plain english overview of your changes here
The support was nearly finished in #371 but later commented out due to
"some issue [..] and limited time".

I tried to find out what those issues were and discovered that the
syntax of the select was wrong. In MySQL the `LIMIT` comes before the
`FOR UPDATE SKIP LOCKED`.

Using that fix `Mysql8CompatibilityTest` passes, I have done no further
testing yet.

## Fixes
#264


## Reminders
- [x] Added/ran automated tests
- [ ] Update README and/or examples (not applicable?)
- [x] Ran `mvn spotless:apply`
---
 .../scheduler/jdbc/MariaDBJdbcCustomization.java          | 1 +
 .../scheduler/jdbc/MssqlJdbcCustomization.java            | 3 ++-
 .../scheduler/jdbc/MySQL8JdbcCustomization.java           | 8 ++++----
 .../scheduler/jdbc/OracleJdbcCustomization.java           | 1 +
 .../scheduler/jdbc/PostgreSqlJdbcCustomization.java       | 1 +
 .../com/github/kagkarlsson/scheduler/jdbc/Queries.java    | 7 +++++--
 .../scheduler/compatibility/Mysql8CompatibilityTest.java  | 2 +-
 7 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MariaDBJdbcCustomization.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MariaDBJdbcCustomization.java
index 36c1c289..f2efed6f 100644
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MariaDBJdbcCustomization.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MariaDBJdbcCustomization.java
@@ -50,6 +50,7 @@ public String createGenericSelectForUpdateQuery(
         Queries.postgresSqlLimitPart(limit),
         requiredAndCondition,
         " FOR UPDATE SKIP LOCKED ",
+        null,
         null);
   }
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MssqlJdbcCustomization.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MssqlJdbcCustomization.java
index 2bb58641..f69a62f0 100644
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MssqlJdbcCustomization.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MssqlJdbcCustomization.java
@@ -68,6 +68,7 @@ public String createGenericSelectForUpdateQuery(
         Queries.ansiSqlLimitPart(limit),
         requiredAndCondition,
         null,
-        " WITH (READPAST,ROWLOCK) ");
+        " WITH (READPAST,ROWLOCK) ",
+        null);
   }
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MySQL8JdbcCustomization.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MySQL8JdbcCustomization.java
index 8b86447e..31831c03 100644
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MySQL8JdbcCustomization.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/MySQL8JdbcCustomization.java
@@ -37,8 +37,7 @@ public String getQueryLimitPart(int limit) {
 
   @Override
   public boolean supportsGenericLockAndFetch() {
-    // FIXLATER: fix syntax and enable
-    return false;
+    return true;
   }
 
   @Override
@@ -49,7 +48,8 @@ public String createGenericSelectForUpdateQuery(
         Queries.ansiSqlOrderPart(orderByPriority),
         Queries.postgresSqlLimitPart(limit),
         requiredAndCondition,
-        " FOR UPDATE SKIP LOCKED ",
-        null);
+        null,
+        null,
+        " FOR UPDATE SKIP LOCKED ");
   }
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/OracleJdbcCustomization.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/OracleJdbcCustomization.java
index 6a2e921d..85946bc5 100644
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/OracleJdbcCustomization.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/OracleJdbcCustomization.java
@@ -41,6 +41,7 @@ public String createGenericSelectForUpdateQuery(
         Queries.ansiSqlLimitPart(limit),
         requiredAndCondition,
         " FOR UPDATE SKIP LOCKED ",
+        null,
         null);
   }
 }
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/PostgreSqlJdbcCustomization.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/PostgreSqlJdbcCustomization.java
index 5961ed6b..311c50fb 100644
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/PostgreSqlJdbcCustomization.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/PostgreSqlJdbcCustomization.java
@@ -59,6 +59,7 @@ public String createGenericSelectForUpdateQuery(
         getQueryLimitPart(limit),
         requiredAndCondition,
         " FOR UPDATE SKIP LOCKED ",
+        null,
         null);
   }
 
diff --git a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/Queries.java b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/Queries.java
index 0dd7f97a..0e30702a 100644
--- a/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/Queries.java
+++ b/db-scheduler/src/main/java/com/github/kagkarlsson/scheduler/jdbc/Queries.java
@@ -23,7 +23,8 @@ public static String selectForUpdate(
       String limitPart,
       String requiredAndCondition,
       String postgresOracleStyleForUpdate,
-      String sqlServerStyleForUpdate) {
+      String sqlServerStyleForUpdate,
+      String mysqlOracleStyleForUpdate) {
     return "SELECT * FROM "
         + tableName
         + Optional.ofNullable(sqlServerStyleForUpdate).orElse("")
@@ -31,7 +32,9 @@ public static String selectForUpdate(
         + requiredAndCondition
         + orderPart
         + Optional.ofNullable(postgresOracleStyleForUpdate).orElse("")
-        + limitPart;
+        + limitPart
+        // Note: Compared to Postgres, MySQL expects `FOR UPDATE SKIP LOCKED` after `LIMIT`
+        + Optional.ofNullable(mysqlOracleStyleForUpdate).orElse("");
   }
 
   public static String postgresSqlLimitPart(int limit) {
diff --git a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/compatibility/Mysql8CompatibilityTest.java b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/compatibility/Mysql8CompatibilityTest.java
index da92da1b..1f6dcfb6 100644
--- a/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/compatibility/Mysql8CompatibilityTest.java
+++ b/db-scheduler/src/test/java/com/github/kagkarlsson/scheduler/compatibility/Mysql8CompatibilityTest.java
@@ -24,7 +24,7 @@ public class Mysql8CompatibilityTest extends CompatibilityTest {
   private static HikariDataSource pooledDatasource;
 
   public Mysql8CompatibilityTest() {
-    super(false, false); // FIXLATER: fix syntax and enable
+    super(true, false);
   }
 
   @BeforeAll