From a9796655cd493f6a0819b7885f747de63f7265a3 Mon Sep 17 00:00:00 2001
From: Herberto Graca <herberto.graca@lendable.co.uk>
Date: Sat, 22 Jul 2023 19:17:22 +0200
Subject: [PATCH] Add skipLocked() to the query builder

This will allow us to create a query that will not
wait for a lock to be released on rows, and instead
return a row set excluding the rows locked by
another transaction.
---
 src/Platforms/AbstractPlatform.php            |   8 +
 src/Platforms/DB2111Platform.php              |   5 +
 src/Platforms/DB2Platform.php                 |   5 +
 src/Platforms/MariaDb1027Platform.php         |   7 +
 src/Platforms/MariaDb1060Platform.php         |  21 ++
 src/Platforms/MySQL57Platform.php             |   8 +
 src/Platforms/MySQL80Platform.php             |  10 +
 src/Platforms/SQLServerPlatform.php           |   5 +
 src/Platforms/SqlitePlatform.php              |   5 +
 src/Query/QueryBuilder.php                    |  12 ++
 src/Query/QueryLockBuilder.php                |   6 +-
 .../Driver/VersionAwarePlatformDriverTest.php |   5 +-
 tests/Functional/Query/QueryBuilderTest.php   |  79 ++++++++
 tests/Query/QueryBuilderTest.php              | 181 ++++++++++++++++++
 14 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 src/Platforms/MariaDb1060Platform.php
 create mode 100644 tests/Functional/Query/QueryBuilderTest.php

diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php
index 033e952e24a..60887a3792a 100644
--- a/src/Platforms/AbstractPlatform.php
+++ b/src/Platforms/AbstractPlatform.php
@@ -1757,6 +1757,14 @@ public function getForUpdateSQL()
         return 'FOR UPDATE';
     }
 
