Problem/Motivation

Adding only a field with a COUNT aggregation on nid, adds a second nid without a GROUP BY clause resulting in this

SQLSTATE[42000]: Syntax error or access violation: 1140 In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'db.node_field_data.nid'; this is incompatible with sql_mode=only_full_group_by: SELECT "node_field_data"."nid" AS "nid_1", COUNT(node_field_data.nid) AS "nid" FROM "node_field_data" "node_field_data" WHERE ("node_field_data"."status" = :db_condition_placeholder_0) AND ("node_field_data"."type" IN (:db_condition_placeholder_1)); Array ( [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => course_section )

This is a regression from D7 views as this would produce the COUNT only field.

Steps to reproduce

  1. Create a new node view on article bundle
  2. Remove sort clause that gets added and title field
  3. Turn on aggregation
  4. Add COUNT on nid field

Proposed resolution

Avoid adding the nid via getEntityTableInfo() when it's not needed, or add the appropriate GROUP By to the field it adds

@see
\Drupal\views\Plugin\views\query\entity_information
core/modules/views/src/Plugin/views/query/Sql.php:1344

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3247121

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented

Just a note, marking this as "Distinct" will fix the problem as a kind of workaround because at the beginning of query() it adds a groupby on nid_1 that it adds and further down, it checks for $groupby and adds min on to what is now 2 additional nid fields + a count(nid)
public/core/modules/views/src/Plugin/views/query/Sql.php:1278

The problem seems to be that $groupby is empty leading to not adding the 'min' function to the new field.
public/core/modules/views/src/Plugin/views/query/Sql.php:1337

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Title: Aggregation of nid when counting fails » Aggregation of nid when using COUNT produces syntax error
jplana’s picture

jplana CreditAttribution: jplana commented
Related issues: +#2815881: Switching on aggregation generates fatal "Column not found: 1054 Unknown column" SQL error when using multi-column Fields

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

muaz91’s picture

muaz91
Location Malaysia
CreditAttribution: muaz91 commented

Having the same problem with user id

johnnny83’s picture

johnnny83 CreditAttribution: johnnny83 commented

I get the same error when I aggregate on Content:Type with COUNT.

kazah’s picture

kazah CreditAttribution: kazah commented

Same problem with webform sumbission fields...

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

junaidpv’s picture

junaidpv
Location Kannur, Kerala
CreditAttribution: junaidpv at Detlus for Wickwood Marketing commented

Facing same issue. But we cannot apply workaround mentioned in #2 as we just want one row aggregating all results.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ramil g’s picture

ramil g CreditAttribution: ramil g at The University of British Columbia commented
Version: 10.1.x-dev » 9.5.x-dev

ramil g opened merge request !2868

ramil g’s picture

ramil g CreditAttribution: ramil g at The University of British Columbia commented
Status: Active » Needs review

Here's a merge request that fixes it. Let's see if the testbot agrees.

ramil g opened merge request !2875

ramil g opened merge request !2876

ramil g closed merge request !2876

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Status: Needs review » Reviewed & tested by the community

With red/green tests I’ll mark this RTBC. This is better and simpler than the proposed changes I had in mind.

Thanks @ramil g

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at iO commented

Reviewed it too, looks good, nice to see the minimal view for the test, makes it nice and clear what is being tested with it.

RTBC +1

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 9.5.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed bc8ed7fec1 to 10.1.x and f9e87b99e6 to 10.0.x and dde89e2238 to 9.5.x. Thanks!

Will backport to 9.4.x once the branch is not in a freeze.

diff --git a/core/modules/views/tests/src/Kernel/QueryGroupByTest.php b/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
index 4a2742439e..171688321e 100644
--- a/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
+++ b/core/modules/views/tests/src/Kernel/QueryGroupByTest.php
@@ -86,6 +86,9 @@ public function testAggregateCount() {
     $this->assertEquals(3, $types['name2']);
   }
 
+  /**
+   * Tests aggregate count feature with no group by.
+   */
   public function testAggregateCountFunction() {
     $this->setupTestEntities();
 
@@ -94,7 +97,6 @@ public function testAggregateCountFunction() {
 
     $this->assertEquals(7, $view->result[0]->id);
     $this->assertCount(1, $view->result, 'Make sure the count of rows is one.');
-
   }
 
   /**

Added some documentation to the test.

  • alexpott committed bc8ed7f on 10.1.x 
    Issue #3247121 by ramil g, joelpittet: Aggregation of nid when using...

  • alexpott committed f9e87b9 on 10.0.x 
    Issue #3247121 by ramil g, joelpittet: Aggregation of nid when using...

  • alexpott committed dde89e2 on 9.5.x 
    Issue #3247121 by ramil g, joelpittet: Aggregation of nid when using...

  • alexpott committed e9733fd on 9.4.x 
    Issue #3247121 by ramil g, joelpittet: Aggregation of nid when using...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.