Make Paginator-internal query cacheable in the query cache

Make the `Paginator`-internal query (`... WHERE ... IN (id, id2,
id3...)`) cacheable in the query cache again.

When the Paginator creates the internal subquery that does the actual
result limiting, it has to take DBAL type conversions for the identifier
column of the paginated root entity into account (doctrine#7820, fixed in
 doctrine#7821).

In order to perform this type conversion, we need to know the DBAL type
class for the root entity's `#[Id]`, and we have to figure it out based
on a given (arbitrary) DQL query. This requires DQL parsing and
inspecting the AST, so doctrine#7821 placed the conversion code in the
`WhereInWalker` where all the necessary information is available.

The problem is that type conversion has to happen every time the
paginator is run, but the query that results from running
`WhereInWalker` would be kept in the query cache. This was reported in
 doctrine#7837 and fixed by doctrine#7865, by making this particular query expire every
time. The query must not be cached, since the necessary ID type
conversion happens as a side-effect of running the `WhereInWalker`.

The Paginator internal query that uses `WhereInWalker` has its DQL
re-parsed and transformed in every request.

This PR moves the code that determines the DBAL type out of
`WhereInWalker` into a dedicated SQL walker class, `RootTypeWalker`.

`RootTypeWalker` uses a ~hack~  clever trick to report the type back: It
sets the type as the resulting "SQL" string. The benefit is that
`RootTypeWalker` results can be cached in the query cache themselves.
Only the first time a given DQL query has to be paginated, we need to
run this walker to find out the root entity's ID type. After that, the
type will be returned from the query cache.

With the type information being provided, `Paginator` can take care of
the necessary conversions by itself. This happens every time the
Paginator is used.

The internal query that uses `WhereInWalker` can be cached again since
it no longer has side effects.

lib/Doctrine/ORM/Tools/Pagination/Paginator.php

@@ -21,7 +21,9 @@
 use function array_key_exists;
 use function array_map;
 use function array_sum;
+use function assert;
 use function count;
+use function is_string;
 
 /**
  * The paginator can handle various complex scenarios with DQL.
@@ -160,9 +162,10 @@ public function getIterator()
  $this->appendTreeWalker($whereInQuery, WhereInWalker::class);
  $whereInQuery->setHint(WhereInWalker::HINT_PAGINATOR_ID_COUNT, count($ids));
  $whereInQuery->setFirstResult(0)->setMaxResults(null);
- $whereInQuery->setParameter(WhereInWalker::PAGINATOR_ID_ALIAS, $ids);
  $whereInQuery->setCacheable($this->query->isCacheable());
- $whereInQuery->useQueryCache(false);
+
+ $databaseIds = $this->convertWhereInIdentifiersToDatabaseValues($ids);
+ $whereInQuery->setParameter(WhereInWalker::PAGINATOR_ID_ALIAS, $databaseIds);
 
  $result = $whereInQuery->getResult($this->query->getHydrationMode());
  } else {
@@ -265,4 +268,23 @@ private function unbindUnusedQueryParams(Query $query): void
 
  $query->setParameters($parameters);
  }
+
+ /**
+ * @param mixed[] $identifiers
+ *
+ * @return mixed[]
+ */
+ private function convertWhereInIdentifiersToDatabaseValues(array $identifiers): array
+ {
+ $query = $this->cloneQuery($this->query);
+ $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, RootTypeWalker::class);
+
+ $connection = $this->query->getEntityManager()->getConnection();
+ $type = $query->getSQL();
+ assert(is_string($type));
+
+ return array_map(static function ($id) use ($connection, $type) {
+ return $connection->convertToDatabaseValue($id, $type);
+ }, $identifiers);
+ }
 }

lib/Doctrine/ORM/Tools/Pagination/RootTypeWalker.php

@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Tools\Pagination;
+
+use Doctrine\ORM\Query\AST;
+use Doctrine\ORM\Query\SqlWalker;
+use Doctrine\ORM\Utility\PersisterHelper;
+use RuntimeException;
+
+use function count;
+use function reset;
+
+/**
+ * Infers the DBAL type of the #Id (identifier) column of the given query's root entity, and
+ * returns it in place of a real SQL statement.
+ *
+ * Obtaining this type is a necessary intermediate step for \Doctrine\ORM\Tools\Pagination\Paginator.
+ * We can best do this from a tree walker because it gives us access to the AST.
+ *
+ * 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.
+ */
+class RootTypeWalker extends SqlWalker
+{
+ public function walkSelectStatement(AST\SelectStatement $AST): string
+ {
+ // Get the root entity and alias from the AST fromClause
+ $from = $AST->fromClause->identificationVariableDeclarations;
+
+ if (count($from) > 1) {
+ throw new RuntimeException('Can only process queries that select only one FROM component');
+ }
+
+ $fromRoot = reset($from);
+ $rootAlias = $fromRoot->rangeVariableDeclaration->aliasIdentificationVariable;
+ $rootClass = $this->getMetadataForDqlAlias($rootAlias);
+ $identifierFieldName = $rootClass->getSingleIdentifierFieldName();
+
+ return PersisterHelper::getTypeOfField(
+ $identifierFieldName,
+ $rootClass,
+ $this->getQuery()
+ ->getEntityManager()
+ )[0];
+ }
+}

lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php

@@ -17,13 +17,9 @@
 use Doctrine\ORM\Query\AST\SimpleArithmeticExpression;
 use Doctrine\ORM\Query\AST\WhereClause;
 use Doctrine\ORM\Query\TreeWalkerAdapter;
-use Doctrine\ORM\Utility\PersisterHelper;
 use RuntimeException;
 
-use function array_map;
-use function assert;
 use function count;
