Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.When a query with a group by clause is extended by PagerDefault, the count query fails.
This is due to:
public function countQuery() {
...
$group_by = array_keys($count->getGroupBy());
containing linear values as the array returned by getGroupBy() is not associative.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 1005674-count-query.patch | 1.88 KB | solotandem |
| #5 | 1005674-count-query.patch | 1.75 KB | solotandem |
| #3 | 1005674-count-query.patch | 1.95 KB | solotandem |
| #1 | 1005674-count-query.patch | 745 bytes | solotandem |
Comments
Comment #1
solotandem CreditAttribution: solotandem commentedThe attached 1-line patch seems to fix the problem.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedew. That looks right, but that means we need more tests :(
Comment #3
solotandem CreditAttribution: solotandem commentedSimplified 1-line patch with revised test.
Comment #5
solotandem CreditAttribution: solotandem commentedWho knew this was a duplicate of #1003860: Count query fails to remove fields and expressions? This 1-line patch is: 1) cleaner than what was committed on that issue (it eliminates a redundant function call), 2) the test more fully tests the count query code by adding an order by clause, and 3) the test has a comment explaining the reason to have a column alias in the test. Posting rerolled patch here instead of reopening the other issue.
Comment #6
Crell CreditAttribution: Crell commentedAs I noted in the issue linked in #5, the existence of drupal_map_assoc() inside SelectQuery is a critical bug as far as I'm concerned. If we need that functionality then inline it. It's not a complicated algorithm. Although I'm not entirely clear at the moment why we need to do that in the first place...
Comment #7
solotandem CreditAttribution: solotandem commentedThis revised 2-line patch removes the Drupal core function call and makes the groupBy() array an associative one so the count query code will work as "intended." With this change, both the group and order arrays are associative.
Comment #8
solotandem CreditAttribution: solotandem commentedComment #9
Crell CreditAttribution: Crell commentedThat looks much better to me. The additions to the unit test have me confused, though. Isn't the point of that test to make sure that the countQuery() method works? Will it still be a valid test if we're adding the COUNT() statement manually?
Comment #10
solotandem CreditAttribution: solotandem commentedThe example:
- tests the removal of expressions from the count query (when a column alias is present) (so the function could be SUM, COUNT, MIN, MAX, etc.)
- is a real life example for a form or view that displays counts of grouped items and uses a pager (possibly it should include its own count query in this case)
Comment #11
Crell CreditAttribution: Crell commentedAh, gotcha. In that case...
Comment #12
webchickGreat, thanks for the clean up and the additional test!
Committed to HEAD.