Skip to content
Snippets Groups Projects
Verified Commit 513933de authored by Alex Pott's avatar Alex Pott
Browse files

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)
parent 8f3a9be2
Branches
Tags
11 merge requests!8506Draft: Issue #3456536 by ibrahim tameme,!5646Issue #3350972 by nod_: [random test failure]...,!5600Issue #3350972 by nod_: [random test failure]...,!5343Issue #3305066 by quietone, Rename RedirectLeadingSlashesSubscriber,!3603#ISSUE 3346218 Add a different message on edit comment,!3555Issue #2473873: Views entity operations lack cacheability support, resulting in incorrect dropbuttons,!3494Issue #3327018 by Spokje, longwave, xjm, mondrake: Update PHPStan to 1.9.3 and...,!3410Issue #3340128: UserLoginForm::submitForm has some dead code,!3389Issue #3325184 by Spokje, andypost, xjm, smustgrave: $this->configFactory is...,!3381Issue #3332363: Refactor Claro's menus-and-lists stylesheet,!3307Issue #3326193: CKEditor 5 can grow past the viewport when there is a lot of content
......@@ -878,7 +878,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
......
......@@ -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.');
}
/**
......
......@@ -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']);
}
}
......@@ -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;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment