diff --git a/UPGRADE.md b/UPGRADE.md index df40b151d24..045c9ed0c09 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,18 @@ +# Upgrade to xxx + +## Changes for custom output walkers + +To implement a custom output walker, extend the `SqlOutputWalker` class. Either override +the `getFinalizer()` method to return an instance of `SqlFinalizer`, or override +`createSqlForFinalizer()` and return your SQL string from it. + +When extending `SqlOutputWalker`, the query cache speeding up the DQL->SQL transformation +will no longer take the `Query::setFirstResult()` and `Query::setMaxResults()` values +into consideration, so your output walker must not use or depend on these values. If +it does, move this part to a custom `SqlFinalizer` class. + +https://github.com/doctrine/orm/pull/11188 gives more background. + # Upgrade to 2.17 ## Deprecate annotations classes for named queries diff --git a/psalm-baseline.xml b/psalm-baseline.xml index fe0d73f1a03..34b44307ff3 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1445,12 +1445,6 @@ $sqlParams - - AbstractSqlExecutor - - - sqlExecutor]]> - getDQL()]]> @@ -1866,12 +1860,6 @@ _sqlStatements = &$this->sqlStatements]]> - - - FinalizedSelectExecutor - FinalizedSelectExecutor - - $numDeleted @@ -2061,6 +2049,11 @@ $token === Lexer::T_IDENTIFIER + + + $sqlExecutor + + parameters)]]> diff --git a/src/Query.php b/src/Query.php index 34c349de45f..bdebb894614 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; @@ -294,6 +295,15 @@ private function initializeSqlExecutor(): void if ($this->parserResult->hasSqlFinalizer()) { $this->sqlExecutor = $this->parserResult->getSqlFinalizer()->createExecutor($this); } else { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'In Doctrine ORM 3.0, instances of %s must provide an instance of %s. Check the %s output walker that created this parser result.', + ParserResult::class, + SqlFinalizer::class, + $this->getHint(self::HINT_CUSTOM_OUTPUT_WALKER) + ); + $this->sqlExecutor = $this->parserResult->getSqlExecutor(); } } @@ -830,6 +840,9 @@ protected function getQueryCacheId(): string { ksort($this->_hints); + // TODO: Once \Doctrine\ORM\Query\Parser::parse accepts only instances of OutputWalker, + // this branching can be removed and we never need to consider first/max results. + 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/src/Query/Exec/SingleSelectExecutor.php b/src/Query/Exec/SingleSelectExecutor.php index fbcab590e09..fd3f75884ac 100644 --- a/src/Query/Exec/SingleSelectExecutor.php +++ b/src/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/src/Query/Parser.php b/src/Query/Parser.php index edde449d2c4..1347226e5dd 100644 --- a/src/Query/Parser.php +++ b/src/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,14 @@ public function parse() $finalizer = $outputWalker->getFinalizer($AST); $this->parserResult->setSqlFinalizer($finalizer); } else { + // TODO once this path is removed, update \Doctrine\ORM\Query::getQueryCacheId as well + 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/src/Query/ParserResult.php b/src/Query/ParserResult.php index 75797317219..3af5d7f456e 100644 --- a/src/Query/ParserResult.php +++ b/src/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,15 @@ 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 %s class will no longer contain an sqlExecutor. The %s::%s method will be removed.', + self::class, + self::class, + __METHOD__ + ); + $this->sqlExecutor = $executor; } @@ -101,6 +111,15 @@ public function setSqlExecutor($executor) */ public function getSqlExecutor() { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'In Doctrine ORM 3.0, the %s class will no longer contain an sqlExecutor. The %s::%s method will be removed.', + self::class, + self::class, + __METHOD__ + ); + return $this->sqlExecutor; } @@ -120,6 +139,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/src/Query/SqlWalker.php b/src/Query/SqlWalker.php index d2c62c2f0da..4cd09f85b3d 100644 --- a/src/Query/SqlWalker.php +++ b/src/Query/SqlWalker.php @@ -179,6 +179,15 @@ class SqlWalker implements TreeWalker */ public function __construct($query, $parserResult, array $queryComponents) { + if (! $this instanceof SqlOutputWalker) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'The %s class may be removed in Doctrine ORM 3.0. To implement an output walker, inherit from %s and override the getFinalizer() or createSqlForFinalizer() methods instead.', + self::class, + SqlOutputWalker::class + ); + } $this->query = $query; $this->parserResult = $parserResult; $this->queryComponents = $queryComponents; @@ -275,12 +284,23 @@ 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 */ public function getExecutor($AST) { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/xxx', + 'In Doctrine ORM 3.0, output walkers need to implement the %s interface and no longer provide a getExecutor() method. Thus, %s::%s will be removed.', + OutputWalker::class, + self::class, + __METHOD__ + ); + switch (true) { case $AST instanceof AST\DeleteStatement: $primaryClass = $this->em->getClassMetadata($AST->deleteClause->abstractSchemaName); diff --git a/tests/Tests/ORM/Functional/PaginationTest.php b/tests/Tests/ORM/Functional/PaginationTest.php index c4fa3296d0b..c2fe447536a 100644 --- a/tests/Tests/ORM/Functional/PaginationTest.php +++ b/tests/Tests/ORM/Functional/PaginationTest.php @@ -635,8 +635,6 @@ public function testCountQueryStripsParametersInSelect(): void self::assertCount(2, $getCountQuery->invoke($paginator)->getParameters()); self::assertCount(9, $paginator); - $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, Query\SqlWalker::class); - $paginator = new Paginator($query); // if select part of query is replaced with count(...) paginator should remove diff --git a/tests/Tests/ORM/Query/CustomTreeWalkersTest.php b/tests/Tests/ORM/Query/CustomTreeWalkersTest.php index eb27b6113a3..c7c3b41188c 100644 --- a/tests/Tests/ORM/Query/CustomTreeWalkersTest.php +++ b/tests/Tests/ORM/Query/CustomTreeWalkersTest.php @@ -97,7 +97,7 @@ public function testSetUnknownQueryComponentThrowsException(): void $this->generateSql( 'select u from Doctrine\Tests\Models\CMS\CmsUser u', [], - AddUnknownQueryComponentWalker::class + AddUnknownQueryComponentOutputWalker::class ); } @@ -111,7 +111,7 @@ public function testSupportsSeveralHintsQueries(): void } } -class AddUnknownQueryComponentWalker extends Query\SqlOutputWalker +class AddUnknownQueryComponentOutputWalker extends Query\SqlOutputWalker { protected function createSqlForFinalizer(AST\SelectStatement $AST): string { diff --git a/tests/Tests/ORM/Query/ParserResultTest.php b/tests/Tests/ORM/Query/ParserResultTest.php index f1bf6ea8e71..b6e670fbcf6 100644 --- a/tests/Tests/ORM/Query/ParserResultTest.php +++ b/tests/Tests/ORM/Query/ParserResultTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM\Query; +use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\ORM\Query\Exec\AbstractSqlExecutor; use Doctrine\ORM\Query\ParserResult; use Doctrine\ORM\Query\ResultSetMapping; @@ -11,6 +12,8 @@ class ParserResultTest extends TestCase { + use VerifyDeprecations; + /** @var ParserResult */ public $parserResult; @@ -26,6 +29,7 @@ public function testGetRsm(): void public function testSetGetSqlExecutor(): void { + self::expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/xxx'); self::assertNull($this->parserResult->getSqlExecutor()); $executor = $this->getMockForAbstractClass(AbstractSqlExecutor::class); diff --git a/tests/Tests/ORM/Query/SqlWalkerTest.php b/tests/Tests/ORM/Query/SqlOutputWalkerTest.php similarity index 76% rename from tests/Tests/ORM/Query/SqlWalkerTest.php rename to tests/Tests/ORM/Query/SqlOutputWalkerTest.php index c760bf23b8b..a8b5ec5484b 100644 --- a/tests/Tests/ORM/Query/SqlWalkerTest.php +++ b/tests/Tests/ORM/Query/SqlOutputWalkerTest.php @@ -6,22 +6,23 @@ use Doctrine\ORM\Query; use Doctrine\ORM\Query\ParserResult; -use Doctrine\ORM\Query\SqlWalker; +use Doctrine\ORM\Query\SqlOutputWalker; use Doctrine\Tests\OrmTestCase; /** - * Tests for {@see \Doctrine\ORM\Query\SqlWalker} + * Tests for {@see \Doctrine\ORM\Query\SqlOutputWalker} * + * @covers \Doctrine\ORM\Query\SqlOutputWalker * @covers \Doctrine\ORM\Query\SqlWalker */ -class SqlWalkerTest extends OrmTestCase +class SqlOutputWalkerTest extends OrmTestCase { - /** @var SqlWalker */ + /** @var SqlOutputWalker */ private $sqlWalker; protected function setUp(): void { - $this->sqlWalker = new SqlWalker(new Query($this->getTestEntityManager()), new ParserResult(), []); + $this->sqlWalker = new SqlOutputWalker(new Query($this->getTestEntityManager()), new ParserResult(), []); } /** @dataProvider getColumnNamesAndSqlAliases */