+    /**
+     * Returns the FOR UPDATE SKIP LOCKED expression.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return 'SKIP LOCKED';
+    }
+
     /**
      * Honors that some SQL vendors such as MsSql use table hints for locking instead of the
      * ANSI SQL FOR UPDATE specification.
diff --git a/src/Platforms/DB2111Platform.php b/src/Platforms/DB2111Platform.php
index 40ab42f6eb8..39ad7a3b1d0 100644
--- a/src/Platforms/DB2111Platform.php
+++ b/src/Platforms/DB2111Platform.php
@@ -37,4 +37,9 @@ protected function doModifyLimitQuery($query, $limit, $offset)
 
         return $query;
     }
+
+    public function getSkipLockedSQL(): string
+    {
+        return 'SKIP LOCKED DATA';
+    }
 }
diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php
index b203ce8a052..c6ae7665aba 100644
--- a/src/Platforms/DB2Platform.php
+++ b/src/Platforms/DB2Platform.php
@@ -1041,4 +1041,9 @@ public function createSchemaManager(Connection $connection): DB2SchemaManager
     {
         return new DB2SchemaManager($connection, $this);
     }
+
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
 }
diff --git a/src/Platforms/MariaDb1027Platform.php b/src/Platforms/MariaDb1027Platform.php
index 68f38133210..073697bebae 100644
--- a/src/Platforms/MariaDb1027Platform.php
+++ b/src/Platforms/MariaDb1027Platform.php
@@ -12,4 +12,11 @@
  */
 class MariaDb1027Platform extends MariaDBPlatform
 {
+    /**
+     * Returns the FOR UPDATE expression, as SKIP LOCKED is only available since MariaDB 10.6.0.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
 }
diff --git a/src/Platforms/MariaDb1060Platform.php b/src/Platforms/MariaDb1060Platform.php
new file mode 100644
index 00000000000..88cec3aa81d
--- /dev/null
+++ b/src/Platforms/MariaDb1060Platform.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace Doctrine\DBAL\Platforms;
+
+/**
+ * Provides the behavior, features and SQL dialect of the MariaDB 10.6 (10.6.0 GA) database platform.
+ *
+ * Note: Should not be used with versions prior to 10.6.0.
+ */
+class MariaDb1060Platform extends MariaDb1052Platform
+{
+    /**
+     * Returns the FOR UPDATE SKIP LOCKED expression.
+     * This method will become obsolete once the minimum MariaDb version is at least 10.6.0,
+     * as this method already exists in the base AbstractPlatform class.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return 'SKIP LOCKED';
+    }
+}
diff --git a/src/Platforms/MySQL57Platform.php b/src/Platforms/MySQL57Platform.php
index f3899959d3b..a01911caaa1 100644
--- a/src/Platforms/MySQL57Platform.php
+++ b/src/Platforms/MySQL57Platform.php
@@ -96,4 +96,12 @@ protected function initializeDoctrineTypeMappings()
 
         $this->doctrineTypeMapping['json'] = Types::JSON;
     }
+
+    /**
+     * Returns '', as SKIP LOCKED is only available since MySQL 8.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
 }
diff --git a/src/Platforms/MySQL80Platform.php b/src/Platforms/MySQL80Platform.php
index 9ea6ee881cf..6ecbe81ca1d 100644
--- a/src/Platforms/MySQL80Platform.php
+++ b/src/Platforms/MySQL80Platform.php
@@ -25,4 +25,14 @@ protected function getReservedKeywordsClass()
 
         return Keywords\MySQL80Keywords::class;
     }
+
+    /**
+     * Returns the SKIP LOCKED expression.
+     * When support for MySQL 5.7 is removed, this method can be removed from this
+     * class as it is already implemented in the base class.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return 'SKIP LOCKED';
+    }
 }
diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php
index 0bbd3d03d42..90648c26705 100644
--- a/src/Platforms/SQLServerPlatform.php
+++ b/src/Platforms/SQLServerPlatform.php
@@ -1616,6 +1616,11 @@ public function getForUpdateSQL()
         return 'WITH (UPDLOCK, ROWLOCK)';
     }
 
+    public function getSkipLockedSQL(): string
+    {
+        return 'WITH (READPAST)';
+    }
+
     /**
      * {@inheritDoc}
      *
diff --git a/src/Platforms/SqlitePlatform.php b/src/Platforms/SqlitePlatform.php
index 5acefc5c82f..ebf6e7fd839 100644
--- a/src/Platforms/SqlitePlatform.php
+++ b/src/Platforms/SqlitePlatform.php
@@ -740,6 +740,11 @@ public function getForUpdateSQL()
         return '';
     }
 
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
+
     /**
      * {@inheritDoc}
      *
diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php
index 8d1a6eab1e4..f60f74456ab 100644
--- a/src/Query/QueryBuilder.php
+++ b/src/Query/QueryBuilder.php
@@ -1650,4 +1650,16 @@ public function lockForUpdate(): self
 
         return $this;
     }
+
+    /**
+     * Sets a lock on the queried rows, until the end of the transaction
+     *
+     * @return $this
+     */
+    public function skipLocked(): self
+    {
+        $this->sqlParts['locks'][] = QueryLockBuilder::SKIP_LOCKED;
+
+        return $this;
+    }
 }
diff --git a/src/Query/QueryLockBuilder.php b/src/Query/QueryLockBuilder.php
index 24d6d1ff59a..28a7ea77ff7 100644
--- a/src/Query/QueryLockBuilder.php
+++ b/src/Query/QueryLockBuilder.php
@@ -16,7 +16,8 @@
 /** @internal */
 final class QueryLockBuilder
 {
-    public const FOR_UPDATE = 'FOR_UPDATE';
+    public const FOR_UPDATE  = 'FOR_UPDATE';
+    public const SKIP_LOCKED = 'SKIP_LOCKED';
 
     private AbstractPlatform $platform;
 
@@ -43,6 +44,9 @@ public function getLocksSql(string ...$lockList): string
                 case self::FOR_UPDATE:
                     $locksSql[0] = $this->platform->getForUpdateSQL();
                     break;
+                case self::SKIP_LOCKED:
+                    $locksSql[1] = $this->platform->getSkipLockedSQL();
+                    break;
             }
         }
 
diff --git a/tests/Driver/VersionAwarePlatformDriverTest.php b/tests/Driver/VersionAwarePlatformDriverTest.php
index 85fe30c3c99..2125f67b4c8 100644
--- a/tests/Driver/VersionAwarePlatformDriverTest.php
+++ b/tests/Driver/VersionAwarePlatformDriverTest.php
@@ -10,6 +10,7 @@
 use Doctrine\DBAL\Platforms\DB2Platform;
 use Doctrine\DBAL\Platforms\MariaDb1027Platform;
 use Doctrine\DBAL\Platforms\MariaDb1052Platform;
+use Doctrine\DBAL\Platforms\MariaDb1060Platform;
 use Doctrine\DBAL\Platforms\MySQL57Platform;
 use Doctrine\DBAL\Platforms\MySQL80Platform;
 use Doctrine\DBAL\Platforms\MySQLPlatform;
@@ -73,7 +74,7 @@ public static function mySQLVersionProvider(): array
                 'https://github.com/doctrine/dbal/pull/5779',
                 false,
             ],
