fixup!

doctrine · Sep 28, 2023 · 02edccb · 02edccb
1 parent d278c6b
commit 02edccb
Show file tree

Hide file tree

Showing 13 changed files with 161 additions and 114 deletions.
diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php
@@ -17,6 +17,7 @@
 use Doctrine\DBAL\Exception\InvalidLockMode;
 use Doctrine\DBAL\LockMode;
 use Doctrine\DBAL\Platforms\Keywords\KeywordList;
+use Doctrine\DBAL\Query\QueryLock;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\ColumnDiff;
@@ -54,6 +55,7 @@
 use function is_bool;
 use function is_int;
 use function is_string;
+use function ksort;
 use function preg_quote;
 use function preg_replace;
 use function sprintf;
@@ -62,6 +64,7 @@
 use function strpos;
 use function strtolower;
 use function strtoupper;
+use function trim;
 
 /**
  * Base class for all DatabasePlatforms. The DatabasePlatforms are the central
@@ -1766,13 +1769,48 @@ public function getForUpdateSQL()
     }
 
     /**
-     * Returns the FOR UPDATE SKIP LOCKED expression.
+     * Returns the SKIP LOCKED expression.
      */
     public function getSkipLockedSQL(): string
     {
         return 'SKIP LOCKED';
     }
 
