DDC-3861: Incorrect count in Paginator for queries using GROUP BY #4730

Open
opened 2026-01-22 14:48:27 +01:00 by admin · 10 comments
Owner

Originally created by @doctrinebot on GitHub (Aug 3, 2015).

Jira issue originally created by user sparrowek:

When using group by clause Paginator::count() method returns incorrect result. For example, the count metod produces the following query:
SELECT count(DISTINCT d0_.id) AS sclr_0 FROM diet_templates d0_ WHERE d0_.database_id = 3 AND d0_.type_id = 1
GROUP BY d0_.diet_id, d0_.day, d0_.meal_id

This query returns multiple records because it counts the number of records in groups and not the number of groups, eg. 1,1,1,2.
Since the count() method currently does array_sum like this:
$this->count = array_sum(array_map('current', $this->getCountQuery()->getScalarResult()));
The overall result is 5 (111+2) instead of 4.
When using group by clause the correct result whould be achived with a simple count on the result like this:
$this->count = count($this->getCountQuery()->getScalarResult());

Originally created by @doctrinebot on GitHub (Aug 3, 2015). Jira issue originally created by user sparrowek: When using group by clause Paginator::count() method returns incorrect result. For example, the count metod produces the following query: SELECT count(DISTINCT d0_.id) AS sclr_0 FROM diet_templates d0_ WHERE d0_.database_id = 3 AND d0_.type_id = 1 GROUP BY d0_.diet_id, d0_.day, d0_.meal_id This query returns multiple records because it counts the number of records in groups and not the number of groups, eg. 1,1,1,2. Since the count() method currently does array_sum like this: $this->count = array_sum(array_map('current', $this->getCountQuery()->getScalarResult())); The overall result is 5 (1<ins>1</ins>1+2) instead of 4. When using group by clause the correct result whould be achived with a simple count on the result like this: $this->count = count($this->getCountQuery()->getScalarResult());
admin added the BugMissing Tests labels 2026-01-22 14:48:27 +01:00
Author
Owner

@arnotae commented on GitHub (Dec 31, 2018):

I have exactly the same problem, is there a way to have the right count now ? Thanks ;)

@arnotae commented on GitHub (Dec 31, 2018): I have exactly the same problem, is there a way to have the right count now ? Thanks ;)
Author
Owner

@Ocramius commented on GitHub (Dec 31, 2018):

@arnotae please reproduce this with a test case first.

@Ocramius commented on GitHub (Dec 31, 2018): @arnotae please reproduce this with a test case first.
Author
Owner

@karser commented on GitHub (Jan 1, 2019):

I'm stuck with this issue too. I'll see if I can isolate it

@karser commented on GitHub (Jan 1, 2019): I'm stuck with this issue too. I'll see if I can isolate it
Author
Owner

@karser commented on GitHub (Jan 1, 2019):

I researched it, here is the explanation.
I want to see only unique types within DietTemplate:

$qb = $this->em->createQueryBuilder()
    ->select("dt.templateType")
    ->from(DietTemplate::class, 'dt')
    ->groupBy("dt.templateType");

$query = $qb->getQuery($dql)
            ->setFirstResult(0)
            ->setMaxResults(100);

$paginator = new Paginator($query, $fetchJoinCollection = true);
$totalItems = \count($paginator); //this number is wrong

This query I expect Doctrine to perform:

SELECT COUNT(DISTINCT d0_.templateType) AS sclr_0 FROM diet_templates d0_
GROUP BY d0_.templateType

The actual query:

SELECT COUNT(DISTINCT d0_.id) AS sclr_0 FROM diet_templates d0_
GROUP BY d0_.templateType

That's because Doctrine\ORM\Tools\Pagination\CountWalker takes $rootClass->getSingleIdentifierFieldName() for the COUNT clause:
image

@karser commented on GitHub (Jan 1, 2019): I researched it, here is the explanation. I want to see only unique types within DietTemplate: ``` $qb = $this->em->createQueryBuilder() ->select("dt.templateType") ->from(DietTemplate::class, 'dt') ->groupBy("dt.templateType"); $query = $qb->getQuery($dql) ->setFirstResult(0) ->setMaxResults(100); $paginator = new Paginator($query, $fetchJoinCollection = true); $totalItems = \count($paginator); //this number is wrong ``` This query I expect Doctrine to perform: ``` SELECT COUNT(DISTINCT d0_.templateType) AS sclr_0 FROM diet_templates d0_ GROUP BY d0_.templateType ``` The actual query: ``` SELECT COUNT(DISTINCT d0_.id) AS sclr_0 FROM diet_templates d0_ GROUP BY d0_.templateType ``` That's because `Doctrine\ORM\Tools\Pagination\CountWalker` takes $rootClass->getSingleIdentifierFieldName() for the COUNT clause: ![image](https://user-images.githubusercontent.com/1675033/50575026-e8ca4300-0dfc-11e9-9bbb-e012b467fa82.png)
Author
Owner

