From 68c348da2643f94ff03f6123034775ee5f1dcc18 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 1 Mar 2020 17:41:39 +0100 Subject: [PATCH 1/4] [GH-7633] Bugfix: Partial queries were stored in 2LC. There was a check in DefaultQueryCache that prevented partial queries, because they are not supported. However the checked hint Query::HINT_FORCE_PARTIAL_LOAD is optional, so cant be used to prevent caching partial DQL queries. Introduce a new hint that the SqlWalker sets on detecing a PARTIAL query and throw an exception in the DefaultQueryCache if thats found. --- lib/Doctrine/ORM/Cache/DefaultQueryCache.php | 2 +- lib/Doctrine/ORM/Query/SqlWalker.php | 7 +++++++ .../ORM/Functional/SecondLevelCacheQueryCacheTest.php | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php index 96f0fbd0ed6..e4bac034a94 100644 --- a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php +++ b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php @@ -260,7 +260,7 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, $result, array $h throw new CacheException("Second-level cache query supports only select statements."); } - if (isset($hints[Query::HINT_FORCE_PARTIAL_LOAD]) && $hints[Query::HINT_FORCE_PARTIAL_LOAD]) { + if (isset($hints[Query\SqlWalker::HINT_PARTIAL]) && $hints[Query\SqlWalker::HINT_PARTIAL]) { throw new CacheException("Second level cache does not support partial entities."); } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index d06d070610f..836de7261a3 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -46,6 +46,11 @@ class SqlWalker implements TreeWalker */ const HINT_DISTINCT = 'doctrine.distinct'; + /** + * @var string + */ + const HINT_PARTIAL = 'doctrine.partial'; + /** * @var ResultSetMapping */ @@ -1366,6 +1371,8 @@ public function walkSelectExpression($selectExpression) default: // IdentificationVariable or PartialObjectExpression if ($expr instanceof AST\PartialObjectExpression) { + $this->query->setHint(self::HINT_PARTIAL, true); + $dqlAlias = $expr->identificationVariable; $partialFieldSet = $expr->partialFieldSet; } else { diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php index 0a32886dd0f..df7e95b7e4b 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php @@ -1095,7 +1095,6 @@ public function testCacheablePartialQueryException() $this->loadFixturesCountries(); $this->_em->createQuery("SELECT PARTIAL c.{id} FROM Doctrine\Tests\Models\Cache\Country c") - ->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true) ->setCacheable(true) ->getResult(); } From 57d64bfb257b4dd3a998f7a0dc87b30bf1358116 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 1 Mar 2020 18:36:44 +0100 Subject: [PATCH 2/4] Housekeeping: CS --- lib/Doctrine/ORM/Query/SqlWalker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 836de7261a3..e3eb975020b 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -47,9 +47,9 @@ class SqlWalker implements TreeWalker const HINT_DISTINCT = 'doctrine.distinct'; /** - * @var string + * Used to mark a query as containing a PARTIAL expression, which needs to be known by SLC. */ - const HINT_PARTIAL = 'doctrine.partial'; + public const HINT_PARTIAL = 'doctrine.partial'; /** * @var ResultSetMapping From c4a8d2cc998c5157573148c8eb17e560c5e829e0 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 1 Mar 2020 23:37:58 +0100 Subject: [PATCH 3/4] [GH-7633] HINT_FORCE_PARTIAL_LOAD still needs to be checked. --- lib/Doctrine/ORM/Cache/DefaultQueryCache.php | 2 +- .../Functional/SecondLevelCacheQueryCacheTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php index e4bac034a94..83cc6128f9b 100644 --- a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php +++ b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php @@ -260,7 +260,7 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, $result, array $h throw new CacheException("Second-level cache query supports only select statements."); } - if (isset($hints[Query\SqlWalker::HINT_PARTIAL]) && $hints[Query\SqlWalker::HINT_PARTIAL]) { + if (($hints[Query\SqlWalker::HINT_PARTIAL] ?? false) === true || ($hints[Query::HINT_FORCE_PARTIAL_LOAD] ?? false) === true) { throw new CacheException("Second level cache does not support partial entities."); } diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php index df7e95b7e4b..181229f4a02 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php @@ -1099,6 +1099,21 @@ public function testCacheablePartialQueryException() ->getResult(); } + /** + * @expectedException \Doctrine\ORM\Cache\CacheException + * @expectedExceptionMessage Second level cache does not support partial entities. + */ + public function testCacheableForcePartialLoadHintQueryException() + { + $this->evictRegions(); + $this->loadFixturesCountries(); + + $this->_em->createQuery("SELECT c FROM Doctrine\Tests\Models\Cache\Country c") + ->setCacheable(true) + ->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true) + ->getResult(); + } + /** * @expectedException \Doctrine\ORM\Cache\CacheException * @expectedExceptionMessage Second-level cache query supports only select statements. From 2ab20bb749ecdaffc7b953108c13daf0a7efc94a Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 15 Mar 2020 01:10:32 +0100 Subject: [PATCH 4/4] Housekeeping: Fix CS [skip-ci] --- .../Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php index 181229f4a02..56188ade6c0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheQueryCacheTest.php @@ -1108,7 +1108,7 @@ public function testCacheableForcePartialLoadHintQueryException() $this->evictRegions(); $this->loadFixturesCountries(); - $this->_em->createQuery("SELECT c FROM Doctrine\Tests\Models\Cache\Country c") + $this->_em->createQuery('SELECT c FROM Doctrine\Tests\Models\Cache\Country c') ->setCacheable(true) ->setHint(Query::HINT_FORCE_PARTIAL_LOAD, true) ->getResult();