-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix different first/max result values taking up query cache space #11188
Merged
Merged
Changes from 30 commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
fb0b27e
Add a test covering the #11112 issue
mpdude 65c5a72
Add new OutputWalker and SqlFinalizer interfaces
mpdude f2b0a37
Add a SingleSelectSqlFinalizer that can take care of adding offset/li…
mpdude 04b89f5
In SqlWalker, split SQL query generation into the two parts that shal…
mpdude 9f085f8
Fix CS violations
mpdude 89e5084
Skip the GH11112 test while applying refactorings
mpdude 67f3108
Avoid a Psalm complaint due to invalid (?) docblock syntax
mpdude 47ff896
Establish alternate code path - queries can obtain the sql executor t…
mpdude 9b6c9dd
Remove a possibly premature comment
mpdude 77062c4
Re-enable the #11112 test
mpdude 9695ec6
Fix CS
mpdude a477dfe
Make RootTypeWalker inherit from SqlOutputWalker so it becomes finali…
mpdude c098fbc
Update QueryCacheTest, since first/max results no longer need extra c…
mpdude 5b7a340
Fix ParserResultSerializationTest by forcing the parser to produce a …
mpdude 1573420
Fix WhereInWalkerTest
mpdude 293771d
Update lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php
mpdude 4f0c01f
Fix tests
mpdude 302a9b7
Fix a Psalm complaint
mpdude 02cad0b
Fix a test
mpdude c27c800
Fix CS
mpdude 6b4c9a2
Make the NullSqlWalker an instance of SqlOutputWalker
mpdude 5103ccc
Merge remote-tracking branch 'origin/2.17.x' into fix-11112
mpdude eb64795
Avoid multiple cache entries caused by LimitSubqueryOutputWalker
mpdude dee1818
Fix Psalm complaints
mpdude e270a84
Merge branch '2.17.x' into fix-11112
mpdude 2fd79ea
Fix static analysis complaints
mpdude db76e84
Remove experimental code that I committed accidentally
mpdude 5ff75bf
Remove unnecessary baseline entry
mpdude 59eed73
Make AddUnknownQueryComponentWalker subclass SqlOutputWalker
mpdude a19245f
Merge remote-tracking branch 'origin/2.18.x' into fix-11112
mpdude 259dfe7
Use more expressive exception classes
mpdude 29e1e40
Merge remote-tracking branch 'origin/2.20.x' into fix-11112
mpdude 50e1f98
Add a deprecation message
mpdude 9c93af9
Move SqlExecutor creation to ParserResult, to minimize public methods…
mpdude 8beab61
Avoid keeping the SqlExecutor in the Query, since it must be generate…
mpdude b2f14c6
Address PHPStan complaints
mpdude f4bda83
Fix tests
mpdude b674182
Small refactorings
mpdude 860bce8
Add an upgrade notice
mpdude 6092429
Small refactorings
mpdude f04927b
Update the Psalm baseline
mpdude 7f576e5
Add a missing namespace import
mpdude efd7e15
Merge remote-tracking branch 'origin/2.20.x' into fix-11112
mpdude 89a46d0
Update Psalm baseline
mpdude e180e2b
Fix CS
mpdude 534640a
Fix Psalm baseline
mpdude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\ORM\Query\Exec; | ||
|
||
use Doctrine\DBAL\Connection; | ||
use Doctrine\DBAL\Result; | ||
use Doctrine\DBAL\Types\Type; | ||
|
||
/** | ||
* SQL executor for a given, final, single SELECT SQL query | ||
* | ||
* @method string getSqlStatements() | ||
*/ | ||
class FinalizedSelectExecutor extends AbstractSqlExecutor | ||
{ | ||
public function __construct(string $sql) | ||
{ | ||
parent::__construct(); | ||
|
||
$this->sqlStatements = $sql; | ||
} | ||
|
||
/** | ||
* @param list<mixed>|array<string, mixed> $params | ||
* @param array<int, int|string|Type|null>|array<string, int|string|Type|null> $types | ||
*/ | ||
public function execute(Connection $conn, array $params, array $types): Result | ||
{ | ||
return $conn->executeQuery($this->getSqlStatements(), $params, $types, $this->queryCacheProfile); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\ORM\Query\Exec; | ||
|
||
use Doctrine\ORM\Query; | ||
|
||
/** | ||
* PreparedExecutorFinalizer is a wrapper for the SQL finalization | ||
* phase that does nothing - it is constructed with the sql executor | ||
* already. | ||
*/ | ||
final class PreparedExecutorFinalizer implements SqlFinalizer | ||
{ | ||
/** @var AbstractSqlExecutor */ | ||
private $executor; | ||
|
||
public function __construct(AbstractSqlExecutor $exeutor) | ||
{ | ||
$this->executor = $exeutor; | ||
} | ||
|
||
public function createExecutor(Query $query): AbstractSqlExecutor | ||
{ | ||
return $this->executor; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\ORM\Query\Exec; | ||
|
||
use Doctrine\DBAL\LockMode; | ||
use Doctrine\ORM\Query; | ||
use Doctrine\ORM\Query\QueryException; | ||
use Doctrine\ORM\Utility\LockSqlHelper; | ||
|
||
/** | ||
* SingleSelectSqlFinalizer finalizes a given SQL query by applying | ||
* the query's firstResult/maxResult values as well as extra read lock/write lock | ||
* statements, both through the platform-specific methods. | ||
* | ||
* The resulting, "finalized" SQL is passed to a FinalizedSelectExecutor. | ||
*/ | ||
class SingleSelectSqlFinalizer implements SqlFinalizer | ||
{ | ||
use LockSqlHelper; | ||
|
||
/** @var string */ | ||
private $sql; | ||
|
||
public function __construct(string $sql) | ||
{ | ||
$this->sql = $sql; | ||
} | ||
|
||
/** | ||
* This method exists temporarily to support old SqlWalker interfaces. | ||
* | ||
* @internal | ||
* | ||
* @psalm-internal Doctrine\ORM | ||
*/ | ||
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 .= ' ' . $this->getReadLockSQL($platform); | ||
} elseif ($lockMode === LockMode::PESSIMISTIC_WRITE) { | ||
$sql .= ' ' . $this->getWriteLockSQL($platform); | ||
} | ||
|
||
return $sql; | ||
} | ||
|
||
/** @return FinalizedSelectExecutor */ | ||
public function createExecutor(Query $query): AbstractSqlExecutor | ||
{ | ||
return new FinalizedSelectExecutor($this->finalizeSql($query)); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\ORM\Query\Exec; | ||
|
||
use Doctrine\ORM\Query; | ||
|
||
/** | ||
* SqlFinalizers are created by OutputWalkers that traversed the DQL AST. | ||
* The SqlFinalizer instance can be kept in the query cache and re-used | ||
* at a later time. | ||
* | ||
* Once the SqlFinalizer has been created or retrieved from the query cache, | ||
* it receives the Query object again in order to yield the AbstractSqlExecutor | ||
* that will then be used to execute the query. | ||
* | ||
* The SqlFinalizer may assume that the DQL that was used to build the AST | ||
* and run the OutputWalker (which created the SqlFinalizer) is equivalent to | ||
* the query that will be passed to the createExecutor() method. Potential differences | ||
* are the parameter values or firstResult/maxResult settings. | ||
*/ | ||
interface SqlFinalizer | ||
{ | ||
public function createExecutor(Query $query): AbstractSqlExecutor; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\ORM\Query; | ||
|
||
use Doctrine\ORM\Query\Exec\SqlFinalizer; | ||
|
||
/** | ||
* Interface for output walkers | ||
* | ||
* Output walkers, like tree walkers, can traverse the DQL AST to perform | ||
* their purpose. | ||
beberlei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* The goal of an OutputWalker is to ultimately provide the SqlFinalizer | ||
* which produces the final, executable SQL statement in a "finalization" phase. | ||
greg0ire marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
interface OutputWalker extends TreeWalker | ||
{ | ||
/** @param AST\DeleteStatement|AST\UpdateStatement|AST\SelectStatement $AST */ | ||
public function getFinalizer($AST): SqlFinalizer; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this immediately trigger a deprecation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It now does.