-use function is_array;
 use function reset;
 
 /**
@@ -80,15 +76,6 @@ public function walkSelectStatement(SelectStatement $AST)
  $arithmeticExpression,
  [new InputParameter(':' . self::PAGINATOR_ID_ALIAS)]
  );
-
- $this->convertWhereInIdentifiersToDatabaseValue(
- PersisterHelper::getTypeOfField(
- $identifierFieldName,
- $rootClass,
- $this->_getQuery()
- ->getEntityManager()
- )[0]
- );
  } else {
  $expression = new NullComparisonExpression($pathExpression);
  }
@@ -130,24 +117,4 @@ public function walkSelectStatement(SelectStatement $AST)
  );
  }
  }
-
- private function convertWhereInIdentifiersToDatabaseValue(string $type): void
- {
- $query = $this->_getQuery();
- $identifiersParameter = $query->getParameter(self::PAGINATOR_ID_ALIAS);
-
- assert($identifiersParameter !== null);
-
- $identifiers = $identifiersParameter->getValue();
-
- assert(is_array($identifiers));
-
- $connection = $this->_getQuery()
- ->getEntityManager()
- ->getConnection();
-
- $query->setParameter(self::PAGINATOR_ID_ALIAS, array_map(static function ($id) use ($connection, $type) {
- return $connection->convertToDatabaseValue($id, $type);
- }, $identifiers));
- }
 }

psalm-baseline.xml

@@ -2959,9 +2959,6 @@
  <code>$AST-&gt;whereClause-&gt;conditionalExpression instanceof ConditionalFactor</code>
  <code>$AST-&gt;whereClause-&gt;conditionalExpression instanceof ConditionalPrimary</code>
  </DocblockTypeContradiction>
- <MissingClosureReturnType>
- <code>static function ($id) use ($connection, $type) {</code>
- </MissingClosureReturnType>
  <PossiblyInvalidPropertyAssignmentValue>
  <code>$AST-&gt;whereClause-&gt;conditionalExpression</code>
  </PossiblyInvalidPropertyAssignmentValue>

psalm.xml

@@ -231,5 +231,10 @@
  <referencedMethod name="Doctrine\DBAL\Platforms\AbstractPlatform::getGuidExpression"/>
  </errorLevel>
  </UndefinedMethod>
+ <MissingClosureReturnType>
+ <errorLevel type="suppress">
+ <file name="lib/Doctrine/ORM/Tools/Pagination/Paginator.php"/>
+ </errorLevel>
+ </MissingClosureReturnType>
  </issueHandlers>
 </psalm>

tests/Doctrine/Tests/ORM/Functional/Ticket/GH7820Test.php

@@ -16,6 +16,7 @@
 use PHPUnit\Framework\Assert;
 use Psr\Cache\CacheItemInterface;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Component\Cache\CacheItem;
 
 use function array_map;
 use function is_string;
@@ -112,6 +113,35 @@ public function save(CacheItemInterface $item): bool
  $this->fetchSongLinesWithPaginator();
  }
 
+ public function testPaginatorQueriesWillBeCached(): void
+ {
+ $cache = new class extends ArrayAdapter {
+ /** @var bool */
+ private $failOnCacheMiss = false;
+
+ public function failOnCacheMiss(): void
+ {
+ $this->failOnCacheMiss = true;
+ }
+
+ public function getItem($key): CacheItem
+ {
+ $item = parent::getItem($key);
+ Assert::assertTrue(! $this->failOnCacheMiss || $item->isHit(), 'cache was missed');
+
+ return $item;
+ }
+ };
+ $this->_em->getConfiguration()->setQueryCache($cache);
+
+ // Prime the cache
+ $this->fetchSongLinesWithPaginator();
+
+ $cache->failOnCacheMiss();
+
+ $this->fetchSongLinesWithPaginator();
+ }
+
  private function fetchSongLinesWithPaginator(): array
  {
  $query = $this->_em->getRepository(GH7820Line::class)

tests/Doctrine/Tests/ORM/Tools/Pagination/RootTypeWalkerTest.php

@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Tools\Pagination;
+
+use Doctrine\DBAL\Types\Type;
+use Doctrine\ORM\Query;
+use Doctrine\ORM\Tools\Pagination\RootTypeWalker;
+use Doctrine\Tests\DbalTypes\Rot13Type;
+use Doctrine\Tests\Models\ValueConversionType\AuxiliaryEntity;
+use Doctrine\Tests\Models\ValueConversionType\OwningManyToOneIdForeignKeyEntity;
+use Generator;
+
+class RootTypeWalkerTest extends PaginationTestCase
+{
+ protected function setUp(): void
+ {
+ parent::setUp();
+
+ if (! Type::hasType('rot13')) {
+ Type::addType('rot13', Rot13Type::class);
+ }
+ }
+
+ /**
+ * @dataProvider exampleQueries
+ */
+ public function testResolveTypeMapping(string $dqlQuery, string $expectedType): void
+ {
+ $query = $this->entityManager->createQuery($dqlQuery);
+ $query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, RootTypeWalker::class);
+
+ self::assertSame($expectedType, $query->getSQL());
+ }
+
+ /** @return iterable<array{string, string}> */
+ public function exampleQueries(): Generator
+ {
+ yield 'Entity with #Id column of special type' => [
+ 'SELECT e.id4 FROM ' . AuxiliaryEntity::class . ' e',
+ 'rot13',
+ ];
+
+ yield 'Entity where #Id is a to-one relation with special type identifier' => [
+ 'SELECT e FROM ' . OwningManyToOneIdForeignKeyEntity::class . ' e',
+ 'rot13',
+ ];
+
+ yield 'Simple integer ID in a query with a JOIN' => [
+ 'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g',
+ 'integer',
+ ];
+ }
+}