Issue #3191623 by mondrake, Medha Kumari, daffie, alexpott: Select queries do...

Issue #3191623 by mondrake, Medha Kumari, daffie, alexpott: Select queries do not escape the GROUP BY fields

(cherry picked from commit 5a193a82)

core/lib/Drupal/Core/Database/Query/Select.php

@@ -868,7 +868,10 @@ public function __toString() {
 
     // GROUP BY
     if ($this->group) {
-      $query .= "\nGROUP BY " . implode(', ', $this->group);
+      $group_by_fields = array_map(function (string $field): string {
+        return $this->connection->escapeField($field);
+      }, $this->group);
+      $query .= "\nGROUP BY " . implode(', ', $group_by_fields);
     }
 
     // HAVING

core/modules/views/tests/src/Kernel/QueryGroupByTest.php

@@ -230,7 +230,7 @@ public function testGroupByBaseField() {
     $view->displayHandlers->get('default')->options['fields']['name']['group_type'] = 'min';
     unset($view->displayHandlers->get('default')->options['fields']['id']['group_type']);
     $this->executeView($view);
-    $this->assertStringContainsString('GROUP BY entity_test.id', (string) $view->build_info['query'], 'GROUP BY field includes the base table name when grouping on the base field.');
+    $this->assertMatchesRegularExpression('/GROUP BY .*[^\w\s]entity_test[^\w\s]\.[^\w\s]id[^\w\s]/', (string) $view->build_info['query'], 'GROUP BY field includes the base table name when grouping on the base field.');
   }
 
   /**

core/tests/Drupal/KernelTests/Core/Database/ReservedWordTest.php

@@ -80,4 +80,30 @@ public function testSelectReservedWordAliasAllFields() {
     $this->assertSame('27', $record->age);
   }
 
+  /**
+   * Tests SELECT query with GROUP BY clauses on fields with reserved names.
+   */
+  public function testGroupBy() {
+    $this->connection->insert('select')
+      ->fields([
+        'id' => 2,
+        'update' => 'Update value 1',
+      ])
+      ->execute();
+
+    // Using aliases.
+    $query = $this->connection->select('select', 's');
+    $query->addExpression('COUNT([id])', 'num');
+    $query->addField('s', 'update');
+    $query->groupBy('s.update');
+    $this->assertSame('2', $query->execute()->fetchAssoc()['num']);
+
+    // Not using aliases.
+    $query = $this->connection->select('select');
+    $query->addExpression('COUNT([id])', 'num');
+    $query->addField('select', 'update');
+    $query->groupBy('update');
+    $this->assertSame('2', $query->execute()->fetchAssoc()['num']);
+  }
+
 }

core/tests/Drupal/KernelTests/Core/Database/SelectComplexTest.php

@@ -82,6 +82,10 @@ public function testGroupBy() {
     $task_field = $query->addField('t', 'task');
     $query->orderBy($count_field);
     $query->groupBy($task_field);
+
+    $this->assertMatchesRegularExpression("/ORDER BY .*[^\w\s]num[^\w\s]/", (string) $query);
+    $this->assertMatchesRegularExpression("/GROUP BY .*[^\w\s]task[^\w\s]/", (string) $query);
+
     $result = $query->execute();
 
     $num_records = 0;