@Ocramius commented on GitHub (Jan 2, 2019):

This seems like expected behaviour, since the paginator iterates over a set of identifiers, and then retrieves matching query results for those identifiers.

The paginator examples in this issue seem to operate over arbitrary scalar values bound to those identifiers (most likely unsupported).

The correct course of action here (since selecting a subset of identifiers is the intended design) is to prevent users from having scalar-only FROM clauses, or enforcing that such scalars do not come from further aggregation operations (DISTINCT, MAX, COUNT, etc.).

@Ocramius commented on GitHub (Jan 2, 2019): This seems like expected behaviour, since the paginator iterates over a set of identifiers, and then retrieves matching query results for those identifiers. The paginator examples in this issue seem to operate over arbitrary scalar values bound to those identifiers (most likely unsupported). The correct course of action here (since selecting a subset of identifiers is the intended design) is to prevent users from having scalar-only `FROM` clauses, or enforcing that such scalars do not come from further aggregation operations (`DISTINCT`, `MAX`, `COUNT`, etc.).
Author
Owner

@karser commented on GitHub (Jan 2, 2019):

It's a pity that it's an expected behaviour. From my point (as a doctrine user), it's a bug because the paginator allows custom fields in the select / group by clause but doesn't count them properly.
if you restrict custom fields at all it'd be a BC.
I'd fix this behavior this way:
If only one field presents at the same time in select and group by clauses - then use this field as an identifier, otherwise, take getSingleIdentifierFieldName()
I could be wrong

@karser commented on GitHub (Jan 2, 2019): It's a pity that it's an expected behaviour. From my point (as a doctrine user), it's a bug because the paginator allows custom fields in the `select / group by` clause but doesn't `count` them properly. if you restrict custom fields at all it'd be a BC. I'd fix this behavior this way: If only one field presents at the same time in `select` and `group by` clauses - then use this field as an identifier, otherwise, take `getSingleIdentifierFieldName()` I could be wrong
Author
Owner

@Ocramius commented on GitHub (Jan 2, 2019):

if you restrict custom fields at all it'd be a BC.

I'd rather say that the current behavior is undefined due to a missing restriction.

then use this field as an identifier

Nothing tells us that it is an identifier, hence it is not possible to assume that.

@Ocramius commented on GitHub (Jan 2, 2019): > if you restrict custom fields at all it'd be a BC. I'd rather say that the current behavior is undefined due to a missing restriction. > then use this field as an identifier Nothing tells us that it is an identifier, hence it is not possible to assume that.
Author
Owner

@azovsky commented on GitHub (Sep 13, 2022):

I have the same issue as described above: group by FIELD, but the result counts as without any grouping.

@azovsky commented on GitHub (Sep 13, 2022): I have the same issue [as described above](https://github.com/doctrine/orm/issues/4713#issuecomment-450746916): group by FIELD, but the result counts as without any grouping.
Author
Owner

@azovsky commented on GitHub (Sep 13, 2022):

The next "fix" is working for me:

in file 'Doctrine/ORM/Tools/Pagination/CountWalker.php > function walkSelectStatement()':

        $identifierFieldName = $rootClass->getSingleIdentifierFieldName();

+       // Check GROUP BY field name.
+       if ($AST->groupByClause) {
+         /** @var \Doctrine\ORM\Query\AST\PathExpression $groupBy */
+         $groupBy = $AST->groupByClause->groupByItems[0];
+         $identifierFieldName = $groupBy->field;
+       }

        $pathType = PathExpression::TYPE_STATE_FIELD;
@azovsky commented on GitHub (Sep 13, 2022): The next "fix" is working for me: in file 'Doctrine/ORM/Tools/Pagination/CountWalker.php > function walkSelectStatement()': ```diff $identifierFieldName = $rootClass->getSingleIdentifierFieldName(); + // Check GROUP BY field name. + if ($AST->groupByClause) { + /** @var \Doctrine\ORM\Query\AST\PathExpression $groupBy */ + $groupBy = $AST->groupByClause->groupByItems[0]; + $identifierFieldName = $groupBy->field; + } $pathType = PathExpression::TYPE_STATE_FIELD; ```
Author
Owner

@kriskoch commented on GitHub (Nov 22, 2022):

For me all I do is $paginator->setUseOutputWalkers(null) and it works fine. If you look at the code if it is set to null it will basically wrap your entire query in a select count(*) from (your query)

@kriskoch commented on GitHub (Nov 22, 2022): For me all I do is $paginator->setUseOutputWalkers(null) and it works fine. If you look at the code if it is set to null it will basically wrap your entire query in a select count(*) from (your query)
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#4730