diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index dc20904dca4..57abb714693 100644 --- 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 index 1dddeccaf21..4cb62861f81 100644 --- 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 index 073697bebae..68f38133210 100644 --- 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 index 1d4578979a1..61496426f92 100644 --- 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 index aa023ba794a..905b17441ad 100644 --- 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 index 2c974c36d6e..1e8f1f980f3 100644 --- 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 index 90648c26705..907040eaee8 100644 --- 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 index f60f74456ab..5e3e897723a 100644 --- a/src/Query/QueryBuilder.php +++ b/src/Query/QueryBuilder.php @@ -138,8 +138,6 @@ class QueryBuilder */ private ?QueryCacheProfile $resultCacheProfile = null; - private QueryLockBuilder $queryLockParser; - /** * Initializes a new QueryBuilder. * @@ -147,8 +145,7 @@ class QueryBuilder */ 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 new file mode 100644 index 00000000000..fe3382ae636 --- /dev/null +++ b/src/Query/QueryLock.php @@ -0,0 +1,30 @@ +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 deleted file mode 100644 index 28a7ea77ff7..00000000000 --- a/src/Query/QueryLockBuilder.php +++ /dev/null @@ -1,75 +0,0 @@ -platform = $platform; - } - - public function isLocatedAfterFrom(): bool - { - return $this->isSQLServerPlatform(); - } - - public function isLocatedAtTheEnd(): bool - { - return ! $this->isLocatedAfterFrom(); - } - - public function getLocksSql(string ...$lockList): string - { - $locksSql = []; - foreach ($lockList as $lock) { - switch ($lock) { - case self::FOR_UPDATE: - $locksSql[0] = $this->platform->getForUpdateSQL(); - break; - case self::SKIP_LOCKED: - $locksSql[1] = $this->platform->getSkipLockedSQL(); - break; - } - } - - ksort($locksSql); - - return trim( - $this->isSQLServerPlatform() - ? $this->mergeSQLServerLocks(...$locksSql) - : implode(' ', $locksSql), - ); - } - - private function isSQLServerPlatform(): bool - { - return $this->platform instanceof SQLServerPlatform; - } - - private function mergeSQLServerLocks(string ...$locksSql): string - { - foreach ($locksSql as $key => $lockSql) { - $locksSql[$key] = rtrim(ltrim($lockSql, 'WITH ('), ')'); - } - - return 'WITH (' . implode(', ', $locksSql) . ')'; - } -} diff --git a/tests/Driver/VersionAwarePlatformDriverTest.php b/tests/Driver/VersionAwarePlatformDriverTest.php index 2125f67b4c8..e68541de0f0 100644 --- 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, diff --git a/tests/Functional/Query/QueryBuilderTest.php b/tests/Functional/Query/QueryBuilderTest.php index 53004b8fcc1..ad69c11f4ac 100644 --- a/tests/Functional/Query/QueryBuilderTest.php +++ b/tests/Functional/Query/QueryBuilderTest.php @@ -8,6 +8,8 @@ use Doctrine\DBAL\Platforms\DB2Platform; use Doctrine\DBAL\Platforms\MariaDb1027Platform; use Doctrine\DBAL\Platforms\MySQL57Platform; +use Doctrine\DBAL\Platforms\OraclePlatform; +use Doctrine\DBAL\Platforms\PostgreSQL94Platform; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\DBAL\Query\QueryBuilder; use Doctrine\DBAL\Schema\Table; @@ -20,11 +22,23 @@ final class QueryBuilderTest extends FunctionalTestCase { - protected function setUp(): void + private function platformSupportsLocks(AbstractPlatform $platform): bool + { + return ! $platform instanceof DB2Platform + && ! $platform instanceof MariaDb1027Platform + && ! $platform instanceof MySQL57Platform + && ! $platform instanceof PostgreSQL94Platform + && ! $platform instanceof OraclePlatform + && ! $platform instanceof SqlitePlatform; + } + + public function testConcurrentConnectionSkipsLockedRows(): void { $platform = $this->connection->getDatabasePlatform(); - if (! $this->supportsPlatform($platform)) { - self::markTestSkipped(sprintf('Skipping since connected to %s', get_class($platform))); + if (! $this->platformSupportsLocks($platform)) { + self::markTestSkipped( + sprintf('Skipping, because platform %s does not support locks', get_class($platform)), + ); } $tableName = 'users'; @@ -32,23 +46,10 @@ protected function setUp(): void $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']); - } - - private function supportsPlatform(AbstractPlatform $platform): bool - { - return ! $platform instanceof DB2Platform - && ! $platform instanceof MariaDb1027Platform - && ! $platform instanceof MySQL57Platform - && ! $platform instanceof SqlitePlatform; - } - public function testConcurrentConnectionSkipsLockedRows(): void - { $connection1 = $this->connection; $qb1 = new QueryBuilder($connection1); $qb1->select('u.id') diff --git a/tests/Query/QueryBuilderTest.php b/tests/Query/QueryBuilderTest.php index b16b370b31c..d0941426d30 100644 --- a/tests/Query/QueryBuilderTest.php +++ b/tests/Query/QueryBuilderTest.php @@ -1653,7 +1653,7 @@ public static function providePlatformAndExpectedSkipLockedSql(): iterable yield 'PostgreSQL94Platform' => [ new PostgreSQL94Platform(), - 'SELECT u.id FROM users u WHERE u.nickname = ? SKIP LOCKED', + 'SELECT u.id FROM users u WHERE u.nickname = ? ', ]; yield 'PostgreSQL100Platform' => [ @@ -1744,7 +1744,7 @@ public static function providePlatformAndExpectedLockHintsSql(): iterable yield 'PostgreSQL94Platform' => [ new PostgreSQL94Platform(), - 'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE SKIP LOCKED', + 'SELECT u.id FROM users u WHERE u.nickname = ? FOR UPDATE', ]; yield 'PostgreSQL100Platform' => [