-            ['mariadb-10.9.3', MariaDB1052Platform::class, 'https://github.com/doctrine/dbal/pull/5779', true],
+            ['mariadb-10.9.3', MariaDb1060Platform::class, 'https://github.com/doctrine/dbal/pull/5779', true],
             [
                 '10.5.2-MariaDB-1~lenny-log',
                 MariaDB1052Platform::class,
@@ -82,7 +83,7 @@ public static function mySQLVersionProvider(): array
             ],
             [
                 '11.0.2-MariaDB-1:11.0.2+maria~ubu2204',
-                MariaDB1052Platform::class,
+                MariaDb1060Platform::class,
                 'https://github.com/doctrine/dbal/pull/5779',
                 false,
             ],
diff --git a/tests/Functional/Query/QueryBuilderTest.php b/tests/Functional/Query/QueryBuilderTest.php
new file mode 100644
index 00000000000..9e1a8368811
--- /dev/null
+++ b/tests/Functional/Query/QueryBuilderTest.php
@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Tests\Functional\Query;
+
+use Doctrine\DBAL\Query\QueryBuilder;
+use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Tests\FunctionalTestCase;
+use Doctrine\DBAL\Tests\TestUtil;
+use Doctrine\DBAL\Types\Types;
+
+final class QueryBuilderTest extends FunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        $tableName = 'users';
+        $table     = new Table($tableName);
+        $table->addColumn('id', Types::INTEGER, ['autoincrement' => true]);
+        $table->addColumn('nickname', Types::STRING);
+        $table->setPrimaryKey(['id']);
+
+        $this->dropAndCreateTable($table);
+
+        $this->connection->insert($tableName, ['nickname' => 'aaa']);
+        $this->connection->insert($tableName, ['nickname' => 'bbb']);
+    }
+
+    public function testConcurrentConnectionSkipsLockedRows(): void
+    {
+        $connection1 = $this->connection;
+        $qb1         = new QueryBuilder($connection1);
+        $qb1->select('u.id')
+            ->from('users', 'u')
+            ->orderBy('id', 'ASC')
+            ->setMaxResults(1)
+            ->lockForUpdate()
+            ->skipLocked();
+
+        self::assertFalse($connection1->isTransactionActive(), 'A transaction should not be active on connection 1');
+        $connection1->beginTransaction();
+        self::assertTrue($connection1->isTransactionActive(), 'A transaction should be active on connection 1');
+        $result     = $connection1->executeQuery($qb1->getSQL());
+        $resultList = $result->fetchAllAssociative();
+        self::assertCount(1, $resultList);
+        self::assertEquals(1, $resultList[0]['id']);
+
+        $connection2 = TestUtil::getConnection();
+        self::assertTrue(
+            $connection1 !== $connection2,
+            "The two competing connections must be different, but they are the same so we can't run this test with it.",
+        );
+        self::assertFalse($connection2->isTransactionActive(), 'A transaction should not be active on connection 2');
+
+        $qb2 = new QueryBuilder($connection2);
+        $qb2->select('u.id')
+            ->from('users', 'u')
+            ->orderBy('id', 'ASC')
+            ->setMaxResults(1)
+            ->lockForUpdate()
+            ->skipLocked();
+
+        self::assertTrue($connection1->isTransactionActive(), 'A transaction should still be active on connection 1');
+        $result     = $connection2->executeQuery($qb2->getSQL());
+        $resultList = $result->fetchAllAssociative();
+        self::assertCount(1, $resultList);
+        self::assertEquals(2, $resultList[0]['id']);
+
+        $connection1->commit();
+        self::assertFalse(
+            $connection1->isTransactionActive(),
+            'A transaction should not be active anymore on connection 1',
+        );
+        $result     = $connection2->executeQuery($qb2->getSQL());
+        $resultList = $result->fetchAllAssociative();
+        self::assertCount(1, $resultList);
+        self::assertEquals(1, $resultList[0]['id']);
+    }
+}
diff --git a/tests/Query/QueryBuilderTest.php b/tests/Query/QueryBuilderTest.php
index fff9f038ae2..57dd1274189 100644
--- a/tests/Query/QueryBuilderTest.php
+++ b/tests/Query/QueryBuilderTest.php
@@ -1579,4 +1579,185 @@ public static function providePlatformAndExpectedForUpdateLockSql(): iterable
             'SELECT u.id FROM users u WITH (UPDLOCK, ROWLOCK) WHERE u.nickname = ?',
         ];
     }
