Skip to content

Commit

Permalink
[BUGFIX] Ensure working count query in DatabaseRecordList::getTable()
Browse files Browse the repository at this point in the history
`\TYPO3\CMS\Backend\RecordList\DatabaseRecordList` provides the ability
to modify the QueryBuilder used for the record list representation with
the PSR-14 `ModifyDatabaseQueryForRecordListingEvent`, after generic
query information like default orderings and similar have been added.

That is covered within the `getQueryBuilder()` method, which is used in
other class methods, for example in `getTables()` where it is changed
to a `COUNT()` query.

MariaDB for example is picky about count queries using `order by`,
`group by` or aggregation methods which are not reflected or added
in the counter parts leading to a not so useful error like

  Mixing of GROUP columns (MIN(),MAX(),COUNT(),...) with no GROUP
  columns is illegal if there is no GROUP BY clause

or similar variants. This error message can be missleading for the
case a order by has been used along with a COUNT() query.

Listener of the event do not get the information if order-by can
be added or should be left out, therefore maybe adding sorting
conditions for a count query.

This change now resets any order statements for the count-query
like other places in the TYPO3 core.

Resolves: #103427
Releases: main, 12.4, 11.5
Change-Id: Ie421993a1890dd6682d02f54203d538e6ae1b76c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83532
Reviewed-by: Garvin Hicking <gh@faktor-e.de>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Garvin Hicking <gh@faktor-e.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
  • Loading branch information
sbuerk committed Apr 8, 2024
1 parent fba8c06 commit 08a78a5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
Expand Up @@ -525,7 +525,10 @@ public function getTable($table)
{
// Finding the total amount of records on the page
$queryBuilderTotalItems = $this->getQueryBuilder($table, ['*'], false, 0, 1);
$totalItems = (int)$queryBuilderTotalItems->count('*')
// @todo Switch to `resetOrderBy()` as soon as the QueryBuilder facade has that method on board.
$queryBuilderTotalItems->resetQueryPart('orderBy');
$totalItems = (int)$queryBuilderTotalItems
->count('*')
->executeQuery()
->fetchOne();
if ($totalItems === 0) {
Expand Down Expand Up @@ -2436,6 +2439,9 @@ public function getQueryBuilder(
);
}

// @todo This event should contain the $addSorting value, so listener knows when to add ORDER-BY stuff.
// Additionally, having QueryBuilder order-by with `addSorting: false` should be deprecated along
// with the additional event flag.
$event = new ModifyDatabaseQueryForRecordListingEvent(
$queryBuilder,
$table,
Expand Down
Expand Up @@ -23,6 +23,7 @@
/**
* Use this Event to alter the database query when loading content for a page (usually in the list module)
* before it is executed.
* @todo This event should contain the $addSorting value, so listener knows when to add ORDER-BY stuff.
*/
final class ModifyDatabaseQueryForRecordListingEvent
{
Expand Down

0 comments on commit 08a78a5

Please sign in to comment.