From 3e34b8e86ae599dd851d508c24c1f09c8e81aa5c Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 1 Oct 2025 19:33:18 +0200 Subject: [PATCH 1/5] Add a test to reproduce the issue --- .../ORM/Functional/Ticket/GH12183Test.php | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/Tests/ORM/Functional/Ticket/GH12183Test.php diff --git a/tests/Tests/ORM/Functional/Ticket/GH12183Test.php b/tests/Tests/ORM/Functional/Ticket/GH12183Test.php new file mode 100644 index 00000000000..26a5674557b --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH12183Test.php @@ -0,0 +1,46 @@ +useModelSet('cms'); + + parent::setUp(); + + $article = new CmsArticle(); + + $article->topic = 'Loomings'; + $article->text = 'Call me Ishmael.'; + + $this->_em->persist($article); + $this->_em->flush(); + $this->_em->clear(); + } + + public function testPaginatorCountWithOutputWalkerAfterQueryHasBeenExecuted(): void + { + $query = $this->_em->createQuery('SELECT a FROM Doctrine\Tests\Models\CMS\CmsArticle a'); + + // Paginator::count is right when the query has not yet been executed + $paginator = new Paginator($query); + $paginator->setUseOutputWalkers(false); + self::assertSame(1, $paginator->count()); + + // Execute the query + $result = $query->getResult(); + self::assertCount(1, $result); + + $paginator = new Paginator($query); + $paginator->setUseOutputWalkers(false); + self::assertSame(1, $paginator->count()); + } +} From e7e2fef56c632ee5d31dcc5ac8ef74288097dc26 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 1 Oct 2025 19:46:58 +0200 Subject: [PATCH 2/5] Fix creation of the count query, to that a new RSM is being created --- src/Tools/Pagination/Paginator.php | 17 ++++++++++++++++- .../Tests/ORM/Functional/Ticket/GH12183Test.php | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Tools/Pagination/Paginator.php b/src/Tools/Pagination/Paginator.php index a0ac0bfef4d..b8c47960f07 100644 --- a/src/Tools/Pagination/Paginator.php +++ b/src/Tools/Pagination/Paginator.php @@ -228,7 +228,22 @@ private function appendTreeWalker(Query $query, string $walkerClass): void */ private function getCountQuery(): Query { - $countQuery = $this->cloneQuery($this->query); + /* + As opposed to using self::cloneQuery, the following code does not transfer + a potentially existing result set mapping (either set directly by the user, + or taken from the parser result from a previous invocation of Query::parse()) + to the new query object. This is fine, since we are going to completely change the + select clause anyway, so a previously existing result set mapping (RSM) is probably wrong anyway. + In the case of using output walkers, we are even creating a new RSM down below. + In the case of using a tree walker, we want to have a new RSM created by the parser. + */ + $countQuery = new Query($this->query->getEntityManager()); + $countQuery->setDQL($this->query->getDQL()); + $countQuery->setParameters(clone $this->query->getParameters()); + $countQuery->setCacheable(false); + foreach ($this->query->getHints() as $name => $value) { + $countQuery->setHint($name, $value); + } if (! $countQuery->hasHint(CountWalker::HINT_DISTINCT)) { $countQuery->setHint(CountWalker::HINT_DISTINCT, true); diff --git a/tests/Tests/ORM/Functional/Ticket/GH12183Test.php b/tests/Tests/ORM/Functional/Ticket/GH12183Test.php index 26a5674557b..b6573aa1226 100644 --- a/tests/Tests/ORM/Functional/Ticket/GH12183Test.php +++ b/tests/Tests/ORM/Functional/Ticket/GH12183Test.php @@ -26,7 +26,7 @@ protected function setUp(): void $this->_em->clear(); } - public function testPaginatorCountWithOutputWalkerAfterQueryHasBeenExecuted(): void + public function testPaginatorCountWithTreeWalkerAfterQueryHasBeenExecuted(): void { $query = $this->_em->createQuery('SELECT a FROM Doctrine\Tests\Models\CMS\CmsArticle a'); From 437259556c89528495a1e209b9cace382b5fbf37 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 1 Oct 2025 19:48:04 +0200 Subject: [PATCH 3/5] Fix CS --- tests/Tests/ORM/Functional/Ticket/GH12183Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tests/ORM/Functional/Ticket/GH12183Test.php b/tests/Tests/ORM/Functional/Ticket/GH12183Test.php index b6573aa1226..15c7846ecde 100644 --- a/tests/Tests/ORM/Functional/Ticket/GH12183Test.php +++ b/tests/Tests/ORM/Functional/Ticket/GH12183Test.php @@ -19,7 +19,7 @@ protected function setUp(): void $article = new CmsArticle(); $article->topic = 'Loomings'; - $article->text = 'Call me Ishmael.'; + $article->text = 'Call me Ishmael.'; $this->_em->persist($article); $this->_em->flush(); From c2b844d2e3ffa689df2172052bb4ca18c5a824df Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sun, 26 Oct 2025 22:08:56 +0100 Subject: [PATCH 4/5] Update src/Tools/Pagination/Paginator.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Grégoire Paris --- src/Tools/Pagination/Paginator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/Pagination/Paginator.php b/src/Tools/Pagination/Paginator.php index b8c47960f07..3d96ac19c20 100644 --- a/src/Tools/Pagination/Paginator.php +++ b/src/Tools/Pagination/Paginator.php @@ -233,7 +233,7 @@ private function getCountQuery(): Query a potentially existing result set mapping (either set directly by the user, or taken from the parser result from a previous invocation of Query::parse()) to the new query object. This is fine, since we are going to completely change the - select clause anyway, so a previously existing result set mapping (RSM) is probably wrong anyway. + select clause, so a previously existing result set mapping (RSM) is probably wrong anyway. In the case of using output walkers, we are even creating a new RSM down below. In the case of using a tree walker, we want to have a new RSM created by the parser. */ From 828b06e20f99fbc299e6a58e9f69915ca31a880a Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sun, 26 Oct 2025 22:13:51 +0100 Subject: [PATCH 5/5] Update the DQL walker cookbook example --- docs/en/cookbook/dql-custom-walkers.rst | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/en/cookbook/dql-custom-walkers.rst b/docs/en/cookbook/dql-custom-walkers.rst index 4320856eb49..578a32c7c68 100644 --- a/docs/en/cookbook/dql-custom-walkers.rst +++ b/docs/en/cookbook/dql-custom-walkers.rst @@ -101,8 +101,16 @@ The ``Paginate::count(Query $query)`` looks like: { static public function count(Query $query) { - /** @var Query $countQuery */ - $countQuery = clone $query; + /* + To avoid changing the $query passed into the method and to make sure a possibly existing + ResultSetMapping is discarded, we create a new query object any copy relevant data over. + */ + $countQuery = new Query($query->getEntityManager()); + $countQuery->setDQL($query->getDQL()); + $countQuery->setParameters(clone $query->getParameters()); + foreach ($query->getHints() as $name => $value) { + $countQuery->setHint($name, $value); + } $countQuery->setHint(Query::HINT_CUSTOM_TREE_WALKERS, array('DoctrineExtensions\Paginate\CountSqlWalker')); $countQuery->setFirstResult(null)->setMaxResults(null); @@ -111,7 +119,7 @@ The ``Paginate::count(Query $query)`` looks like: } } -It clones the query, resets the limit clause first and max results +This resets the limit clause first and max results and registers the ``CountSqlWalker`` custom tree walker which will modify the AST to execute a count query. The walkers implementation is: