[PR #12183] [MERGED] Paginator with output walker returns count 0 when the query has previously been executed #13549

Closed
opened 2026-01-22 16:17:29 +01:00 by admin · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/doctrine/orm/pull/12183
Author: @mpdude
Created: 10/1/2025
Status: Merged
Merged: 10/27/2025
Merged by: @greg0ire

Base: 2.20.xHead: paginator-confused-result-set-mapping-initialized


📝 Commits (5)

  • 3e34b8e Add a test to reproduce the issue
  • e7e2fef Fix creation of the count query, to that a new RSM is being created
  • 4372595 Fix CS
  • c2b844d Update src/Tools/Pagination/Paginator.php
  • 828b06e Update the DQL walker cookbook example

📊 Changes

3 files changed (+73 additions, -4 deletions)

View changed files

📝 docs/en/cookbook/dql-custom-walkers.rst (+11 -3)
📝 src/Tools/Pagination/Paginator.php (+16 -1)
tests/Tests/ORM/Functional/Ticket/GH12183Test.php (+46 -0)

📄 Description

When the Paginator creates the dedicated count query, it starts with a clone of the query underlying the pagination.

If that query has previously been executed, it contains a result set mapping (RSM) in the AbstractQuery::$_resultSetMapping property. That RSM may have been provided by the user, or, more probably, will have been created by the DQL parser when the query was first analyzed.

Either way, the RSM does not match or accurately reflect the actual count query. The count query modifies at least the select clause through either an output or a tree walker. That change needs to be reflected in the RSM for hydration to work correctly.

In the output walker case, the code in Paginator::getCountQuery even creates and sets a new RSM explicitly. In the tree walker case, the RSM was not taken care of.

My current suggestion is not to clone the query in preparation for counting the items, but instead create a copy of the query to start with. This will leave the RSM as null in the new query.

I was wondering whether there might be any good reasons to keep the existing RSM since it might have been provided by the user. But, since we modify the query and completely replace the select clause with the scalar expression for counting the rows, I don't see how any pre-existing RSM could apply to the new query.

Nevertheless, reviewers might want to take a step back and consider other options we might have to fix this:

  • Query::__clone marks a query as dirty when it is cloned. Should a dirty query drop its RSM to make sure it is re-created upon the next parse()?
  • Should we make a difference between RSMs provided by the client and those derived by the parser? Maybe only put user-provided ones in the $_resultSetMapping property and keep/use them unconditionally, but treat parser-derived RSMs differently and forget about them as soon as the query gets dirty?

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/doctrine/orm/pull/12183 **Author:** [@mpdude](https://github.com/mpdude) **Created:** 10/1/2025 **Status:** ✅ Merged **Merged:** 10/27/2025 **Merged by:** [@greg0ire](https://github.com/greg0ire) **Base:** `2.20.x` ← **Head:** `paginator-confused-result-set-mapping-initialized` --- ### 📝 Commits (5) - [`3e34b8e`](https://github.com/doctrine/orm/commit/3e34b8e86ae599dd851d508c24c1f09c8e81aa5c) Add a test to reproduce the issue - [`e7e2fef`](https://github.com/doctrine/orm/commit/e7e2fef56c632ee5d31dcc5ac8ef74288097dc26) Fix creation of the count query, to that a new RSM is being created - [`4372595`](https://github.com/doctrine/orm/commit/437259556c89528495a1e209b9cace382b5fbf37) Fix CS - [`c2b844d`](https://github.com/doctrine/orm/commit/c2b844d2e3ffa689df2172052bb4ca18c5a824df) Update src/Tools/Pagination/Paginator.php - [`828b06e`](https://github.com/doctrine/orm/commit/828b06e20f99fbc299e6a58e9f69915ca31a880a) Update the DQL walker cookbook example ### 📊 Changes **3 files changed** (+73 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/en/cookbook/dql-custom-walkers.rst` (+11 -3) 📝 `src/Tools/Pagination/Paginator.php` (+16 -1) ➕ `tests/Tests/ORM/Functional/Ticket/GH12183Test.php` (+46 -0) </details> ### 📄 Description When the `Paginator` creates the dedicated count query, it starts with a clone of the query underlying the pagination. If that query has previously been executed, it contains a result set mapping (RSM) in the `AbstractQuery::$_resultSetMapping` property. That RSM may have been provided by the user, or, more probably, will have been created by the DQL parser when the query was first analyzed. Either way, the RSM does not match or accurately reflect the actual count query. The count query modifies at least the select clause through either an output or a tree walker. That change needs to be reflected in the RSM for hydration to work correctly. In the output walker case, the code in `Paginator::getCountQuery` even creates and sets a new RSM explicitly. In the tree walker case, the RSM was not taken care of. My current suggestion is not to clone the query in preparation for counting the items, but instead create a copy of the query to start with. This will leave the RSM as `null` in the new query. I was wondering whether there might be any good reasons to keep the existing RSM since it might have been provided by the user. But, since we modify the query and completely replace the select clause with the scalar expression for counting the rows, I don't see how any pre-existing RSM could apply to the new query. Nevertheless, reviewers might want to take a step back and consider other options we might have to fix this: * `Query::__clone` marks a query as dirty when it is cloned. Should a dirty query drop its RSM to make sure it is re-created upon the next `parse()`? * Should we make a difference between RSMs provided by the client and those derived by the parser? Maybe only put user-provided ones in the `$_resultSetMapping` property and keep/use them unconditionally, but treat parser-derived RSMs differently and forget about them as soon as the query gets dirty? --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
admin added the pull-request label 2026-01-22 16:17:29 +01:00
admin closed this issue 2026-01-22 16:17:29 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#13549