+
+    /** @dataProvider providePlatformAndExpectedSkipLockedSql */
+    public function testQueryWithSkipLocked(AbstractPlatform $platform, string $expectedSql): void
+    {
+        $conn = $this->createConnectionMock();
+        $conn->expects(self::any())
+            ->method('getDatabasePlatform')
+            ->willReturn($platform);
+
+        $qb   = new QueryBuilder($conn);
+        $expr = $qb->expr();
+
+        $qb->select('u.id')
+            ->from('users', 'u')
+            ->where($expr->and($expr->eq('u.nickname', '?')))
+            ->skipLocked();
+
+        self::assertEquals($expectedSql, (string) $qb);
+    }
+
+    /**
+     * @return iterable<
+     *     string,
+     *     array{
+     *          AbstractPlatform,
+     *          string
+     *  }>
+     */
+    public static function providePlatformAndExpectedSkipLockedSql(): iterable
+    {
+        yield 'DB2Platform' => [
+            new DB2Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? ',
+        ];
+
+        yield 'DB2111Platform' => [
+            new DB2111Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED DATA',
+        ];
+
+        yield 'MySQLPlatform' => [
+            new MySQLPlatform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED',
+        ];
+
+        yield 'MySQL57Platform' => [
+            new MySQL57Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? ',
+        ];
+
+        yield 'MySQL80Platform' => [
+            new MySQL80Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED',
+        ];
+
+        yield 'OraclePlatform' => [
+            new MySQL80Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED',
+        ];
+
+        yield 'PostgreSQLPlatform' => [
+            new PostgreSQLPlatform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED',
+        ];
+
+        yield 'PostgreSQL94Platform' => [
+            new PostgreSQL94Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED',
+        ];
+
+        yield 'PostgreSQL100Platform' => [
+            new PostgreSQL100Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED',
+        ];
+
+        yield 'SqlitePlatform' => [
+            new SqlitePlatform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? ',
+        ];
+
+        yield 'SQLServerPlatform' => [
+            new SQLServerPlatform(),
+            'SELECT u.id FROM users u WITH (READPAST) WHERE u.nickname = ?',
+        ];
+
+        yield 'SQLServer2012Platform' => [
+            new SQLServer2012Platform(),
+            'SELECT u.id FROM users u WITH (READPAST) WHERE u.nickname = ?',
+        ];
+    }
+
+    /** @dataProvider providePlatformAndExpectedLockHintsSql */
+    public function testQueryWithAllLockOptions(AbstractPlatform $platform, string $expectedSql): void
+    {
+        $conn = $this->createConnectionMock();
+        $conn->expects(self::any())
+            ->method('getDatabasePlatform')
+            ->willReturn($platform);
+
+        $qb   = new QueryBuilder($conn);
+        $expr = $qb->expr();
+
+        $qb->select('u.id')
+            ->from('users', 'u')
+            ->where($expr->and($expr->eq('u.nickname', '?')))
+            ->skipLocked()
+            ->lockForUpdate();
+
+        self::assertEquals($expectedSql, (string) $qb);
+    }
+
+    /**
+     * @return iterable<
+     *     string,
+     *     array{
+     *          AbstractPlatform,
+     *          string
+     *  }>
+     */
+    public static function providePlatformAndExpectedLockHintsSql(): iterable
+    {
+        yield 'DB2Platform' => [
+            new DB2Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? WITH RR USE AND KEEP UPDATE LOCKS',
+        ];
+
+        yield 'DB2111Platform' => [
+            new DB2111Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? WITH RR USE AND KEEP UPDATE LOCKS SKIP LOCKED DATA',
+        ];
+
+        yield 'MySQLPlatform' => [
+            new MySQLPlatform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED',
+        ];
+
+        yield 'MySQL57Platform' => [
+            new MySQL57Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE',
+        ];
+
+        yield 'MySQL80Platform' => [
+            new MySQL80Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED',
+        ];
+
+        yield 'OraclePlatform' => [
+            new MySQL80Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED',
+        ];
+
+        yield 'PostgreSQLPlatform' => [
+            new PostgreSQLPlatform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED',
+        ];
+
+        yield 'PostgreSQL94Platform' => [
+            new PostgreSQL94Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED',
+        ];
+
+        yield 'PostgreSQL100Platform' => [
+            new PostgreSQL100Platform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED',
+        ];
+
+        yield 'SqlitePlatform' => [
+            new SqlitePlatform(),
+            'SELECT u.id FROM users u WHERE u.nickname = ? ',
+        ];
+
+        yield 'SQLServerPlatform' => [
+            new SQLServerPlatform(),
+            'SELECT u.id FROM users u WITH (UPDLOCK, ROWLOCK, READPAST) WHERE u.nickname = ?',
+        ];
+
+        yield 'SQLServer2012Platform' => [
+            new SQLServer2012Platform(),
+            'SELECT u.id FROM users u WITH (UPDLOCK, ROWLOCK, READPAST) WHERE u.nickname = ?',
+        ];
+    }
 }