From fb0b27e1801e0fc5422ed6a9f856f54bd7b7a21f Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 27 Jan 2024 14:48:49 +0100 Subject: [PATCH 01/41] Add a test covering the #11112 issue --- .../ORM/Functional/Ticket/GH11112Test.php | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php new file mode 100644 index 00000000000..4ecbea9d3e3 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php @@ -0,0 +1,79 @@ +useModelSet('cms'); + self::$queryCache = new ArrayAdapter(); + + parent::setUp(); + } + + public function testSimpleQueryHasLimitAndOffsetApplied(): void + { + $platform = $this->_em->getConnection()->getDatabasePlatform(); + $query = $this->_em->createQuery('SELECT u FROM ' . CmsUser::class . ' u'); + $originalSql = $query->getSQL(); + + $query->setMaxResults(10); + $query->setFirstResult(20); + $sql10_20 = $query->getSQL(); + + $query->setMaxResults(30); + $query->setFirstResult(40); + $sql30_40 = $query->getSQL(); + + // The SQL is platform specific and may even be something with outer SELECTS being added. So, + // derive the expected value at runtime through the platform. + self::assertSame($platform->modifyLimitQuery($originalSql, 10, 20), $sql10_20); + self::assertSame($platform->modifyLimitQuery($originalSql, 30, 40), $sql30_40); + + $cacheEntries = self::$queryCache->getValues(); + self::assertCount(1, $cacheEntries); + } + + public function testSubqueryLimitAndOffsetAreIgnored(): void + { + // Not sure what to do about this test. Basically, I want to make sure that + // firstResult/maxResult for subqueries are not relevant, they do not make it + // into the final query at all. That would give us the guarantee that the + // "sql finalizer" step is sufficient for the final, "outer" query and we + // do not need to run finalizers for the subqueries. + + // This DQL/query makes no sense, it's just about creating a subquery in the first place + $queryBuilder = $this->_em->createQueryBuilder(); + $queryBuilder + ->select('o') + ->from(CmsUser::class, 'o') + ->where($queryBuilder->expr()->exists( + $this->_em->createQueryBuilder() + ->select('u') + ->from(CmsUser::class, 'u') + ->setFirstResult(10) + ->setMaxResults(20) + )); + + $query = $queryBuilder->getQuery(); + $originalSql = $query->getSQL(); + + $clone = clone $query; + $clone->setFirstResult(24); + $clone->setMaxResults(42); + $limitedSql = $clone->getSQL(); + + $platform = $this->_em->getConnection()->getDatabasePlatform(); + + // The SQL is platform specific and may even be something with outer SELECTS being added. So, + // derive the expected value at runtime through the platform. + self::assertSame($platform->modifyLimitQuery($originalSql, 42, 24), $limitedSql); + } +} From 65c5a72571e6b3e5302abf0b5916f059a05d2027 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 27 Jan 2024 15:08:28 +0100 Subject: [PATCH 02/41] Add new OutputWalker and SqlFinalizer interfaces --- lib/Doctrine/ORM/Query/Exec/SqlFinalizer.php | 26 ++++++++++++++++++++ lib/Doctrine/ORM/Query/OutputWalker.php | 22 +++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 lib/Doctrine/ORM/Query/Exec/SqlFinalizer.php create mode 100644 lib/Doctrine/ORM/Query/OutputWalker.php diff --git a/lib/Doctrine/ORM/Query/Exec/SqlFinalizer.php b/lib/Doctrine/ORM/Query/Exec/SqlFinalizer.php new file mode 100644 index 00000000000..cddad84e8a3 --- /dev/null +++ b/lib/Doctrine/ORM/Query/Exec/SqlFinalizer.php @@ -0,0 +1,26 @@ + Date: Sat, 27 Jan 2024 15:19:48 +0100 Subject: [PATCH 03/41] Add a SingleSelectSqlFinalizer that can take care of adding offset/limit as well as locking mode statements to a given SQL query. Add a FinalizedSelectExecutor that executes given, finalized SQL statements. --- .../Query/Exec/FinalizedSelectExecutor.php | 26 ++++++++ .../Query/Exec/SingleSelectSqlFinalizer.php | 60 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php create mode 100644 lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php diff --git a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php new file mode 100644 index 00000000000..b777b3d68cc --- /dev/null +++ b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php @@ -0,0 +1,26 @@ +sqlStatements = $sql; + } + + public function execute(Connection $conn, array $params, array $types): Result + { + return $conn->executeQuery($this->getSqlStatements(), $params, $types, $this->queryCacheProfile); + } +} diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php new file mode 100644 index 00000000000..1420bf39c1a --- /dev/null +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php @@ -0,0 +1,60 @@ +sql = $sql; + } + + /** + * @internal + * + * @psalm-internal Doctrine\ORM + * + * This method exists temporarily to support old SqlWalker interfaces. + */ + public function finalizeSql(Query $query): string + { + $platform = $query->getEntityManager()->getConnection()->getDatabasePlatform(); + + $sql = $platform->modifyLimitQuery($this->sql, $query->getMaxResults(), $query->getFirstResult()); + + $lockMode = $query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; + + if ($lockMode !== LockMode::NONE && $lockMode !== LockMode::OPTIMISTIC && $lockMode !== LockMode::PESSIMISTIC_READ && $lockMode !== LockMode::PESSIMISTIC_WRITE) { + throw QueryException::invalidLockMode(); + } + + if ($lockMode === LockMode::PESSIMISTIC_READ) { + $sql .= ' ' . $platform->getReadLockSQL(); + } elseif ($lockMode === LockMode::PESSIMISTIC_WRITE) { + $sql .= ' ' . $platform->getWriteLockSQL(); + } + + return $sql; + } + + public function createExecutor(Query $query): AbstractSqlExecutor + { + return new FinalizedSelectExecutor($this->finalizeSql($query)); + } +} From 04b89f5ce8120320a90061f66ba7639b49ebc894 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 27 Jan 2024 15:26:21 +0100 Subject: [PATCH 04/41] In SqlWalker, split SQL query generation into the two parts that shall happen before and after the finalization phase. Move the part that generates "pre-finalization" SQL into a dedicated method. Use a side channel in SingleSelectSqlFinalizer to access the "finalization" logic and avoid duplication. --- lib/Doctrine/ORM/Query/SqlWalker.php | 44 +++++++++++++--------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 34e5b22580d..870ccb67f02 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -546,10 +546,15 @@ private function generateFilterConditionSQL( */ public function walkSelectStatement(AST\SelectStatement $AST) { - $limit = $this->query->getMaxResults(); - $offset = $this->query->getFirstResult(); - $lockMode = $this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; - $sql = $this->walkSelectClause($AST->selectClause) + $sql = $this->createSqlForFinalizer($AST); + $finalizer = new Exec\SingleSelectSqlFinalizer($sql); + + return $finalizer->finalizeSql($this->query); + } + + protected function createSqlForFinalizer(AST\SelectStatement $AST): string + { + $sql = $this->walkSelectClause($AST->selectClause) . $this->walkFromClause($AST->fromClause) . $this->walkWhereClause($AST->whereClause); @@ -570,31 +575,22 @@ public function walkSelectStatement(AST\SelectStatement $AST) $sql .= ' ORDER BY ' . $orderBySql; } - $sql = $this->platform->modifyLimitQuery($sql, $limit, $offset); - - if ($lockMode === LockMode::NONE) { - return $sql; - } - - if ($lockMode === LockMode::PESSIMISTIC_READ) { - return $sql . ' ' . $this->platform->getReadLockSQL(); - } + $this->assertOptimisticLockingHasAllClassesVersioned(); - if ($lockMode === LockMode::PESSIMISTIC_WRITE) { - return $sql . ' ' . $this->platform->getWriteLockSQL(); - } + return $sql; + } - if ($lockMode !== LockMode::OPTIMISTIC) { - throw QueryException::invalidLockMode(); - } + private function assertOptimisticLockingHasAllClassesVersioned(): void + { + $lockMode = $this->query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; - foreach ($this->selectedClasses as $selectedClass) { - if (! $selectedClass['class']->isVersioned) { - throw OptimisticLockException::lockFailed($selectedClass['class']->name); + if ($lockMode === LockMode::OPTIMISTIC) { + foreach ($this->selectedClasses as $selectedClass) { + if (! $selectedClass['class']->isVersioned) { + throw OptimisticLockException::lockFailed($selectedClass['class']->name); + } } } - - return $sql; } /** From 9f085f85d6ac585aa1ad6326c9e4d82a3b61601d Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 27 Jan 2024 15:32:17 +0100 Subject: [PATCH 05/41] Fix CS violations --- lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php | 5 +++++ .../Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php index b777b3d68cc..87a5ebcebd6 100644 --- a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php @@ -6,6 +6,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Result; +use Doctrine\DBAL\Types\Type; /** * SQL executor for a given, final, single SELECT SQL query @@ -19,6 +20,10 @@ public function __construct(string $sql) $this->sqlStatements = $sql; } + /** + * @param list|array $params + * @param array|array $types + */ public function execute(Connection $conn, array $params, array $types): Result { return $conn->executeQuery($this->getSqlStatements(), $params, $types, $this->queryCacheProfile); diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php index 4ecbea9d3e3..3dbdb533b33 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php @@ -26,16 +26,16 @@ public function testSimpleQueryHasLimitAndOffsetApplied(): void $query->setMaxResults(10); $query->setFirstResult(20); - $sql10_20 = $query->getSQL(); + $sqlMax10First20 = $query->getSQL(); $query->setMaxResults(30); $query->setFirstResult(40); - $sql30_40 = $query->getSQL(); + $sqlMax30First40 = $query->getSQL(); // The SQL is platform specific and may even be something with outer SELECTS being added. So, // derive the expected value at runtime through the platform. - self::assertSame($platform->modifyLimitQuery($originalSql, 10, 20), $sql10_20); - self::assertSame($platform->modifyLimitQuery($originalSql, 30, 40), $sql30_40); + self::assertSame($platform->modifyLimitQuery($originalSql, 10, 20), $sqlMax10First20); + self::assertSame($platform->modifyLimitQuery($originalSql, 30, 40), $sqlMax30First40); $cacheEntries = self::$queryCache->getValues(); self::assertCount(1, $cacheEntries); From 89e5084481d098121239a5dd4fe5278ce7a938ad Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 27 Jan 2024 15:34:12 +0100 Subject: [PATCH 06/41] Skip the GH11112 test while applying refactorings --- tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php index 3dbdb533b33..a3a5a683457 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php @@ -12,6 +12,8 @@ class GH11112Test extends OrmFunctionalTestCase { protected function setUp(): void { + $this->markTestSkipped('Skipping test while applying refactorings'); + $this->useModelSet('cms'); self::$queryCache = new ArrayAdapter(); From 67f3108360d79c2ce78f5ca97acf45a2d4be644d Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sun, 28 Jan 2024 23:54:05 +0100 Subject: [PATCH 07/41] Avoid a Psalm complaint due to invalid (?) docblock syntax --- lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php index 1420bf39c1a..4f60686a9da 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php @@ -26,11 +26,10 @@ public function __construct(string $sql) } /** - * @internal - * - * @psalm-internal Doctrine\ORM - * * This method exists temporarily to support old SqlWalker interfaces. + * + * @internal + * @psalm-internal Doctrine\ORM\Query */ public function finalizeSql(Query $query): string { From 47ff89681bf16295faec157ff7e34fcba2ff836e Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 10:00:38 +0100 Subject: [PATCH 08/41] Establish alternate code path - queries can obtain the sql executor through the finalizer, parser knows about output walkers yielding finalizers --- lib/Doctrine/ORM/Query.php | 46 +++++++++++++++++-- .../Query/Exec/PreparedExecutorFinalizer.php | 30 ++++++++++++ lib/Doctrine/ORM/Query/Parser.php | 11 +++-- lib/Doctrine/ORM/Query/ParserResult.php | 29 ++++++++++++ lib/Doctrine/ORM/Query/SqlOutputWalker.php | 21 +++++++++ 5 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php create mode 100644 lib/Doctrine/ORM/Query/SqlOutputWalker.php diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index a9be33b4da3..a48d2060d10 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -18,6 +18,7 @@ use Doctrine\ORM\Query\AST\SelectStatement; use Doctrine\ORM\Query\AST\UpdateStatement; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; +use Doctrine\ORM\Query\OutputWalker; use Doctrine\ORM\Query\Parameter; use Doctrine\ORM\Query\ParameterTypeInferer; use Doctrine\ORM\Query\Parser; @@ -33,6 +34,7 @@ use function count; use function get_debug_type; use function in_array; +use function is_a; use function is_int; use function ksort; use function md5; @@ -147,6 +149,11 @@ class Query extends AbstractQuery */ private $parserResult; + /** + * @var AbstractSqlExecutor + */ + private $sqlExecutor = null; + /** * The first result to return (the "offset"). * @@ -196,7 +203,7 @@ class Query extends AbstractQuery */ public function getSQL() { - return $this->parse()->getSqlExecutor()->getSqlStatements(); + return $this->getSqlExecutor()->getSqlStatements(); } /** @@ -254,6 +261,7 @@ private function parse(): ParserResult $parser = new Parser($this); $this->parserResult = $parser->parse(); + $this->initializeSqlExecutor(); return $this->parserResult; } @@ -265,6 +273,7 @@ private function parse(): ParserResult if ($cached instanceof ParserResult) { // Cache hit. $this->parserResult = $cached; + $this->initializeSqlExecutor(); return $this->parserResult; } @@ -277,15 +286,27 @@ private function parse(): ParserResult $queryCache->save($cacheItem->set($this->parserResult)->expiresAfter($this->queryCacheTTL)); + $this->initializeSqlExecutor(); + return $this->parserResult; } + private function initializeSqlExecutor(): void + { + // This will be the only code path in 3.0 + if ($this->parserResult->hasSqlFinalizer()) { + $this->sqlExecutor = $this->parserResult->getSqlFinalizer()->createExecutor($this); + } else { + $this->sqlExecutor = $this->parserResult->getSqlExecutor(); + } + } + /** * {@inheritDoc} */ protected function _doExecute() { - $executor = $this->parse()->getSqlExecutor(); + $executor = $this->getSqlExecutor(); if ($this->_queryCacheProfile) { $executor->setQueryCacheProfile($this->_queryCacheProfile); @@ -812,11 +833,23 @@ protected function getQueryCacheId(): string { ksort($this->_hints); + if (! $this->hasHint(self::HINT_CUSTOM_OUTPUT_WALKER)) { + // Assume Parser will create the SqlOutputWalker; save is_a call, which might trigger a class load + $firstAndMaxResult = ''; + } else { + $outputWalkerClass = $this->getHint(self::HINT_CUSTOM_OUTPUT_WALKER); + if (is_a($outputWalkerClass, OutputWalker::class, true)) { + $firstAndMaxResult = ''; + } else { + $firstAndMaxResult = '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults; + } + } + return md5( $this->getDQL() . serialize($this->_hints) . '&platform=' . get_debug_type($this->getEntityManager()->getConnection()->getDatabasePlatform()) . ($this->_em->hasFilters() ? $this->_em->getFilters()->getHash() : '') . - '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults . + $firstAndMaxResult . '&hydrationMode=' . $this->_hydrationMode . '&types=' . serialize($this->parsedTypes) . 'DOCTRINE_QUERY_CACHE_SALT' ); } @@ -835,4 +868,11 @@ public function __clone() $this->state = self::STATE_DIRTY; } + + private function getSqlExecutor(): AbstractSqlExecutor + { + $this->parse(); + + return $this->sqlExecutor; + } } diff --git a/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php b/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php new file mode 100644 index 00000000000..0201266c211 --- /dev/null +++ b/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php @@ -0,0 +1,30 @@ +executor = $exeutor; + } + + public function createExecutor(Query $query): AbstractSqlExecutor + { + return $this->executor; + } +} diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index 7e69b4865d0..edde449d2c4 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -391,11 +391,16 @@ public function parse() $this->queryComponents = $treeWalkerChain->getQueryComponents(); } - $outputWalkerClass = $this->customOutputWalker ?: SqlWalker::class; + $outputWalkerClass = $this->customOutputWalker ?: SqlOutputWalker::class; $outputWalker = new $outputWalkerClass($this->query, $this->parserResult, $this->queryComponents); - // Assign an SQL executor to the parser result - $this->parserResult->setSqlExecutor($outputWalker->getExecutor($AST)); + if ($outputWalker instanceof OutputWalker) { + $finalizer = $outputWalker->getFinalizer($AST); + $this->parserResult->setSqlFinalizer($finalizer); + } else { + $executor = $outputWalker->getExecutor($AST); + $this->parserResult->setSqlExecutor($executor); + } return $this->parserResult; } diff --git a/lib/Doctrine/ORM/Query/ParserResult.php b/lib/Doctrine/ORM/Query/ParserResult.php index e5d36c9bbe3..ef78cc5e4cf 100644 --- a/lib/Doctrine/ORM/Query/ParserResult.php +++ b/lib/Doctrine/ORM/Query/ParserResult.php @@ -5,6 +5,8 @@ namespace Doctrine\ORM\Query; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use RuntimeException; use function sprintf; @@ -20,6 +22,7 @@ class ParserResult 'sqlExecutor' => '_sqlExecutor', 'resultSetMapping' => '_resultSetMapping', 'parameterMappings' => '_parameterMappings', + 'sqlFinalizer' => 'sqlFinalizer', ]; /** @@ -29,6 +32,13 @@ class ParserResult */ private $sqlExecutor; + /** + * The SQL executor used for executing the SQL. + * + * @var SqlFinalizer + */ + private $sqlFinalizer = null; + /** * The ResultSetMapping that describes how to map the SQL result set. * @@ -94,6 +104,25 @@ public function getSqlExecutor() return $this->sqlExecutor; } + public function setSqlFinalizer(SqlFinalizer $finalizer): void + { + $this->sqlFinalizer = $finalizer; + } + + public function getSqlFinalizer(): SqlFinalizer + { + if ($this->sqlFinalizer === null) { + throw new RuntimeException('This ParserResult was not created with an SqlFinalizer'); + } + + return $this->sqlFinalizer; + } + + public function hasSqlFinalizer(): bool + { + return $this->sqlFinalizer !== null; + } + /** * Adds a DQL to SQL parameter mapping. One DQL parameter name/position can map to * several SQL parameter positions. diff --git a/lib/Doctrine/ORM/Query/SqlOutputWalker.php b/lib/Doctrine/ORM/Query/SqlOutputWalker.php new file mode 100644 index 00000000000..bef3adc5441 --- /dev/null +++ b/lib/Doctrine/ORM/Query/SqlOutputWalker.php @@ -0,0 +1,21 @@ +createSqlForFinalizer($AST)); + } +} From 9b6c9dd49cf8092323bf12ded48dff12d9cf1f90 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 10:04:09 +0100 Subject: [PATCH 09/41] Remove a possibly premature comment --- lib/Doctrine/ORM/Query.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index a48d2060d10..bd309267c31 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -293,7 +293,6 @@ private function parse(): ParserResult private function initializeSqlExecutor(): void { - // This will be the only code path in 3.0 if ($this->parserResult->hasSqlFinalizer()) { $this->sqlExecutor = $this->parserResult->getSqlFinalizer()->createExecutor($this); } else { From 77062c4554b71a8c4954c7d9ba5e58f48f4ed43c Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 10:07:12 +0100 Subject: [PATCH 10/41] Re-enable the #11112 test --- tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php index a3a5a683457..3dbdb533b33 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH11112Test.php @@ -12,8 +12,6 @@ class GH11112Test extends OrmFunctionalTestCase { protected function setUp(): void { - $this->markTestSkipped('Skipping test while applying refactorings'); - $this->useModelSet('cms'); self::$queryCache = new ArrayAdapter(); From 9695ec6530d015cb5fa8215df29ce06827d1364d Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 10:08:59 +0100 Subject: [PATCH 11/41] Fix CS --- lib/Doctrine/ORM/Query.php | 4 +--- lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php | 4 +--- lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php | 1 + 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index bd309267c31..a8ccf3ae503 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -149,9 +149,7 @@ class Query extends AbstractQuery */ private $parserResult; - /** - * @var AbstractSqlExecutor - */ + /** @var AbstractSqlExecutor */ private $sqlExecutor = null; /** diff --git a/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php b/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php index 0201266c211..447410e5cb1 100644 --- a/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php +++ b/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php @@ -13,9 +13,7 @@ */ final class PreparedExecutorFinalizer implements SqlFinalizer { - /** - * @var AbstractSqlExecutor - */ + /** @var AbstractSqlExecutor */ private $executor; public function __construct(AbstractSqlExecutor $exeutor) diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php index 4f60686a9da..45008b2c4b8 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php @@ -29,6 +29,7 @@ public function __construct(string $sql) * This method exists temporarily to support old SqlWalker interfaces. * * @internal + * * @psalm-internal Doctrine\ORM\Query */ public function finalizeSql(Query $query): string From a477dfed49d5b7c4a9ecdc42d6d3e001c3b81810 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 10:53:05 +0100 Subject: [PATCH 12/41] Make RootTypeWalker inherit from SqlOutputWalker so it becomes finalizer-aware --- .../ORM/Tools/Pagination/RootTypeWalker.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/RootTypeWalker.php b/lib/Doctrine/ORM/Tools/Pagination/RootTypeWalker.php index dc1d77a51c7..ec45dbd6fc1 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/RootTypeWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/RootTypeWalker.php @@ -5,7 +5,10 @@ namespace Doctrine\ORM\Tools\Pagination; use Doctrine\ORM\Query\AST; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\Exec\FinalizedSelectExecutor; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use Doctrine\ORM\Query\SqlOutputWalker; use Doctrine\ORM\Utility\PersisterHelper; use RuntimeException; @@ -22,7 +25,7 @@ * Returning the type instead of a "real" SQL statement is a slight hack. However, it has the * benefit that the DQL -> root entity id type resolution can be cached in the query cache. */ -final class RootTypeWalker extends SqlWalker +final class RootTypeWalker extends SqlOutputWalker { public function walkSelectStatement(AST\SelectStatement $AST): string { @@ -45,4 +48,13 @@ public function walkSelectStatement(AST\SelectStatement $AST): string ->getEntityManager() )[0]; } + + public function getFinalizer($AST): SqlFinalizer + { + if (! $AST instanceof AST\SelectStatement) { + throw new RuntimeException(self::class . ' is to be used on SelectStatements only'); + } + + return new PreparedExecutorFinalizer(new FinalizedSelectExecutor($this->walkSelectStatement($AST))); + } } From c098fbc36220c80c2f9eb33895e4a0ff2af80530 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 10:54:13 +0100 Subject: [PATCH 13/41] Update QueryCacheTest, since first/max results no longer need extra cache entries --- tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php index 4935c3d78ee..3894ecef697 100644 --- a/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php @@ -43,7 +43,7 @@ public function testQueryCacheDependsOnHints(): array } /** @depends testQueryCacheDependsOnHints */ - public function testQueryCacheDependsOnFirstResult(array $previous): void + public function testQueryCacheDoesNotDependOnFirstResultForDefaultOutputWalker(array $previous): void { [$query, $cache] = $previous; assert($query instanceof Query); @@ -55,11 +55,11 @@ public function testQueryCacheDependsOnFirstResult(array $previous): void $query->setMaxResults(9999); $query->getResult(); - self::assertCount($cacheCount + 1, $cache->getValues()); + self::assertCount($cacheCount, $cache->getValues()); } /** @depends testQueryCacheDependsOnHints */ - public function testQueryCacheDependsOnMaxResults(array $previous): void + public function testQueryCacheDoesNotDependOnMaxResultsForDefaultOutputWalker(array $previous): void { [$query, $cache] = $previous; assert($query instanceof Query); @@ -70,7 +70,7 @@ public function testQueryCacheDependsOnMaxResults(array $previous): void $query->setMaxResults(10); $query->getResult(); - self::assertCount($cacheCount + 1, $cache->getValues()); + self::assertCount($cacheCount, $cache->getValues()); } /** @depends testQueryCacheDependsOnHints */ From 5b7a340f70faf7dcef6f6337b1119e4b253cfc87 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 11:04:19 +0100 Subject: [PATCH 14/41] Fix ParserResultSerializationTest by forcing the parser to produce a ParserResult of the old kind (with the executor already constructed) --- .../Tests/ORM/Functional/ParserResultSerializationTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php index 0841c5ff3a0..c5e0ed1237a 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -75,6 +75,9 @@ public function testItSerializesParserResultWithAForwardCompatibleFormat(): void $query = $this->_em ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + // Use the (legacy) SqlWalker which directly puts an SqlExecutor instance into the parser result + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, Query\SqlWalker::class); + $parserResult = self::parseQuery($query); $serialized = serialize($parserResult); $this->assertStringNotContainsString( From 1573420593fdc51918c5ee41a0ae78f562139561 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 11:06:47 +0100 Subject: [PATCH 15/41] Fix WhereInWalkerTest --- .../Tests/ORM/Tools/Pagination/WhereInWalkerTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php index 1f1710d2cf1..4d27d1792fd 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php @@ -21,9 +21,10 @@ public function testDqlQueryTransformation(string $dql, string $expectedSql): vo $query->setHint(Query::HINT_CUSTOM_TREE_WALKERS, [WhereInWalker::class]); $query->setHint(WhereInWalker::HINT_PAGINATOR_HAS_IDS, true); - $result = (new Parser($query))->parse(); + $result = (new Parser($query))->parse(); + $executor = $result->getSqlFinalizer()->createExecutor($query); - self::assertEquals($expectedSql, $result->getSqlExecutor()->getSqlStatements()); + self::assertEquals($expectedSql, $executor->getSqlStatements()); self::assertEquals([0], $result->getSqlParameterPositions(WhereInWalker::PAGINATOR_ID_ALIAS)); } From 293771dd214a8cfebd00771610ede9d64511e6b3 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 16:23:02 +0100 Subject: [PATCH 16/41] Update lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Grégoire Paris --- lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php b/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php index 447410e5cb1..b89b124e352 100644 --- a/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php +++ b/lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php @@ -8,7 +8,7 @@ /** * PreparedExecutorFinalizer is a wrapper for the SQL finalization - * phase that does nothing - it constructed with the sql executor + * phase that does nothing - it is constructed with the sql executor * already. */ final class PreparedExecutorFinalizer implements SqlFinalizer From 4f0c01f3d1305cc17f3588f49124d865cc44c74c Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 16:35:10 +0100 Subject: [PATCH 17/41] Fix tests --- .../ParserResultSerializationTest.php | 26 ++++++++++++++++++- .../ORM/Tools/Pagination/PaginatorTest.php | 6 +++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php index c5e0ed1237a..5ded9aeeae2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -35,20 +35,44 @@ protected function setUp(): void * * @dataProvider provideToSerializedAndBack */ - public function testSerializeParserResult(Closure $toSerializedAndBack): void + public function testSerializeParserResultForQueryWithSqlWalker(Closure $toSerializedAndBack): void { $query = $this->_em ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + // Use the (legacy) SqlWalker which directly puts an SqlExecutor instance into the parser result + $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, Query\SqlWalker::class); + $parserResult = self::parseQuery($query); $unserialized = $toSerializedAndBack($parserResult); $this->assertInstanceOf(ParserResult::class, $unserialized); $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); + $this->assertFalse($unserialized->hasSqlFinalizer()); $this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor()); } + /** + * @param Closure(ParserResult): ParserResult $toSerializedAndBack + * + * @dataProvider provideToSerializedAndBack + */ + public function testSerializeParserResultForQueryWithSqlOutputWalker(Closure $toSerializedAndBack): void + { + $query = $this->_em + ->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyEmployee u WHERE u.name = :name'); + + $parserResult = self::parseQuery($query); + $unserialized = $toSerializedAndBack($parserResult); + + $this->assertInstanceOf(ParserResult::class, $unserialized); + $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); + $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); + $this->assertTrue($unserialized->hasSqlFinalizer()); + $this->assertNotNull($unserialized->getSqlFinalizer()); + } + /** @return Generator */ public function provideToSerializedAndBack(): Generator { diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginatorTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginatorTest.php index 1c8b47ac0cf..23ff3b90be4 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginatorTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/PaginatorTest.php @@ -87,7 +87,8 @@ public function testExtraParametersAreStrippedWhenWalkerRemovingOriginalSelectEl public function testPaginatorNotCaringAboutExtraParametersWithoutOutputWalkers(): void { - $this->connection->expects(self::exactly(3))->method('executeQuery'); + $result = $this->getMockBuilder(Result::class)->disableOriginalConstructor()->getMock(); + $this->connection->expects(self::exactly(3))->method('executeQuery')->willReturn($result); $this->createPaginatorWithExtraParametersWithoutOutputWalkers([])->count(); $this->createPaginatorWithExtraParametersWithoutOutputWalkers([[10]])->count(); @@ -96,7 +97,8 @@ public function testPaginatorNotCaringAboutExtraParametersWithoutOutputWalkers() public function testgetIteratorDoesCareAboutExtraParametersWithoutOutputWalkersWhenResultIsNotEmpty(): void { - $this->connection->expects(self::exactly(1))->method('executeQuery'); + $result = $this->getMockBuilder(Result::class)->disableOriginalConstructor()->getMock(); + $this->connection->expects(self::exactly(1))->method('executeQuery')->willReturn($result); $this->expectException(Query\QueryException::class); $this->expectExceptionMessage('Too many parameters: the query defines 1 parameters and you bound 2'); From 302a9b78feaa2bb52d3901b0e90d822c3483933e Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 16:44:50 +0100 Subject: [PATCH 18/41] Fix a Psalm complaint --- lib/Doctrine/ORM/Query.php | 4 ++-- lib/Doctrine/ORM/Query/ParserResult.php | 10 +++++++--- psalm-baseline.xml | 22 ++++++++++++++++------ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index a8ccf3ae503..34c349de45f 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -149,8 +149,8 @@ class Query extends AbstractQuery */ private $parserResult; - /** @var AbstractSqlExecutor */ - private $sqlExecutor = null; + /** @var ?AbstractSqlExecutor */ + private $sqlExecutor; /** * The first result to return (the "offset"). diff --git a/lib/Doctrine/ORM/Query/ParserResult.php b/lib/Doctrine/ORM/Query/ParserResult.php index ef78cc5e4cf..fedbfe28837 100644 --- a/lib/Doctrine/ORM/Query/ParserResult.php +++ b/lib/Doctrine/ORM/Query/ParserResult.php @@ -28,16 +28,16 @@ class ParserResult /** * The SQL executor used for executing the SQL. * - * @var AbstractSqlExecutor + * @var ?AbstractSqlExecutor */ private $sqlExecutor; /** * The SQL executor used for executing the SQL. * - * @var SqlFinalizer + * @var ?SqlFinalizer */ - private $sqlFinalizer = null; + private $sqlFinalizer; /** * The ResultSetMapping that describes how to map the SQL result set. @@ -101,6 +101,10 @@ public function setSqlExecutor($executor) */ public function getSqlExecutor() { + if ($this->sqlExecutor === null) { + throw new RuntimeException('This ParserResult was not created with an SqlExecutor'); + } + return $this->sqlExecutor; } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 90cbc63a1c0..878893cc7c3 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1445,12 +1445,18 @@ $sqlParams + + AbstractSqlExecutor + - parse()->getSqlExecutor()->getSqlStatements()]]> + getSqlExecutor()->getSqlStatements()]]> |string]]> + + sqlExecutor]]> + getDQL()]]> @@ -1866,6 +1872,15 @@ _sqlStatements = &$this->sqlStatements]]> + + + getSqlStatements()]]> + + + FinalizedSelectExecutor + FinalizedSelectExecutor + + $numDeleted @@ -2058,11 +2073,6 @@ $token === Lexer::T_IDENTIFIER - - - $sqlExecutor - - parameters)]]> From 02cad0b2471fb39ed245443934364efe1a586cb8 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 20:59:07 +0100 Subject: [PATCH 19/41] Fix a test --- lib/Doctrine/ORM/Query/ParserResult.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Query/ParserResult.php b/lib/Doctrine/ORM/Query/ParserResult.php index fedbfe28837..6afa9f5d472 100644 --- a/lib/Doctrine/ORM/Query/ParserResult.php +++ b/lib/Doctrine/ORM/Query/ParserResult.php @@ -97,13 +97,10 @@ public function setSqlExecutor($executor) /** * Gets the SQL executor used by this ParserResult. * - * @return AbstractSqlExecutor + * @return ?AbstractSqlExecutor */ public function getSqlExecutor() { - if ($this->sqlExecutor === null) { - throw new RuntimeException('This ParserResult was not created with an SqlExecutor'); - } return $this->sqlExecutor; } @@ -122,6 +119,12 @@ public function getSqlFinalizer(): SqlFinalizer return $this->sqlFinalizer; } + /** + * @internal + * + * @psalm-internal Doctrine\ORM\Query + * @psalm-assert-if-true !null $this->sqlFinalizer + */ public function hasSqlFinalizer(): bool { return $this->sqlFinalizer !== null; From c27c800c48843c42b8f07e91e8c92687c74a3997 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 21:05:41 +0100 Subject: [PATCH 20/41] Fix CS --- lib/Doctrine/ORM/Query/ParserResult.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/ParserResult.php b/lib/Doctrine/ORM/Query/ParserResult.php index 6afa9f5d472..75797317219 100644 --- a/lib/Doctrine/ORM/Query/ParserResult.php +++ b/lib/Doctrine/ORM/Query/ParserResult.php @@ -101,7 +101,6 @@ public function setSqlExecutor($executor) */ public function getSqlExecutor() { - return $this->sqlExecutor; } From 6b4c9a21ddf944dc530d254b914784c4e0aec004 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 29 Jan 2024 22:30:42 +0100 Subject: [PATCH 21/41] Make the NullSqlWalker an instance of SqlOutputWalker --- tests/Doctrine/Tests/Mocks/NullSqlWalker.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/Doctrine/Tests/Mocks/NullSqlWalker.php b/tests/Doctrine/Tests/Mocks/NullSqlWalker.php index 9d54e703547..8cd4884811d 100644 --- a/tests/Doctrine/Tests/Mocks/NullSqlWalker.php +++ b/tests/Doctrine/Tests/Mocks/NullSqlWalker.php @@ -7,12 +7,14 @@ use Doctrine\DBAL\Connection; use Doctrine\ORM\Query\AST; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; +use Doctrine\ORM\Query\SqlOutputWalker; /** * SqlWalker implementation that does not produce SQL. */ -class NullSqlWalker extends SqlWalker +class NullSqlWalker extends SqlOutputWalker { public function walkSelectStatement(AST\SelectStatement $AST): string { @@ -29,13 +31,15 @@ public function walkDeleteStatement(AST\DeleteStatement $AST): string return ''; } - public function getExecutor($AST): AbstractSqlExecutor + public function getFinalizer($AST): SqlFinalizer { - return new class extends AbstractSqlExecutor { - public function execute(Connection $conn, array $params, array $types): int - { - return 0; + return new PreparedExecutorFinalizer( + new class extends AbstractSqlExecutor { + public function execute(Connection $conn, array $params, array $types) + { + return 0; + } } - }; + ); } } From eb6479521476e7a79c7a14ab77c2306024e99fcf Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Tue, 30 Jan 2024 16:26:16 +0100 Subject: [PATCH 22/41] Avoid multiple cache entries caused by LimitSubqueryOutputWalker --- .../Pagination/LimitSubqueryOutputWalker.php | 63 +++++++++++++------ .../LimitSubqueryOutputWalkerTest.php | 23 +++++++ 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index f92c1145d4f..0b8c58ae9a2 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -18,11 +18,13 @@ use Doctrine\ORM\Query\AST\PathExpression; use Doctrine\ORM\Query\AST\SelectExpression; use Doctrine\ORM\Query\AST\SelectStatement; +use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Query\Parser; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\SqlOutputWalker; use RuntimeException; use function array_diff; @@ -50,7 +52,7 @@ * * @psalm-import-type QueryComponent from Parser */ -class LimitSubqueryOutputWalker extends SqlWalker +class LimitSubqueryOutputWalker extends SqlOutputWalker { private const ORDER_BY_PATH_EXPRESSION = '/(?platform = $query->getEntityManager()->getConnection()->getDatabasePlatform(); $this->rsm = $parserResult->getResultSetMapping(); + $query = clone $query; + // Reset limit and offset $this->firstResult = $query->getFirstResult(); $this->maxResults = $query->getMaxResults(); @@ -158,11 +162,28 @@ private function rebuildOrderByForRowNumber(SelectStatement $AST): void */ public function walkSelectStatement(SelectStatement $AST) { + $sqlFinalizer = $this->getFinalizer($AST); + + $query = $this->getQuery(); + + $abstractSqlExecutor = $sqlFinalizer->createExecutor($query); + + return $abstractSqlExecutor->getSqlStatements(); + } + + public function getFinalizer($AST): SqlFinalizer + { + if (! $AST instanceof SelectStatement) { + throw new RuntimeException(self::class . ' is to be used on SelectStatements only'); + } + if ($this->platformSupportsRowNumber()) { - return $this->walkSelectStatementWithRowNumber($AST); + $sql = $this->createSqlWithRowNumber($AST); + } else { + $sql = $this->createSqlWithoutRowNumber($AST); } - return $this->walkSelectStatementWithoutRowNumber($AST); + return new SingleSelectSqlFinalizer($sql); } /** @@ -174,6 +195,16 @@ public function walkSelectStatement(SelectStatement $AST) * @throws RuntimeException */ public function walkSelectStatementWithRowNumber(SelectStatement $AST) + { + // Apply the limit and offset. + return $this->platform->modifyLimitQuery( + $this->createSqlWithRowNumber($AST), + $this->maxResults, + $this->firstResult + ); + } + + private function createSqlWithRowNumber(SelectStatement $AST): string { $hasOrderBy = false; $outerOrderBy = ' ORDER BY dctrn_minrownum ASC'; @@ -203,13 +234,6 @@ public function walkSelectStatementWithRowNumber(SelectStatement $AST) $sql .= $orderGroupBy . $outerOrderBy; } - // Apply the limit and offset. - $sql = $this->platform->modifyLimitQuery( - $sql, - $this->maxResults, - $this->firstResult - ); - // Add the columns to the ResultSetMapping. It's not really nice but // it works. Preferably I'd clear the RSM or simply create a new one // but that is not possible from inside the output walker, so we dirty @@ -232,6 +256,16 @@ public function walkSelectStatementWithRowNumber(SelectStatement $AST) * @throws RuntimeException */ public function walkSelectStatementWithoutRowNumber(SelectStatement $AST, $addMissingItemsFromOrderByToSelect = true) + { + // Apply the limit and offset. + return $this->platform->modifyLimitQuery( + $this->createSqlWithoutRowNumber($AST, $addMissingItemsFromOrderByToSelect), + $this->maxResults, + $this->firstResult + ); + } + + private function createSqlWithoutRowNumber(SelectStatement $AST, $addMissingItemsFromOrderByToSelect = true): string { // We don't want to call this recursively! if ($AST->orderByClause instanceof OrderByClause && $addMissingItemsFromOrderByToSelect) { @@ -260,13 +294,6 @@ public function walkSelectStatementWithoutRowNumber(SelectStatement $AST, $addMi // https://github.com/doctrine/orm/issues/2630 $sql = $this->preserveSqlOrdering($sqlIdentifier, $innerSql, $sql, $orderByClause); - // Apply the limit and offset. - $sql = $this->platform->modifyLimitQuery( - $sql, - $this->maxResults, - $this->firstResult - ); - // Add the columns to the ResultSetMapping. It's not really nice but // it works. Preferably I'd clear the RSM or simply create a new one // but that is not possible from inside the output walker, so we dirty diff --git a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php index cb69a3a5674..95d2047d593 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php @@ -10,6 +10,7 @@ use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\ORM\Query; use Doctrine\ORM\Tools\Pagination\LimitSubqueryOutputWalker; +use Symfony\Component\Cache\Adapter\ArrayAdapter; use function class_exists; @@ -385,6 +386,28 @@ public function testLimitSubqueryOrderBySubSelectOrderByExpressionOracle(): void ); } + public function testParsingQueryWithDifferentLimitOffsetValuesTakesOnlyOneCacheEntry(): void + { + $queryCache = new ArrayAdapter(); + $this->entityManager->getConfiguration()->setQueryCache($queryCache); + + $query = $this->createQuery('SELECT p, c, a FROM Doctrine\Tests\ORM\Tools\Pagination\MyBlogPost p JOIN p.category c JOIN p.author a'); + + self::assertSame( + 'SELECT DISTINCT id_0 FROM (SELECT m0_.id AS id_0, m0_.title AS title_1, c1_.id AS id_2, a2_.id AS id_3, a2_.name AS name_4, m0_.author_id AS author_id_5, m0_.category_id AS category_id_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id) dctrn_result LIMIT 20 OFFSET 10', + $query->getSQL() + ); + + $query->setFirstResult(30)->setMaxResults(40); + + self::assertSame( + 'SELECT DISTINCT id_0 FROM (SELECT m0_.id AS id_0, m0_.title AS title_1, c1_.id AS id_2, a2_.id AS id_3, a2_.name AS name_4, m0_.author_id AS author_id_5, m0_.category_id AS category_id_6 FROM MyBlogPost m0_ INNER JOIN Category c1_ ON m0_.category_id = c1_.id INNER JOIN Author a2_ ON m0_.author_id = a2_.id) dctrn_result LIMIT 40 OFFSET 30', + $query->getSQL() + ); + + self::assertCount(1, $queryCache->getValues()); + } + private function createQuery(string $dql): Query { $query = $this->entityManager->createQuery($dql); From dee1818eb19fc9ee23ff5f73e7640af7bef1fe81 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Tue, 30 Jan 2024 22:28:40 +0100 Subject: [PATCH 23/41] Fix Psalm complaints --- lib/Doctrine/ORM/Query.php | 3 ++ .../Query/Exec/FinalizedSelectExecutor.php | 2 ++ .../ORM/Query/Exec/SingleSelectExecutor.php | 10 ++++++ .../Query/Exec/SingleSelectSqlFinalizer.php | 5 ++- lib/Doctrine/ORM/Query/Parser.php | 8 +++++ lib/Doctrine/ORM/Query/ParserResult.php | 16 +++++++++ lib/Doctrine/ORM/Query/SqlWalker.php | 36 +++++++++++++++++-- .../Tools/Pagination/CountOutputWalker.php | 11 +++--- .../Pagination/LimitSubqueryOutputWalker.php | 9 ++++- tests/Doctrine/Tests/Mocks/NullSqlWalker.php | 1 + .../Tests/ORM/Query/CustomTreeWalkersTest.php | 4 +-- 11 files changed, 92 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 34c349de45f..89e0653d8bf 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -291,6 +291,7 @@ private function parse(): ParserResult private function initializeSqlExecutor(): void { + // This will be the only code path in 3.0 if ($this->parserResult->hasSqlFinalizer()) { $this->sqlExecutor = $this->parserResult->getSqlFinalizer()->createExecutor($this); } else { @@ -830,6 +831,8 @@ protected function getQueryCacheId(): string { ksort($this->_hints); + // TODO: In 3.0, make the finalization phase mandatory for OutputWalkers, so we never need this check + if (! $this->hasHint(self::HINT_CUSTOM_OUTPUT_WALKER)) { // Assume Parser will create the SqlOutputWalker; save is_a call, which might trigger a class load $firstAndMaxResult = ''; diff --git a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php index 87a5ebcebd6..916c7b1eab4 100644 --- a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php @@ -10,6 +10,8 @@ /** * SQL executor for a given, final, single SELECT SQL query + * + * @method getSqlStatements(): string */ class FinalizedSelectExecutor extends AbstractSqlExecutor { diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php index fbcab590e09..fd3f75884ac 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php @@ -6,18 +6,28 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Result; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Query\AST\SelectStatement; use Doctrine\ORM\Query\SqlWalker; /** * Executor that executes the SQL statement for simple DQL SELECT statements. * + * @deprecated This class will be removed in 3.0 + * * @link www.doctrine-project.org */ class SingleSelectExecutor extends AbstractSqlExecutor { public function __construct(SelectStatement $AST, SqlWalker $sqlWalker) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'The %s class will be removed in Doctrine ORM 3.0', + self::class + ); + parent::__construct(); $this->sqlStatements = $sqlWalker->walkSelectStatement($AST); diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php index 45008b2c4b8..98bfc176ac9 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php @@ -30,7 +30,7 @@ public function __construct(string $sql) * * @internal * - * @psalm-internal Doctrine\ORM\Query + * @psalm-internal Doctrine\ORM */ public function finalizeSql(Query $query): string { @@ -53,6 +53,9 @@ public function finalizeSql(Query $query): string return $sql; } + /** + * @return FinalizedSelectExecutor + */ public function createExecutor(Query $query): AbstractSqlExecutor { return new FinalizedSelectExecutor($this->finalizeSql($query)); diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index edde449d2c4..df3f2a4cc22 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -391,6 +391,7 @@ public function parse() $this->queryComponents = $treeWalkerChain->getQueryComponents(); } + // TODO: In 3.0, add a runtime check that the class implements OutputWalker? $outputWalkerClass = $this->customOutputWalker ?: SqlOutputWalker::class; $outputWalker = new $outputWalkerClass($this->query, $this->parserResult, $this->queryComponents); @@ -398,6 +399,13 @@ public function parse() $finalizer = $outputWalker->getFinalizer($AST); $this->parserResult->setSqlFinalizer($finalizer); } else { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'In Doctrine ORM 3.0, output walkers will need to implement the %s interface', + OutputWalker::class + ); + $executor = $outputWalker->getExecutor($AST); $this->parserResult->setSqlExecutor($executor); } diff --git a/lib/Doctrine/ORM/Query/ParserResult.php b/lib/Doctrine/ORM/Query/ParserResult.php index 75797317219..59358c94ed3 100644 --- a/lib/Doctrine/ORM/Query/ParserResult.php +++ b/lib/Doctrine/ORM/Query/ParserResult.php @@ -4,6 +4,7 @@ namespace Doctrine\ORM\Query; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; use Doctrine\ORM\Query\Exec\SqlFinalizer; use RuntimeException; @@ -91,6 +92,13 @@ public function setResultSetMapping(ResultSetMapping $rsm) */ public function setSqlExecutor($executor) { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'In Doctrine ORM 3.0, the sqlExecutor will be removed from the ParserResult class.', + OutputWalker::class + ); + $this->sqlExecutor = $executor; } @@ -101,6 +109,13 @@ public function setSqlExecutor($executor) */ public function getSqlExecutor() { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'In Doctrine ORM 3.0, the sqlExecutor will be removed from the ParserResult class.', + OutputWalker::class + ); + return $this->sqlExecutor; } @@ -120,6 +135,7 @@ public function getSqlFinalizer(): SqlFinalizer /** * @internal + * @deprecated will be removed in 3.0, SqlFinalizers will have to be present in every ParserResult * * @psalm-internal Doctrine\ORM\Query * @psalm-assert-if-true !null $this->sqlFinalizer diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 870ccb67f02..49307380aa7 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -275,11 +275,11 @@ public function setQueryComponent($dqlAlias, array $queryComponent) /** * Gets an executor that can be used to execute the result of this walker. * + * @deprecated This method will be replaced by the getFinalizer() method in 3.0 + * * @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST * * @return Exec\AbstractSqlExecutor - * - * @not-deprecated */ public function getExecutor($AST) { @@ -593,6 +593,38 @@ private function assertOptimisticLockingHasAllClassesVersioned(): void } } +// public static function modifyLateQueryXX(string $sql, Query $query, AbstractPlatform $platform): string +// { +// $sqlWithLimit = self::modifyLimitQueryXX($sql, $query, $platform); +// $sqlWithLocking = self::modifyLockingQueryXX($sqlWithLimit, $query, $platform); +// +// return $sqlWithLocking; +// } +// +// private static function modifyLimitQueryXX(string $sql, Query $query, AbstractPlatform $platform): string +// { +// return $platform->modifyLimitQuery($sql, $query->getMaxResults(), $query->getFirstResult()); +// } +// +// private static function modifyLockingQueryXX(string $sql, Query $query, AbstractPlatform $platform): string +// { +// $lockMode = $query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; +// +// if ($lockMode !== LockMode::NONE && $lockMode !== LockMode::OPTIMISTIC && $lockMode !== LockMode::PESSIMISTIC_READ && $lockMode !== LockMode::PESSIMISTIC_WRITE) { +// throw QueryException::invalidLockMode(); +// } +// +// if ($lockMode === LockMode::PESSIMISTIC_READ) { +// return $sql . ' ' . $platform->getReadLockSQL(); +// } +// +// if ($lockMode === LockMode::PESSIMISTIC_WRITE) { +// return $sql . ' ' . $platform->getWriteLockSQL(); +// } +// +// return $sql; +// } + /** * Walks down an UpdateStatement AST node, thereby generating the appropriate SQL. * diff --git a/lib/Doctrine/ORM/Tools/Pagination/CountOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/CountOutputWalker.php index 0929e2ab065..92615f8aab5 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/CountOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/CountOutputWalker.php @@ -11,7 +11,7 @@ use Doctrine\ORM\Query\Parser; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\ResultSetMapping; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\SqlOutputWalker; use RuntimeException; use function array_diff; @@ -36,7 +36,7 @@ * * @psalm-import-type QueryComponent from Parser */ -class CountOutputWalker extends SqlWalker +class CountOutputWalker extends SqlOutputWalker { /** @var AbstractPlatform */ private $platform; @@ -62,16 +62,13 @@ public function __construct($query, $parserResult, array $queryComponents) parent::__construct($query, $parserResult, $queryComponents); } - /** - * {@inheritDoc} - */ - public function walkSelectStatement(SelectStatement $AST) + protected function createSqlForFinalizer(SelectStatement $AST): string { if ($this->platform instanceof SQLServerPlatform) { $AST->orderByClause = null; } - $sql = parent::walkSelectStatement($AST); + $sql = parent::createSqlForFinalizer($AST); if ($AST->groupByClause) { return sprintf( diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index 0b8c58ae9a2..a3fc7a8c367 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -17,7 +17,9 @@ use Doctrine\ORM\Query\AST\OrderByClause; use Doctrine\ORM\Query\AST\PathExpression; use Doctrine\ORM\Query\AST\SelectExpression; +use Doctrine\ORM\Query\AST\DeleteStatement; use Doctrine\ORM\Query\AST\SelectStatement; +use Doctrine\ORM\Query\AST\UpdateStatement; use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Query\Parser; @@ -171,6 +173,11 @@ public function walkSelectStatement(SelectStatement $AST) return $abstractSqlExecutor->getSqlStatements(); } + /** + * @param DeleteStatement|UpdateStatement|SelectStatement $AST + * + * @return SingleSelectSqlFinalizer + */ public function getFinalizer($AST): SqlFinalizer { if (! $AST instanceof SelectStatement) { @@ -265,7 +272,7 @@ public function walkSelectStatementWithoutRowNumber(SelectStatement $AST, $addMi ); } - private function createSqlWithoutRowNumber(SelectStatement $AST, $addMissingItemsFromOrderByToSelect = true): string + private function createSqlWithoutRowNumber(SelectStatement $AST, bool $addMissingItemsFromOrderByToSelect = true): string { // We don't want to call this recursively! if ($AST->orderByClause instanceof OrderByClause && $addMissingItemsFromOrderByToSelect) { diff --git a/tests/Doctrine/Tests/Mocks/NullSqlWalker.php b/tests/Doctrine/Tests/Mocks/NullSqlWalker.php index 8cd4884811d..19e16089c74 100644 --- a/tests/Doctrine/Tests/Mocks/NullSqlWalker.php +++ b/tests/Doctrine/Tests/Mocks/NullSqlWalker.php @@ -35,6 +35,7 @@ public function getFinalizer($AST): SqlFinalizer { return new PreparedExecutorFinalizer( new class extends AbstractSqlExecutor { + /** @return int */ public function execute(Connection $conn, array $params, array $types) { return 0; diff --git a/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php b/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php index b2267078ff5..e9ad14d8c6d 100644 --- a/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php +++ b/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php @@ -96,7 +96,7 @@ public function testSetUnknownQueryComponentThrowsException(): void $this->generateSql( 'select u from Doctrine\Tests\Models\CMS\CmsUser u', [], - AddUnknownQueryComponentWalker::class + AddUnknownQueryComponentOutputWalker::class ); } @@ -110,7 +110,7 @@ public function testSupportsSeveralHintsQueries(): void } } -class AddUnknownQueryComponentWalker extends Query\SqlWalker +class AddUnknownQueryComponentOutputWalker extends Query\SqlOutputWalker { public function walkSelectStatement(Query\AST\SelectStatement $selectStatement): void { From 2fd79eafae7a079d16a7c22219d21fe41c1ff050 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 31 Jan 2024 10:03:34 +0100 Subject: [PATCH 24/41] Fix static analysis complaints --- lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php | 2 +- lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php | 4 +--- .../ORM/Tools/Pagination/LimitSubqueryOutputWalker.php | 2 +- tests/Doctrine/Tests/Mocks/NullSqlWalker.php | 3 +-- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php index 916c7b1eab4..a97e6f428c5 100644 --- a/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/FinalizedSelectExecutor.php @@ -11,7 +11,7 @@ /** * SQL executor for a given, final, single SELECT SQL query * - * @method getSqlStatements(): string + * @method string getSqlStatements() */ class FinalizedSelectExecutor extends AbstractSqlExecutor { diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php index b553b5144c5..5180be9e7b2 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectSqlFinalizer.php @@ -56,9 +56,7 @@ public function finalizeSql(Query $query): string return $sql; } - /** - * @return FinalizedSelectExecutor - */ + /** @return FinalizedSelectExecutor */ public function createExecutor(Query $query): AbstractSqlExecutor { return new FinalizedSelectExecutor($this->finalizeSql($query)); diff --git a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php index a3fc7a8c367..6a1b7877b4d 100644 --- a/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -14,10 +14,10 @@ use Doctrine\ORM\Mapping\QuoteStrategy; use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\AST\DeleteStatement; use Doctrine\ORM\Query\AST\OrderByClause; use Doctrine\ORM\Query\AST\PathExpression; use Doctrine\ORM\Query\AST\SelectExpression; -use Doctrine\ORM\Query\AST\DeleteStatement; use Doctrine\ORM\Query\AST\SelectStatement; use Doctrine\ORM\Query\AST\UpdateStatement; use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; diff --git a/tests/Doctrine/Tests/Mocks/NullSqlWalker.php b/tests/Doctrine/Tests/Mocks/NullSqlWalker.php index 19e16089c74..90cf0c6e926 100644 --- a/tests/Doctrine/Tests/Mocks/NullSqlWalker.php +++ b/tests/Doctrine/Tests/Mocks/NullSqlWalker.php @@ -35,8 +35,7 @@ public function getFinalizer($AST): SqlFinalizer { return new PreparedExecutorFinalizer( new class extends AbstractSqlExecutor { - /** @return int */ - public function execute(Connection $conn, array $params, array $types) + public function execute(Connection $conn, array $params, array $types): int { return 0; } From db76e84fb7922646d33b1e60e23a6322d814fa1d Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 31 Jan 2024 10:16:47 +0100 Subject: [PATCH 25/41] Remove experimental code that I committed accidentally --- lib/Doctrine/ORM/Query.php | 3 -- .../ORM/Query/Exec/SingleSelectExecutor.php | 10 ------ lib/Doctrine/ORM/Query/Parser.php | 8 ----- lib/Doctrine/ORM/Query/ParserResult.php | 16 --------- lib/Doctrine/ORM/Query/SqlWalker.php | 34 ------------------- .../Tests/ORM/Query/CustomTreeWalkersTest.php | 4 +-- 6 files changed, 2 insertions(+), 73 deletions(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 89e0653d8bf..34c349de45f 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -291,7 +291,6 @@ private function parse(): ParserResult private function initializeSqlExecutor(): void { - // This will be the only code path in 3.0 if ($this->parserResult->hasSqlFinalizer()) { $this->sqlExecutor = $this->parserResult->getSqlFinalizer()->createExecutor($this); } else { @@ -831,8 +830,6 @@ protected function getQueryCacheId(): string { ksort($this->_hints); - // TODO: In 3.0, make the finalization phase mandatory for OutputWalkers, so we never need this check - if (! $this->hasHint(self::HINT_CUSTOM_OUTPUT_WALKER)) { // Assume Parser will create the SqlOutputWalker; save is_a call, which might trigger a class load $firstAndMaxResult = ''; diff --git a/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php b/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php index fd3f75884ac..fbcab590e09 100644 --- a/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php +++ b/lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php @@ -6,28 +6,18 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Result; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Query\AST\SelectStatement; use Doctrine\ORM\Query\SqlWalker; /** * Executor that executes the SQL statement for simple DQL SELECT statements. * - * @deprecated This class will be removed in 3.0 - * * @link www.doctrine-project.org */ class SingleSelectExecutor extends AbstractSqlExecutor { public function __construct(SelectStatement $AST, SqlWalker $sqlWalker) { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/orm/pull/xxx', - 'The %s class will be removed in Doctrine ORM 3.0', - self::class - ); - parent::__construct(); $this->sqlStatements = $sqlWalker->walkSelectStatement($AST); diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index df3f2a4cc22..edde449d2c4 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -391,7 +391,6 @@ public function parse() $this->queryComponents = $treeWalkerChain->getQueryComponents(); } - // TODO: In 3.0, add a runtime check that the class implements OutputWalker? $outputWalkerClass = $this->customOutputWalker ?: SqlOutputWalker::class; $outputWalker = new $outputWalkerClass($this->query, $this->parserResult, $this->queryComponents); @@ -399,13 +398,6 @@ public function parse() $finalizer = $outputWalker->getFinalizer($AST); $this->parserResult->setSqlFinalizer($finalizer); } else { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/orm/pull/xxx', - 'In Doctrine ORM 3.0, output walkers will need to implement the %s interface', - OutputWalker::class - ); - $executor = $outputWalker->getExecutor($AST); $this->parserResult->setSqlExecutor($executor); } diff --git a/lib/Doctrine/ORM/Query/ParserResult.php b/lib/Doctrine/ORM/Query/ParserResult.php index 59358c94ed3..75797317219 100644 --- a/lib/Doctrine/ORM/Query/ParserResult.php +++ b/lib/Doctrine/ORM/Query/ParserResult.php @@ -4,7 +4,6 @@ namespace Doctrine\ORM\Query; -use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; use Doctrine\ORM\Query\Exec\SqlFinalizer; use RuntimeException; @@ -92,13 +91,6 @@ public function setResultSetMapping(ResultSetMapping $rsm) */ public function setSqlExecutor($executor) { - Deprecation::triggerIfCalledFromOutside( - 'doctrine/orm', - 'https://github.com/doctrine/orm/pull/xxx', - 'In Doctrine ORM 3.0, the sqlExecutor will be removed from the ParserResult class.', - OutputWalker::class - ); - $this->sqlExecutor = $executor; } @@ -109,13 +101,6 @@ public function setSqlExecutor($executor) */ public function getSqlExecutor() { - Deprecation::triggerIfCalledFromOutside( - 'doctrine/orm', - 'https://github.com/doctrine/orm/pull/xxx', - 'In Doctrine ORM 3.0, the sqlExecutor will be removed from the ParserResult class.', - OutputWalker::class - ); - return $this->sqlExecutor; } @@ -135,7 +120,6 @@ public function getSqlFinalizer(): SqlFinalizer /** * @internal - * @deprecated will be removed in 3.0, SqlFinalizers will have to be present in every ParserResult * * @psalm-internal Doctrine\ORM\Query * @psalm-assert-if-true !null $this->sqlFinalizer diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 49307380aa7..d2c62c2f0da 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -275,8 +275,6 @@ public function setQueryComponent($dqlAlias, array $queryComponent) /** * Gets an executor that can be used to execute the result of this walker. * - * @deprecated This method will be replaced by the getFinalizer() method in 3.0 - * * @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST * * @return Exec\AbstractSqlExecutor @@ -593,38 +591,6 @@ private function assertOptimisticLockingHasAllClassesVersioned(): void } } -// public static function modifyLateQueryXX(string $sql, Query $query, AbstractPlatform $platform): string -// { -// $sqlWithLimit = self::modifyLimitQueryXX($sql, $query, $platform); -// $sqlWithLocking = self::modifyLockingQueryXX($sqlWithLimit, $query, $platform); -// -// return $sqlWithLocking; -// } -// -// private static function modifyLimitQueryXX(string $sql, Query $query, AbstractPlatform $platform): string -// { -// return $platform->modifyLimitQuery($sql, $query->getMaxResults(), $query->getFirstResult()); -// } -// -// private static function modifyLockingQueryXX(string $sql, Query $query, AbstractPlatform $platform): string -// { -// $lockMode = $query->getHint(Query::HINT_LOCK_MODE) ?: LockMode::NONE; -// -// if ($lockMode !== LockMode::NONE && $lockMode !== LockMode::OPTIMISTIC && $lockMode !== LockMode::PESSIMISTIC_READ && $lockMode !== LockMode::PESSIMISTIC_WRITE) { -// throw QueryException::invalidLockMode(); -// } -// -// if ($lockMode === LockMode::PESSIMISTIC_READ) { -// return $sql . ' ' . $platform->getReadLockSQL(); -// } -// -// if ($lockMode === LockMode::PESSIMISTIC_WRITE) { -// return $sql . ' ' . $platform->getWriteLockSQL(); -// } -// -// return $sql; -// } - /** * Walks down an UpdateStatement AST node, thereby generating the appropriate SQL. * diff --git a/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php b/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php index e9ad14d8c6d..b2267078ff5 100644 --- a/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php +++ b/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php @@ -96,7 +96,7 @@ public function testSetUnknownQueryComponentThrowsException(): void $this->generateSql( 'select u from Doctrine\Tests\Models\CMS\CmsUser u', [], - AddUnknownQueryComponentOutputWalker::class + AddUnknownQueryComponentWalker::class ); } @@ -110,7 +110,7 @@ public function testSupportsSeveralHintsQueries(): void } } -class AddUnknownQueryComponentOutputWalker extends Query\SqlOutputWalker +class AddUnknownQueryComponentWalker extends Query\SqlWalker { public function walkSelectStatement(Query\AST\SelectStatement $selectStatement): void { From 5ff75bf2033aef426fe0e5e7385cc903797354f7 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 31 Jan 2024 10:30:42 +0100 Subject: [PATCH 26/41] Remove unnecessary baseline entry --- psalm-baseline.xml | 3 --- 1 file changed, 3 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 878893cc7c3..ea11f9130d7 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1873,9 +1873,6 @@ - - getSqlStatements()]]> - FinalizedSelectExecutor FinalizedSelectExecutor From 59eed73fc4177304e06482485dfaa97066217a75 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 31 Jan 2024 15:31:37 +0100 Subject: [PATCH 27/41] Make AddUnknownQueryComponentWalker subclass SqlOutputWalker That way, we have no remaining classes in the codebase subclassing SqlWalker but not SqlOutputWalker --- tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php b/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php index b2267078ff5..eb27b6113a3 100644 --- a/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php +++ b/tests/Doctrine/Tests/ORM/Query/CustomTreeWalkersTest.php @@ -6,6 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\AST; use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Query\SqlWalker; use Doctrine\ORM\Query\TreeWalker; @@ -110,13 +111,13 @@ public function testSupportsSeveralHintsQueries(): void } } -class AddUnknownQueryComponentWalker extends Query\SqlWalker +class AddUnknownQueryComponentWalker extends Query\SqlOutputWalker { - public function walkSelectStatement(Query\AST\SelectStatement $selectStatement): void + protected function createSqlForFinalizer(AST\SelectStatement $AST): string { - parent::walkSelectStatement($selectStatement); - $this->setQueryComponent('x', []); + + return parent::createSqlForFinalizer($AST); } } From 259dfe76d5760eee29b858593cdf6cd89941439b Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Tue, 8 Oct 2024 17:26:11 +0200 Subject: [PATCH 28/41] Use more expressive exception classes --- src/Query/ParserResult.php | 4 ++-- src/Tools/Pagination/LimitSubqueryOutputWalker.php | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Query/ParserResult.php b/src/Query/ParserResult.php index 75797317219..773acef8b23 100644 --- a/src/Query/ParserResult.php +++ b/src/Query/ParserResult.php @@ -6,7 +6,7 @@ use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; use Doctrine\ORM\Query\Exec\SqlFinalizer; -use RuntimeException; +use LogicException; use function sprintf; @@ -112,7 +112,7 @@ public function setSqlFinalizer(SqlFinalizer $finalizer): void public function getSqlFinalizer(): SqlFinalizer { if ($this->sqlFinalizer === null) { - throw new RuntimeException('This ParserResult was not created with an SqlFinalizer'); + throw new LogicException('This ParserResult was not created with an SqlFinalizer'); } return $this->sqlFinalizer; diff --git a/src/Tools/Pagination/LimitSubqueryOutputWalker.php b/src/Tools/Pagination/LimitSubqueryOutputWalker.php index 6a1b7877b4d..0e858ce0ecb 100644 --- a/src/Tools/Pagination/LimitSubqueryOutputWalker.php +++ b/src/Tools/Pagination/LimitSubqueryOutputWalker.php @@ -27,6 +27,7 @@ use Doctrine\ORM\Query\QueryException; use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\ORM\Query\SqlOutputWalker; +use LogicException; use RuntimeException; use function array_diff; @@ -181,7 +182,7 @@ public function walkSelectStatement(SelectStatement $AST) public function getFinalizer($AST): SqlFinalizer { if (! $AST instanceof SelectStatement) { - throw new RuntimeException(self::class . ' is to be used on SelectStatements only'); + throw new LogicException(self::class . ' is to be used on SelectStatements only'); } if ($this->platformSupportsRowNumber()) { From 50e1f984e9e1866a34392102d2143910e0490477 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Tue, 8 Oct 2024 17:50:39 +0200 Subject: [PATCH 29/41] Add a deprecation message --- src/Query.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Query.php b/src/Query.php index 34c349de45f..fd85fba40d7 100644 --- a/src/Query.php +++ b/src/Query.php @@ -18,6 +18,7 @@ use Doctrine\ORM\Query\AST\SelectStatement; use Doctrine\ORM\Query\AST\UpdateStatement; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; +use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Query\OutputWalker; use Doctrine\ORM\Query\Parameter; use Doctrine\ORM\Query\ParameterTypeInferer; @@ -838,6 +839,14 @@ protected function getQueryCacheId(): string if (is_a($outputWalkerClass, OutputWalker::class, true)) { $firstAndMaxResult = ''; } else { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/11188/', + 'Your output walker class %s should implement %s and provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be added in the finalization phase only.', + $outputWalkerClass, + OutputWalker::class, + SqlFinalizer::class + ); $firstAndMaxResult = '&firstResult=' . $this->firstResult . '&maxResult=' . $this->maxResults; } } From 9c93af9827fedcf5acd67c23e9373cf6de3cb80f Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 9 Oct 2024 19:07:45 +0200 Subject: [PATCH 30/41] Move SqlExecutor creation to ParserResult, to minimize public methods available on it --- src/Query.php | 8 ++----- src/Query/Parser.php | 9 +++++++ src/Query/ParserResult.php | 24 ++++++++----------- .../ParserResultSerializationTest.php | 6 ++--- .../Tools/Pagination/WhereInWalkerTest.php | 2 +- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/Query.php b/src/Query.php index fd85fba40d7..7effc46aa87 100644 --- a/src/Query.php +++ b/src/Query.php @@ -292,11 +292,7 @@ private function parse(): ParserResult private function initializeSqlExecutor(): void { - if ($this->parserResult->hasSqlFinalizer()) { - $this->sqlExecutor = $this->parserResult->getSqlFinalizer()->createExecutor($this); - } else { - $this->sqlExecutor = $this->parserResult->getSqlExecutor(); - } + $this->sqlExecutor = $this->parserResult->prepareSqlExecutor($this); } /** @@ -842,7 +838,7 @@ protected function getQueryCacheId(): string Deprecation::trigger( 'doctrine/orm', 'https://github.com/doctrine/orm/pull/11188/', - 'Your output walker class %s should implement %s and provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be added in the finalization phase only.', + 'Your output walker class %s should implement %s in order to provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be read from the query by the SqlFinalizer only.', $outputWalkerClass, OutputWalker::class, SqlFinalizer::class diff --git a/src/Query/Parser.php b/src/Query/Parser.php index ab4ffdc32ab..6020c5aa861 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -10,6 +10,7 @@ use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Query; use Doctrine\ORM\Query\AST\Functions; +use Doctrine\ORM\Query\Exec\SqlFinalizer; use LogicException; use ReflectionClass; @@ -398,6 +399,14 @@ public function parse() $finalizer = $outputWalker->getFinalizer($AST); $this->parserResult->setSqlFinalizer($finalizer); } else { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/11188/', + 'Your output walker class %s should implement %s in order to provide a %s. This also means the output walker should not use the query firstResult/maxResult values, which should be read from the query by the SqlFinalizer only.', + $outputWalkerClass, + OutputWalker::class, + SqlFinalizer::class + ); $executor = $outputWalker->getExecutor($AST); $this->parserResult->setSqlExecutor($executor); } diff --git a/src/Query/ParserResult.php b/src/Query/ParserResult.php index 773acef8b23..ec8791bf1b0 100644 --- a/src/Query/ParserResult.php +++ b/src/Query/ParserResult.php @@ -4,6 +4,7 @@ namespace Doctrine\ORM\Query; +use Doctrine\ORM\Query; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; use Doctrine\ORM\Query\Exec\SqlFinalizer; use LogicException; @@ -88,6 +89,7 @@ public function setResultSetMapping(ResultSetMapping $rsm) * @param AbstractSqlExecutor $executor * * @return void + * @deprecated */ public function setSqlExecutor($executor) { @@ -97,6 +99,7 @@ public function setSqlExecutor($executor) /** * Gets the SQL executor used by this ParserResult. * + * @deprecated * @return ?AbstractSqlExecutor */ public function getSqlExecutor() @@ -109,24 +112,17 @@ public function setSqlFinalizer(SqlFinalizer $finalizer): void $this->sqlFinalizer = $finalizer; } - public function getSqlFinalizer(): SqlFinalizer + public function prepareSqlExecutor(Query $query): AbstractSqlExecutor { - if ($this->sqlFinalizer === null) { - throw new LogicException('This ParserResult was not created with an SqlFinalizer'); + if (null !== $this->sqlFinalizer) { + return $this->sqlFinalizer->createExecutor($query); } - return $this->sqlFinalizer; - } + if (null !== $this->sqlExecutor) { + return $this->sqlExecutor; + } - /** - * @internal - * - * @psalm-internal Doctrine\ORM\Query - * @psalm-assert-if-true !null $this->sqlFinalizer - */ - public function hasSqlFinalizer(): bool - { - return $this->sqlFinalizer !== null; + throw new LogicException('This ParserResult lacks both the SqlFinalizer as well as the (legacy) SqlExecutor'); } /** diff --git a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php index 5ded9aeeae2..8cb109cf216 100644 --- a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -49,8 +49,7 @@ public function testSerializeParserResultForQueryWithSqlWalker(Closure $toSerial $this->assertInstanceOf(ParserResult::class, $unserialized); $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); - $this->assertFalse($unserialized->hasSqlFinalizer()); - $this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor()); + $this->assertNotNull($unserialized->prepareSqlExecutor($query)); } /** @@ -69,8 +68,7 @@ public function testSerializeParserResultForQueryWithSqlOutputWalker(Closure $to $this->assertInstanceOf(ParserResult::class, $unserialized); $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); - $this->assertTrue($unserialized->hasSqlFinalizer()); - $this->assertNotNull($unserialized->getSqlFinalizer()); + $this->assertNotNull($unserialized->prepareSqlExecutor($query)); } /** @return Generator */ diff --git a/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php b/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php index 4d27d1792fd..6f5c9c6dd22 100644 --- a/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php +++ b/tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php @@ -22,7 +22,7 @@ public function testDqlQueryTransformation(string $dql, string $expectedSql): vo $query->setHint(WhereInWalker::HINT_PAGINATOR_HAS_IDS, true); $result = (new Parser($query))->parse(); - $executor = $result->getSqlFinalizer()->createExecutor($query); + $executor = $result->prepareSqlExecutor($query); self::assertEquals($expectedSql, $executor->getSqlStatements()); self::assertEquals([0], $result->getSqlParameterPositions(WhereInWalker::PAGINATOR_ID_ALIAS)); From 8beab61070d91e96fc30bd87c1ba668b32a7fbca Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 10:04:34 +0200 Subject: [PATCH 31/41] Avoid keeping the SqlExecutor in the Query, since it must be generated just in time (e. g. in case Query parameters change) --- src/Query.php | 16 +--------------- src/Query/ParserResult.php | 8 +++++--- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/Query.php b/src/Query.php index 7effc46aa87..d98c99984b5 100644 --- a/src/Query.php +++ b/src/Query.php @@ -150,9 +150,6 @@ class Query extends AbstractQuery */ private $parserResult; - /** @var ?AbstractSqlExecutor */ - private $sqlExecutor; - /** * The first result to return (the "offset"). * @@ -260,7 +257,6 @@ private function parse(): ParserResult $parser = new Parser($this); $this->parserResult = $parser->parse(); - $this->initializeSqlExecutor(); return $this->parserResult; } @@ -272,7 +268,6 @@ private function parse(): ParserResult if ($cached instanceof ParserResult) { // Cache hit. $this->parserResult = $cached; - $this->initializeSqlExecutor(); return $this->parserResult; } @@ -285,16 +280,9 @@ private function parse(): ParserResult $queryCache->save($cacheItem->set($this->parserResult)->expiresAfter($this->queryCacheTTL)); - $this->initializeSqlExecutor(); - return $this->parserResult; } - private function initializeSqlExecutor(): void - { - $this->sqlExecutor = $this->parserResult->prepareSqlExecutor($this); - } - /** * {@inheritDoc} */ @@ -873,8 +861,6 @@ public function __clone() private function getSqlExecutor(): AbstractSqlExecutor { - $this->parse(); - - return $this->sqlExecutor; + return $this->parse()->prepareSqlExecutor($this); } } diff --git a/src/Query/ParserResult.php b/src/Query/ParserResult.php index ec8791bf1b0..86a4e4d3b64 100644 --- a/src/Query/ParserResult.php +++ b/src/Query/ParserResult.php @@ -86,10 +86,11 @@ public function setResultSetMapping(ResultSetMapping $rsm) /** * Sets the SQL executor that should be used for this ParserResult. * + * @deprecated + * * @param AbstractSqlExecutor $executor * * @return void - * @deprecated */ public function setSqlExecutor($executor) { @@ -100,6 +101,7 @@ public function setSqlExecutor($executor) * Gets the SQL executor used by this ParserResult. * * @deprecated + * * @return ?AbstractSqlExecutor */ public function getSqlExecutor() @@ -114,11 +116,11 @@ public function setSqlFinalizer(SqlFinalizer $finalizer): void public function prepareSqlExecutor(Query $query): AbstractSqlExecutor { - if (null !== $this->sqlFinalizer) { + if ($this->sqlFinalizer !== null) { return $this->sqlFinalizer->createExecutor($query); } - if (null !== $this->sqlExecutor) { + if ($this->sqlExecutor !== null) { return $this->sqlExecutor; } From b2f14c612397c087beff38a041d4ba40d2af35d8 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 10:36:06 +0200 Subject: [PATCH 32/41] Address PHPStan complaints --- src/Query/Parser.php | 1 + src/Query/SqlWalker.php | 2 ++ .../ORM/Functional/ParserResultSerializationTest.php | 11 +++++++---- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 6020c5aa861..3005be0560a 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -408,6 +408,7 @@ public function parse() SqlFinalizer::class ); $executor = $outputWalker->getExecutor($AST); + // @phpstan-ignore method.deprecated $this->parserResult->setSqlExecutor($executor); } diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 30854ad4150..5fe53965185 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -278,6 +278,8 @@ public function setQueryComponent($dqlAlias, array $queryComponent) * @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST * * @return Exec\AbstractSqlExecutor + * + * @not-deprecated */ public function getExecutor($AST) { diff --git a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php index 8cb109cf216..9ce011605b5 100644 --- a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -6,7 +6,9 @@ use Closure; use Doctrine\ORM\Query; -use Doctrine\ORM\Query\Exec\SingleSelectExecutor; +use Doctrine\ORM\Query\Exec\FinalizedSelectExecutor; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\Tests\OrmFunctionalTestCase; @@ -143,11 +145,12 @@ public static function provideSerializedSingleSelectResults(): Generator public function testSymfony44ProvidedData(): void { - $sqlExecutor = $this->createMock(SingleSelectExecutor::class); + $sqlExecutor = new FinalizedSelectExecutor('test'); + $sqlFinalizeer = new PreparedExecutorFinalizer($sqlExecutor); $resultSetMapping = $this->createMock(ResultSetMapping::class); $parserResult = new ParserResult(); - $parserResult->setSqlExecutor($sqlExecutor); + $parserResult->setSqlFinalizer($sqlFinalizeer); $parserResult->setResultSetMapping($resultSetMapping); $parserResult->addParameterMapping('name', 0); @@ -157,7 +160,7 @@ public function testSymfony44ProvidedData(): void $this->assertInstanceOf(ParserResult::class, $unserialized); $this->assertInstanceOf(ResultSetMapping::class, $unserialized->getResultSetMapping()); $this->assertEquals(['name' => [0]], $unserialized->getParameterMappings()); - $this->assertInstanceOf(SingleSelectExecutor::class, $unserialized->getSqlExecutor()); + $this->assertEquals($sqlExecutor, $unserialized->prepareSqlExecutor($this->createMock(Query::class))); } private static function parseQuery(Query $query): ParserResult From f4bda83b2724d69aadb074b50b9d3b0d4da0dd88 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 10:45:41 +0200 Subject: [PATCH 33/41] Fix tests --- .../Tests/ORM/Functional/ParserResultSerializationTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php index 9ce011605b5..7b2ae59119a 100644 --- a/tests/Tests/ORM/Functional/ParserResultSerializationTest.php +++ b/tests/Tests/ORM/Functional/ParserResultSerializationTest.php @@ -8,7 +8,7 @@ use Doctrine\ORM\Query; use Doctrine\ORM\Query\Exec\FinalizedSelectExecutor; use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; -use Doctrine\ORM\Query\Exec\SqlFinalizer; +use Doctrine\ORM\Query\Exec\SingleSelectExecutor; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\Tests\OrmFunctionalTestCase; @@ -146,11 +146,11 @@ public static function provideSerializedSingleSelectResults(): Generator public function testSymfony44ProvidedData(): void { $sqlExecutor = new FinalizedSelectExecutor('test'); - $sqlFinalizeer = new PreparedExecutorFinalizer($sqlExecutor); + $sqlFinalizer = new PreparedExecutorFinalizer($sqlExecutor); $resultSetMapping = $this->createMock(ResultSetMapping::class); $parserResult = new ParserResult(); - $parserResult->setSqlFinalizer($sqlFinalizeer); + $parserResult->setSqlFinalizer($sqlFinalizer); $parserResult->setResultSetMapping($resultSetMapping); $parserResult->addParameterMapping('name', 0); From b674182a90879de38d25750303a17b2694564bd4 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 12:07:28 +0200 Subject: [PATCH 34/41] Small refactorings --- src/Query/OutputWalker.php | 8 +++++- src/Query/Parser.php | 1 + src/Query/SqlOutputWalker.php | 12 -------- src/Query/SqlWalker.php | 53 +++++++++++++++++++++++++++-------- 4 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/Query/OutputWalker.php b/src/Query/OutputWalker.php index ef4704f1188..c4b6372b35f 100644 --- a/src/Query/OutputWalker.php +++ b/src/Query/OutputWalker.php @@ -14,8 +14,14 @@ * * The goal of an OutputWalker is to ultimately provide the SqlFinalizer * which produces the final, executable SQL statement in a "finalization" phase. + * + * It must be possible to use the same SqlFinalizer for Queries with different + * firstResult/maxResult values. In other words, SQL produced by the + * output walker should not depend on those values, and any SQL generation/modification + * specific to them should happen in the finalizer's `\Doctrine\ORM\Query\Exec\SqlFinalizer::createExecutor()` + * method instead. */ -interface OutputWalker extends TreeWalker +interface OutputWalker { /** @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST */ public function getFinalizer($AST): SqlFinalizer; diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 3005be0560a..6e0e214899a 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -407,6 +407,7 @@ public function parse() OutputWalker::class, SqlFinalizer::class ); + // @phpstan-ignore method.deprecated $executor = $outputWalker->getExecutor($AST); // @phpstan-ignore method.deprecated $this->parserResult->setSqlExecutor($executor); diff --git a/src/Query/SqlOutputWalker.php b/src/Query/SqlOutputWalker.php index bef3adc5441..61e0d8a3a4c 100644 --- a/src/Query/SqlOutputWalker.php +++ b/src/Query/SqlOutputWalker.php @@ -4,18 +4,6 @@ namespace Doctrine\ORM\Query; -use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; -use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; -use Doctrine\ORM\Query\Exec\SqlFinalizer; - class SqlOutputWalker extends SqlWalker implements OutputWalker { - public function getFinalizer($AST): SqlFinalizer - { - if (! $AST instanceof AST\SelectStatement) { - return new PreparedExecutorFinalizer(parent::getExecutor($AST)); - } - - return new SingleSelectSqlFinalizer($this->createSqlForFinalizer($AST)); - } } diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 5fe53965185..7f32302a62e 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -15,6 +15,9 @@ use Doctrine\ORM\Mapping\QuoteStrategy; use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver; use Doctrine\ORM\Utility\PersisterHelper; use InvalidArgumentException; @@ -279,30 +282,58 @@ public function setQueryComponent($dqlAlias, array $queryComponent) * * @return Exec\AbstractSqlExecutor * - * @not-deprecated + * @deprecated Output walkers should no longer create the executor directly, but instead provide + * a SqlFinalizer by implementing the `OutputWalker` interface. Thus, this method is + * no longer needed and will be removed in 4.0. */ public function getExecutor($AST) { switch (true) { case $AST instanceof AST\DeleteStatement: - $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); - - return $primaryClass->isInheritanceTypeJoined() - ? new Exec\MultiTableDeleteExecutor($AST, $this) - : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + return $this->createDeleteStatementExecutor($AST); case $AST instanceof AST\UpdateStatement: - $primaryClass = $this->em->getClassMetadata($AST->updateClause->abstractSchemaName); - - return $primaryClass->isInheritanceTypeJoined() - ? new Exec\MultiTableUpdateExecutor($AST, $this) - : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + return $this->createUpdateStatementExecutor($AST); default: return new Exec\SingleSelectExecutor($AST, $this); } } + public function getFinalizer($AST): SqlFinalizer + { + switch (true) { + case $AST instanceof AST\SelectStatement: + return new SingleSelectSqlFinalizer($this->createSqlForFinalizer($AST)); + + case $AST instanceof AST\UpdateStatement: + return new PreparedExecutorFinalizer($this->createUpdateStatementExecutor($AST)); + + case $AST instanceof AST\DeleteStatement: + return new PreparedExecutorFinalizer($this->createDeleteStatementExecutor($AST)); + } + + throw new LogicException('Unexpected AST node type'); + } + + private function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec\AbstractSqlExecutor + { + $primaryClass = $this->em->getClassMetadata($AST->updateClause->abstractSchemaName); + + return $primaryClass->isInheritanceTypeJoined() + ? new Exec\MultiTableUpdateExecutor($AST, $this) + : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + } + + private function createDeleteStatementExecutor(AST\DeleteStatement $AST): Exec\AbstractSqlExecutor + { + $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); + + return $primaryClass->isInheritanceTypeJoined() + ? new Exec\MultiTableDeleteExecutor($AST, $this) + : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); + } + /** * Generates a unique, short SQL table alias. * From 860bce83a31ba8bb2d5793f44686264906c3d9ea Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 13:50:35 +0200 Subject: [PATCH 35/41] Add an upgrade notice --- UPGRADE.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index e20a8fef89b..e11e12b84fd 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,14 @@ # Upgrade to 2.20 +## Add `Doctrine\ORM\Query\OutputWalker` interface, deprecate `Doctrine\ORM\Query\SqlWalker::getExecutor()` + +Output walkers should implement the new `\Doctrine\ORM\Query\OutputWalker` interface and create +`Doctrine\ORM\Query\Exec\SqlFinalizer` instances instead of `Doctrine\ORM\Query\Exec\AbstractSqlExecutor`s. +The output walker must not base its workings on the query `firstResult`/`maxResult` values, so that the +`SqlFinalizer` can be kept in the query cache and used regardless of the actual `firstResult`/`maxResult` values. +Any operation dependent on `firstResult`/`maxResult` should take place within the `SqlFinalizer::createExecutor()` +method. Details can be found at https://github.com/doctrine/orm/pull/11188. + ## PARTIAL DQL syntax is undeprecated for non-object hydration Use of the PARTIAL keyword is not deprecated anymore in DQL when used with a hydrator From 60924296a522379235b603efa206983a03213e6d Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 13:51:07 +0200 Subject: [PATCH 36/41] Small refactorings --- src/Query/Parser.php | 2 ++ src/Query/SqlOutputWalker.php | 19 +++++++++++++++++++ src/Query/SqlWalker.php | 26 ++++++++------------------ 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/Query/Parser.php b/src/Query/Parser.php index 6e0e214899a..fe1595b5296 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -408,8 +408,10 @@ public function parse() SqlFinalizer::class ); // @phpstan-ignore method.deprecated + /** @psalm-suppress DeprecatedMethod */ $executor = $outputWalker->getExecutor($AST); // @phpstan-ignore method.deprecated + /** @psalm-suppress DeprecatedMethod */ $this->parserResult->setSqlExecutor($executor); } diff --git a/src/Query/SqlOutputWalker.php b/src/Query/SqlOutputWalker.php index 61e0d8a3a4c..82aad14a5de 100644 --- a/src/Query/SqlOutputWalker.php +++ b/src/Query/SqlOutputWalker.php @@ -4,6 +4,25 @@ namespace Doctrine\ORM\Query; +use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; +use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; +use Doctrine\ORM\Query\Exec\SqlFinalizer; + class SqlOutputWalker extends SqlWalker implements OutputWalker { + public function getFinalizer($AST): SqlFinalizer + { + switch (true) { + case $AST instanceof AST\SelectStatement: + return new SingleSelectSqlFinalizer($this->createSqlForFinalizer($AST)); + + case $AST instanceof AST\UpdateStatement: + return new PreparedExecutorFinalizer($this->createUpdateStatementExecutor($AST)); + + case $AST instanceof AST\DeleteStatement: + return new PreparedExecutorFinalizer($this->createDeleteStatementExecutor($AST)); + } + + throw new LogicException('Unexpected AST node type'); + } } diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 7f32302a62e..0ae2329733c 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -300,23 +300,10 @@ public function getExecutor($AST) } } - public function getFinalizer($AST): SqlFinalizer - { - switch (true) { - case $AST instanceof AST\SelectStatement: - return new SingleSelectSqlFinalizer($this->createSqlForFinalizer($AST)); - - case $AST instanceof AST\UpdateStatement: - return new PreparedExecutorFinalizer($this->createUpdateStatementExecutor($AST)); - - case $AST instanceof AST\DeleteStatement: - return new PreparedExecutorFinalizer($this->createDeleteStatementExecutor($AST)); - } - - throw new LogicException('Unexpected AST node type'); - } - - private function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec\AbstractSqlExecutor + /** + * @psalm-internal Doctrine\ORM + */ + protected function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec\AbstractSqlExecutor { $primaryClass = $this->em->getClassMetadata($AST->updateClause->abstractSchemaName); @@ -325,7 +312,10 @@ private function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec\A : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); } - private function createDeleteStatementExecutor(AST\DeleteStatement $AST): Exec\AbstractSqlExecutor + /** + * @psalm-internal Doctrine\ORM + */ + protected function createDeleteStatementExecutor(AST\DeleteStatement $AST): Exec\AbstractSqlExecutor { $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); From f04927ba3a03fe97c3db5299f1d377f192fbd429 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 13:53:55 +0200 Subject: [PATCH 37/41] Update the Psalm baseline --- psalm-baseline.xml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index b9473bebb66..9703239dd6b 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1865,6 +1865,12 @@ _sqlStatements = &$this->sqlStatements]]> + + + + + + @@ -2054,11 +2060,6 @@ - - - - - parameters)]]> From 7f576e5c01fa2566063b8fda11d5b00af917512c Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 14:01:48 +0200 Subject: [PATCH 38/41] Add a missing namespace import --- src/Query/SqlOutputWalker.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Query/SqlOutputWalker.php b/src/Query/SqlOutputWalker.php index 82aad14a5de..e737e1c9c53 100644 --- a/src/Query/SqlOutputWalker.php +++ b/src/Query/SqlOutputWalker.php @@ -7,6 +7,7 @@ use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; use Doctrine\ORM\Query\Exec\SqlFinalizer; +use LogicException; class SqlOutputWalker extends SqlWalker implements OutputWalker { From 89a46d07af8cded5ed95a3e0c86eb9a8b3becefb Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 14:25:00 +0200 Subject: [PATCH 39/41] Update Psalm baseline --- psalm-baseline.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8fb1e381f15..61f00a9d9a6 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1099,6 +1099,11 @@ + + + + + ]]> @@ -2002,6 +2007,9 @@ + + + @@ -2706,6 +2714,9 @@ + + + From e180e2b026fe2e4d6129d0c296f93498e402177d Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 14:28:31 +0200 Subject: [PATCH 40/41] Fix CS --- src/Query/Parser.php | 2 -- src/Query/SqlWalker.php | 19 ++++++------------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/Query/Parser.php b/src/Query/Parser.php index b3781d973b1..272a91ad2c1 100644 --- a/src/Query/Parser.php +++ b/src/Query/Parser.php @@ -413,10 +413,8 @@ public function parse() SqlFinalizer::class ); // @phpstan-ignore method.deprecated - /** @psalm-suppress DeprecatedMethod */ $executor = $outputWalker->getExecutor($AST); // @phpstan-ignore method.deprecated - /** @psalm-suppress DeprecatedMethod */ $this->parserResult->setSqlExecutor($executor); } diff --git a/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index 0ae2329733c..e4e2de5c5c1 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -15,9 +15,6 @@ use Doctrine\ORM\Mapping\QuoteStrategy; use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\Query; -use Doctrine\ORM\Query\Exec\PreparedExecutorFinalizer; -use Doctrine\ORM\Query\Exec\SingleSelectSqlFinalizer; -use Doctrine\ORM\Query\Exec\SqlFinalizer; use Doctrine\ORM\Utility\HierarchyDiscriminatorResolver; use Doctrine\ORM\Utility\PersisterHelper; use InvalidArgumentException; @@ -278,13 +275,13 @@ public function setQueryComponent($dqlAlias, array $queryComponent) /** * Gets an executor that can be used to execute the result of this walker. * - * @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST - * - * @return Exec\AbstractSqlExecutor - * * @deprecated Output walkers should no longer create the executor directly, but instead provide * a SqlFinalizer by implementing the `OutputWalker` interface. Thus, this method is * no longer needed and will be removed in 4.0. + * + * @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST + * + * @return Exec\AbstractSqlExecutor */ public function getExecutor($AST) { @@ -300,9 +297,7 @@ public function getExecutor($AST) } } - /** - * @psalm-internal Doctrine\ORM - */ + /** @psalm-internal Doctrine\ORM */ protected function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec\AbstractSqlExecutor { $primaryClass = $this->em->getClassMetadata($AST->updateClause->abstractSchemaName); @@ -312,9 +307,7 @@ protected function createUpdateStatementExecutor(AST\UpdateStatement $AST): Exec : new Exec\SingleTableDeleteUpdateExecutor($AST, $this); } - /** - * @psalm-internal Doctrine\ORM - */ + /** @psalm-internal Doctrine\ORM */ protected function createDeleteStatementExecutor(AST\DeleteStatement $AST): Exec\AbstractSqlExecutor { $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); From 534640a1c09cd573dcf928822cf3e1521edabea8 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 10 Oct 2024 14:35:27 +0200 Subject: [PATCH 41/41] Fix Psalm baseline --- psalm-baseline.xml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 61f00a9d9a6..28ed8aab049 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1099,11 +1099,6 @@ - - - - - ]]> @@ -2714,9 +2709,6 @@ - - -