+    public function getLocksSql(QueryLock ...$locks): string
+    {
+        return trim(implode(' ', $this->getLocksSqlList(...$locks)));
+    }
+
+    /** @return string[] */
+    protected function getLocksSqlList(QueryLock ...$locks): array
+    {
+        $locksSqlList = [];
+        foreach ($locks as $lock) {
+            switch ($lock->value()) {
+                case QueryLock::forUpdate()->value():
+                    $locksSqlList[0] = $this->getForUpdateSQL();
+                    break;
+                case QueryLock::skipLocked()->value():
+                    $locksSqlList[1] = $this->getSkipLockedSQL();
+                    break;
+            }
+        }
+
+        ksort($locksSqlList);
+
+        return $locksSqlList;
+    }
+
+    public function isLockLocatedAfterFrom(): bool
+    {
+        return false;
+    }
+
+    public function isLockLocatedAtTheEnd(): bool
+    {
+        return true;
+    }
+
     /**
      * 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/MariaDBPlatform.php b/src/Platforms/MariaDBPlatform.php
@@ -10,6 +10,11 @@
  */
 class MariaDBPlatform extends MySQLPlatform
 {
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
+
     /**
      * {@inheritDoc}
      *

diff --git a/src/Platforms/MariaDb1027Platform.php b/src/Platforms/MariaDb1027Platform.php
@@ -12,11 +12,4 @@
  */
 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/OraclePlatform.php b/src/Platforms/OraclePlatform.php
@@ -35,6 +35,24 @@
  */
 class OraclePlatform extends AbstractPlatform
 {
+    /**
+     * Returns the FOR UPDATE expression.
+     *
+     * @return string
+     */
+    public function getForUpdateSQL()
+    {
+        return '';
+    }
+
+    /**
+     * Returns the SKIP LOCKED expression.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
+
     /**
      * Assertion for Oracle identifiers.
      *

diff --git a/src/Platforms/PostgreSQL100Platform.php b/src/Platforms/PostgreSQL100Platform.php
@@ -27,4 +27,12 @@ protected function getReservedKeywordsClass(): string
 
         return PostgreSQL100Keywords::class;
     }
+
+    /**
+     * Returns the SKIP LOCKED expression.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return 'SKIP LOCKED';
+    }
 }
diff --git a/src/Platforms/PostgreSQL94Platform.php b/src/Platforms/PostgreSQL94Platform.php
@@ -9,4 +9,11 @@
  */
 class PostgreSQL94Platform extends PostgreSQLPlatform
 {
+    /**
+     * Returns the SKIP LOCKED expression.
+     */
+    public function getSkipLockedSQL(): string
+    {
+        return '';
+    }
 }
diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php
@@ -5,6 +5,7 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Exception\InvalidLockMode;
 use Doctrine\DBAL\LockMode;
+use Doctrine\DBAL\Query\QueryLock;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\ColumnDiff;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
@@ -33,8 +34,10 @@
 use function is_bool;
 use function is_numeric;
 use function is_string;
+use function ltrim;
 use function preg_match;
 use function preg_match_all;
+use function rtrim;
 use function sprintf;
 use function str_replace;
 use function strpos;
@@ -1621,6 +1624,27 @@ public function getSkipLockedSQL(): string
         return 'WITH (READPAST)';
     }
 
+    public function getLocksSql(QueryLock ...$locks): string
+    {
+        $locksSqlList = $this->getLocksSqlList(...$locks);
+
+        foreach ($locksSqlList as $key => $lockSql) {
+            $locksSqlList[$key] = rtrim(ltrim($lockSql, 'WITH ('), ')');
+        }
+
+        return 'WITH (' . implode(', ', $locksSqlList) . ')';
+    }
+
+    public function isLockLocatedAfterFrom(): bool
+    {
+        return true;
+    }
+
+    public function isLockLocatedAtTheEnd(): bool
+    {
+        return false;
+    }
+
     /**
      * {@inheritDoc}
      *

diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php
@@ -138,17 +138,14 @@ class QueryBuilder
      */
     private ?QueryCacheProfile $resultCacheProfile = null;
 
-    private QueryLockBuilder $queryLockParser;
-
     /**
      * Initializes a new <tt>QueryBuilder</tt>.
      *
      * @param Connection $connection The DBAL Connection.
      */
     public function __construct(Connection $connection)
     {
-        $this->connection      = $connection;
-        $this->queryLockParser = new QueryLockBuilder($this->connection->getDatabasePlatform());
+        $this->connection = $connection;
     }
 
     /**
@@ -1360,20 +1357,20 @@ private function getSQLForSelect(): string
         $query = 'SELECT ' . ($this->sqlParts['distinct'] ? 'DISTINCT ' : '') .
                   implode(', ', $this->sqlParts['select']);
 
-        $locksSql = $this->hasLocks()
-            ? ' ' . $this->queryLockParser->getLocksSql(...$this->sqlParts['locks'])
-            : '';
+        $platform = $this->connection->getDatabasePlatform();
+
+        $locksSql = $this->hasLocks() ? ' ' . $platform->getLocksSql(...$this->sqlParts['locks']) : '';
 
         $query .= ($this->sqlParts['from'] ? ' FROM ' . implode(', ', $this->getFromClauses()) : '')
-            . ($this->queryLockParser->isLocatedAfterFrom() ? $locksSql : '')
+            . ($platform->isLockLocatedAfterFrom() ? $locksSql : '')
             . ($this->sqlParts['where'] !== null ? ' WHERE ' . ((string) $this->sqlParts['where']) : '')
             . ($this->sqlParts['groupBy'] ? ' GROUP BY ' . implode(', ', $this->sqlParts['groupBy']) : '')
             . ($this->sqlParts['having'] !== null ? ' HAVING ' . ((string) $this->sqlParts['having']) : '')
             . ($this->sqlParts['orderBy'] ? ' ORDER BY ' . implode(', ', $this->sqlParts['orderBy']) : '')
-            . ($this->queryLockParser->isLocatedAtTheEnd() ? $locksSql : '');
+            . ($platform->isLockLocatedAtTheEnd() ? $locksSql : '');
 
         if ($this->isLimitQuery()) {
-            return $this->connection->getDatabasePlatform()->modifyLimitQuery(
+            return $platform->modifyLimitQuery(
                 $query,
                 $this->maxResults,
                 $this->firstResult,
@@ -1646,7 +1643,7 @@ private function hasLocks(): bool
      */
     public function lockForUpdate(): self
     {
-        $this->sqlParts['locks'][] = QueryLockBuilder::FOR_UPDATE;
+        $this->sqlParts['locks'][] = QueryLock::forUpdate();
 
         return $this;
     }
@@ -1658,7 +1655,7 @@ public function lockForUpdate(): self
      */
     public function skipLocked(): self
     {
-        $this->sqlParts['locks'][] = QueryLockBuilder::SKIP_LOCKED;
+        $this->sqlParts['locks'][] = QueryLock::skipLocked();
 
         return $this;
     }

diff --git a/src/Query/QueryLock.php b/src/Query/QueryLock.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Query;
+
+final class QueryLock
+{
+    private string $value;
+
+    private function __construct(string $value)
+    {
+        $this->value = $value;
+    }
+
+    public static function forUpdate(): self
+    {
+        return new self('FOR_UPDATE');
+    }
+
+    public static function skipLocked(): self
+    {
+        return new self('SKIP_LOCKED');
+    }
+
+    public function value(): string
+    {
+        return $this->value;
+    }
+}
diff --git a/src/Query/QueryLockBuilder.php b/src/Query/QueryLockBuilder.php
diff --git a/tests/Driver/VersionAwarePlatformDriverTest.php b/tests/Driver/VersionAwarePlatformDriverTest.php
@@ -74,13 +74,14 @@ public static function mySQLVersionProvider(): array
                 'https://github.com/doctrine/dbal/pull/5779',
                 false,
             ],
-            ['mariadb-10.9.3', MariaDb1060Platform::class, 'https://github.com/doctrine/dbal/pull/5779', true],
             [
                 '10.5.2-MariaDB-1~lenny-log',
                 MariaDB1052Platform::class,
                 'https://github.com/doctrine/dbal/pull/5779',
                 false,
             ],
+            ['mariadb-10.6.0', MariaDb1060Platform::class, 'https://github.com/doctrine/dbal/pull/5779', true],
+            ['mariadb-10.9.3', MariaDb1060Platform::class, 'https://github.com/doctrine/dbal/pull/5779', true],
             [
                 '11.0.2-MariaDB-1:11.0.2+maria~ubu2204',
                 MariaDb1060Platform::class,