Skip to content

Commit

Permalink
Move SqlExecutor creation to ParserResult, to minimize public methods…
Browse files Browse the repository at this point in the history
… available on it
  • Loading branch information
mpdude committed Oct 9, 2024
1 parent 50e1f98 commit 9c93af9
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 25 deletions.
8 changes: 2 additions & 6 deletions src/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/Query/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
Expand Down
24 changes: 10 additions & 14 deletions src/Query/ParserResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -88,6 +89,7 @@ public function setResultSetMapping(ResultSetMapping $rsm)
* @param AbstractSqlExecutor $executor
*
* @return void

Check failure on line 91 in src/Query/ParserResult.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.3)

Incorrect annotations group.
* @deprecated
*/
public function setSqlExecutor($executor)
{
Expand All @@ -97,6 +99,7 @@ public function setSqlExecutor($executor)
/**
* Gets the SQL executor used by this ParserResult.
*
* @deprecated

Check failure on line 102 in src/Query/ParserResult.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.3)

Incorrect annotations group.
* @return ?AbstractSqlExecutor
*/
public function getSqlExecutor()
Expand All @@ -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) {

Check failure on line 117 in src/Query/ParserResult.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.3)

Yoda comparisons are disallowed.
return $this->sqlFinalizer->createExecutor($query);
}

return $this->sqlFinalizer;
}
if (null !== $this->sqlExecutor) {

Check failure on line 121 in src/Query/ParserResult.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.3)

Yoda comparisons are disallowed.
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');

Check warning on line 125 in src/Query/ParserResult.php

View check run for this annotation

Codecov / codecov/patch

src/Query/ParserResult.php#L125

Added line #L125 was not covered by tests
}

/**
Expand Down
6 changes: 2 additions & 4 deletions tests/Tests/ORM/Functional/ParserResultSerializationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand All @@ -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<string, array{Closure(ParserResult): ParserResult}> */
Expand Down
2 changes: 1 addition & 1 deletion tests/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 9c93af9

Please sign in to comment.