Skip to content

Commit 9f42bb4

Browse files
committed
[BUGFIX] Mitigate query buffer issue in OptimizeDatabaseTableTask
Using `pdo_mysql` driver with `MySQL` and `MariaDB` databases requires that buffered result sets are fully read or explicitly free'd. That means, that queries returning a result set needs to consume the full result set by reading all rows and not only one record and leaving the rest in the buffer. TYPO3 mitigates this in several places for select queries by explicitly enforcing `setMaxResults(1)` when reading only one record and it could be a duplicates, which is not an ideal or reading the full result set. The DDL `OPTIMIZE TABLE` for `MariaDB` and `MySQL` databases returns a result stating the outcome of the operation and is a buffered query, which requires to use `executeQuery()` and not `executeStatement()` on the connection and fully consume the result when using `pdo_mysql`. However, the `mysqli` driver is more forgiving here. This change now uses `executeQuery()` in the scheduler task `OptimizeDatabaseTableTask` to retrieve the result without using it to ensure buffer is consumed and works with both possible drivers for the aforementioned database vendors. As a first guard, one functional test execution is changed to use `pdo_mysql` and is scheduled to rework the matrix in a dedicated change to cover differences with both drivers for `MySQL` and `MariaDB`. Can be verified executing the single test case, working with `mysqli` and failing with `pdo_mysql` (without this change): > Build/Scripts/runTests.sh -s functional -d mariadb -a mysqli -- \ --filter 'OptimizeDatabaseTableTaskTest' > Build/Scripts/runTests.sh -s functional -d mariadb -a pdo_mysql -- \ --filter 'OptimizeDatabaseTableTaskTest' > Build/Scripts/runTests.sh -s functional -d mysql -a mysqli -- \ --filter 'OptimizeDatabaseTableTaskTest' > Build/Scripts/runTests.sh -s functional -d mysql -a pdo_mysql -- \ --filter 'OptimizeDatabaseTableTaskTest' Added tests are executed against all databases albeit the task silently does nothing for other database systems than `MariaDB` or `MySQL` to ensure that the platform detection stays in place. TYPO3 v12 used the `Connection::query()` method using `executeQuery()` internally, which means that it has been replaced with the wrong method while upgrading to Doctrine DBAL 4 with #102875 in TYPO3 v13. Resolves: #106563 Related: #102875 Releases: main, 13.4 Change-Id: Ie8cc10d917ea7695da826bb7f8ec5dfeba2c89bf Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/89082 Reviewed-by: Garvin Hicking <gh@faktor-e.de> Tested-by: core-ci <typo3@b13.com> Tested-by: Garvin Hicking <gh@faktor-e.de> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Benni Mack <benni@typo3.org>
1 parent 859c291 commit 9f42bb4

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

Build/gitlab-ci/pre-merge/functional.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
functional mariadb 10.4 php 8.3 pre-merge:
1+
functional mariadb 10.4 php 8.3 pdo_mysql pre-merge:
22
stage: main
33
tags:
44
- metal2
@@ -9,7 +9,7 @@ functional mariadb 10.4 php 8.3 pre-merge:
99
parallel: 6
1010
script:
1111
- Build/Scripts/runTests.sh -s composerInstall -p 8.3
12-
- Build/Scripts/runTests.sh -s functional -d mariadb -i 10.4 -p 8.3 -c $CI_NODE_INDEX/$CI_NODE_TOTAL
12+
- Build/Scripts/runTests.sh -s functional -d mariadb -a pdo_mysql -i 10.4 -p 8.3 -c $CI_NODE_INDEX/$CI_NODE_TOTAL
1313

1414
functional postgres 10 php 8.4 pre-merge:
1515
stage: main

typo3/sysext/scheduler/Classes/Task/OptimizeDatabaseTableTask.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ public function execute()
5353

5454
if ($platform instanceof DoctrineMariaDBPlatform || $platform instanceof DoctrineMySQLPlatform) {
5555
try {
56-
$connection->executeStatement('OPTIMIZE TABLE ' . $connection->quoteIdentifier($tableName));
56+
// `OPTIMIZE TABLE` returns a result set and must be executed using `executeQuery()`,
57+
// otherwise following database queries would fail with a database exception because of a
58+
// not-consumed query buffer with `pdo_mysql` driver and the full result set is retrieved
59+
// with `fetchAllAssociative()` and discarded as handling is not intended here.
60+
$connection->executeQuery('OPTIMIZE TABLE ' . $connection->quoteIdentifier($tableName))->fetchAllAssociative();
5761
} catch (DBALException $e) {
5862
throw new \RuntimeException(
5963
TableGarbageCollectionTask::class . ' failed for: ' . $tableName . ': ' .
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the TYPO3 CMS project.
7+
*
8+
* It is free software; you can redistribute it and/or modify it under
9+
* the terms of the GNU General Public License, either version 2
10+
* of the License, or any later version.
11+
*
12+
* For the full copyright and license information, please read the
13+
* LICENSE.txt file that was distributed with this source code.
14+
*
15+
* The TYPO3 project - inspiring people to share!
16+
*/
17+
18+
namespace TYPO3\CMS\Scheduler\Tests\Functional\Task;
19+
20+
use PHPUnit\Framework\Attributes\Test;
21+
use TYPO3\CMS\Scheduler\Task\OptimizeDatabaseTableTask;
22+
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
23+
24+
final class OptimizeDatabaseTableTaskTest extends FunctionalTestCase
25+
{
26+
protected array $coreExtensionsToLoad = ['scheduler'];
27+
28+
#[Test]
29+
public function executedSuccessfullyWithOneSelectedTable(): void
30+
{
31+
$selectedTables = ['be_users'];
32+
$optimizeDatabaseTableTask = new OptimizeDatabaseTableTask();
33+
$optimizeDatabaseTableTask->setTaskParameters([
34+
'tables' => $selectedTables,
35+
]);
36+
self::assertSame($selectedTables, (new \ReflectionProperty($optimizeDatabaseTableTask, 'selectedTables'))->getValue($optimizeDatabaseTableTask));
37+
self::assertTrue($optimizeDatabaseTableTask->execute());
38+
}
39+
40+
#[Test]
41+
public function executedSuccessfullyWithMultipleSelectedTables(): void
42+
{
43+
$selectedTables = [
44+
'be_users',
45+
'be_groups',
46+
'pages',
47+
'tt_content',
48+
];
49+
$optimizeDatabaseTableTask = new OptimizeDatabaseTableTask();
50+
$optimizeDatabaseTableTask->setTaskParameters([
51+
'tables' => $selectedTables,
52+
]);
53+
self::assertSame($selectedTables, (new \ReflectionProperty($optimizeDatabaseTableTask, 'selectedTables'))->getValue($optimizeDatabaseTableTask));
54+
self::assertTrue($optimizeDatabaseTableTask->execute());
55+
}
56+
}

0 commit comments

Comments
 (0)