Paginator with output walker returns count 0 when the query has previously been executed

[mpdude]

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?

[janopae]

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()?

I don't really think this is about cloning. In case we modified an existing query that has already been executed without cloning, we'd run into the same issue – say

$query = $queryBuilder->select('something')->getQuery();
$query->getResult();
$query->setHint(Query::HINT_CUSTOM_TREE_WALKERS, ChangeSelectClause::class);
$query->getResult();

The point is that we add a Tree Walker, and that might alter the SELECT clause (and therefore, the result set map).

If we wanted to solve this on the level of the Query, we'd need to distinguish between

1. walkers that do not alter the SELECT clause (in this case, a user provided result set map must be kept)
2. walkers that completely replace the selected columns (in this case, we should discard any previous result set map, user provided or not)
3. walkers that change the SELECT clause but keep some of the original fields (in this case, we can't know what to do with a user-defined RSM – the only one knowing might be the Tree Walker itself. A parser generated RSM without user modifications could be discarded and re-generated.)

For the third case with a user-defined RSM, the current code structure is a bit problematic, because we always start with an empty RSM when parsing. So the walkers can't know any user-defined modifications. Can we change this without breaking existing Tree Walkers? I'm not sure.

In case we can't provide the TreeWalkers with information about a previous RSM with user modifications, and we can't get the TreeWalkers to tell us whether they modify the selected columns, I'd say we can't handle this on Query level. In this case, we should make providing the correct RSM the responsibility of the one setting the TreeWalker.

This is what this PR does: The Paginator knows it doesn't need any of the previous RSM for counting, as the TreeWalker it sets will completely replace the selected columns. Therefore, it creates a new Query with an empty RSM.