From a5ebe3631b07be8d9198b3d7e518835fbee7570c Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Thu, 5 Sep 2024 15:41:48 +0200 Subject: [PATCH 01/10] Add configuration for database connection collation --- config/global.ini.php | 7 +++ core/Db.php | 1 + core/Tracker/Db/Mysqli.php | 19 +++++-- core/Tracker/Db/Pdo/Mysql.php | 32 ++++++++++- libs/Zend/Db/Adapter/Mysqli.php | 7 ++- libs/Zend/Db/Adapter/Pdo/Mysql.php | 5 ++ tests/PHPUnit/Integration/DbTest.php | 37 +++++++++++++ .../Integration/Tracker/Db/MysqliTest.php | 53 +++++++++++++++++++ tests/PHPUnit/Integration/Tracker/DbTest.php | 21 ++++++++ 9 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php diff --git a/config/global.ini.php b/config/global.ini.php index 0c28045b51f..b4d5ba93367 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -45,6 +45,13 @@ ; Matomo should work correctly without this setting but we recommend to have a charset set. charset = utf8 +; In some database setups the collation for the connection changes after a "SET NAMES" statement +; is issued, unless a "COLLATE" argument is added. +; If you encounter "Illegal mix of collation" errors, setting this config to the value matching +; your existing database tables can help. +; This setting will only be used if "charset" is also set. +connection_collation = + ; Database error codes to ignore during updates ; ;ignore_error_codes[] = 1105 diff --git a/core/Db.php b/core/Db.php index 6df549b5fc7..81d8f708c79 100644 --- a/core/Db.php +++ b/core/Db.php @@ -186,6 +186,7 @@ public static function createReaderDatabaseObject($dbConfig = null) $dbConfig['type'] = $masterDbConfig['type']; $dbConfig['tables_prefix'] = $masterDbConfig['tables_prefix']; $dbConfig['charset'] = $masterDbConfig['charset']; + $dbConfig['connection_collation'] = $masterDbConfig['connection_collation'] ?? null; $db = @Adapter::factory($dbConfig['adapter'], $dbConfig); diff --git a/core/Tracker/Db/Mysqli.php b/core/Tracker/Db/Mysqli.php index b5c769d9864..77d084b73bf 100644 --- a/core/Tracker/Db/Mysqli.php +++ b/core/Tracker/Db/Mysqli.php @@ -26,6 +26,7 @@ class Mysqli extends Db protected $username; protected $password; protected $charset; + protected $connectionCollation; protected $activeTransaction = false; protected $enable_ssl; @@ -57,11 +58,12 @@ public function __construct($dbInfo, $driverName = 'mysql') $this->port = (int)$dbInfo['port']; $this->socket = null; } + $this->dbname = $dbInfo['dbname']; $this->username = $dbInfo['username']; $this->password = $dbInfo['password']; - $this->charset = isset($dbInfo['charset']) ? $dbInfo['charset'] : null; - + $this->charset = $dbInfo['charset'] ?? null; + $this->connectionCollation = $dbInfo['connection_collation'] ?? null; if (!empty($dbInfo['enable_ssl'])) { $this->enable_ssl = $dbInfo['enable_ssl']; @@ -133,8 +135,17 @@ public function connect() throw new DbException("Connect failed: " . mysqli_connect_error()); } - if ($this->charset && !mysqli_set_charset($this->connection, $this->charset)) { - throw new DbException("Set Charset failed: " . mysqli_error($this->connection)); + if ($this->charset && $this->connectionCollation) { + // mysqli_set_charset does not support setting a collation + $query = "SET NAMES '" . $this->charset . "' COLLATE '" . $this->connectionCollation . "'"; + + if (!mysqli_query($this->connection, $query)) { + throw new DbException("Set charset/connection collation failed: " . mysqli_error($this->connection)); + } + } elseif ($this->charset) { + if (!mysqli_set_charset($this->connection, $this->charset)) { + throw new DbException("Set Charset failed: " . mysqli_error($this->connection)); + } } $this->password = ''; diff --git a/core/Tracker/Db/Pdo/Mysql.php b/core/Tracker/Db/Pdo/Mysql.php index 3d9a75f2a70..b8493fbf2d8 100644 --- a/core/Tracker/Db/Pdo/Mysql.php +++ b/core/Tracker/Db/Pdo/Mysql.php @@ -26,13 +26,33 @@ class Mysql extends Db * @var PDO */ protected $connection = null; + + /** + * @var string + */ protected $dsn; + + /** + * @var string + */ private $username; + + /** + * @var string + */ private $password; + + /** + * @var string|null + */ protected $charset; - protected $mysqlOptions = array(); + /** + * @var string|null + */ + private $connectionCollation; + protected $mysqlOptions = []; protected $activeTransaction = false; @@ -58,8 +78,11 @@ public function __construct($dbInfo, $driverName = 'mysql') if (isset($dbInfo['charset'])) { $this->charset = $dbInfo['charset']; $this->dsn .= ';charset=' . $this->charset; - } + if (!empty($dbInfo['connection_collation'])) { + $this->connectionCollation = $dbInfo['connection_collation']; + } + } if (isset($dbInfo['enable_ssl']) && $dbInfo['enable_ssl']) { if (!empty($dbInfo['ssl_key'])) { @@ -409,6 +432,11 @@ private function establishConnection(): void */ if (!empty($this->charset)) { $sql = "SET NAMES '" . $this->charset . "'"; + + if (!empty($this->connectionCollation)) { + $sql .= " COLLATE '" . $this->connectionCollation . "'"; + } + $this->connection->exec($sql); } } diff --git a/libs/Zend/Db/Adapter/Mysqli.php b/libs/Zend/Db/Adapter/Mysqli.php index 2dcdd67813f..4f0a1ad9c38 100644 --- a/libs/Zend/Db/Adapter/Mysqli.php +++ b/libs/Zend/Db/Adapter/Mysqli.php @@ -375,7 +375,12 @@ protected function _connect() throw new Zend_Db_Adapter_Mysqli_Exception(mysqli_connect_error()); } - if (!empty($this->_config['charset'])) { + if (!empty($this->_config['charset']) && !empty($this->_config['connection_collation'])) { + // mysqli_set_charset does not support setting a collation + $query = "SET NAMES '" . $this->_config['charset'] . "' COLLATE '" . $this->_config['connection_collation'] . "'"; + + mysqli_query($this->_connection, $query); + } elseif (!empty($this->_config['charset'])) { mysqli_set_charset($this->_connection, $this->_config['charset']); } } diff --git a/libs/Zend/Db/Adapter/Pdo/Mysql.php b/libs/Zend/Db/Adapter/Pdo/Mysql.php index 363196334fc..6c3665421a8 100644 --- a/libs/Zend/Db/Adapter/Pdo/Mysql.php +++ b/libs/Zend/Db/Adapter/Pdo/Mysql.php @@ -103,6 +103,11 @@ protected function _connect() if (!empty($this->_config['charset'])) { $initCommand = "SET NAMES '" . $this->_config['charset'] . "'"; + + if (!empty($this->_config['connection_collation'])) { + $initCommand .= " COLLATE '" . $this->_config['connection_collation'] . "'"; + } + $this->_config['driver_options'][1002] = $initCommand; // 1002 = PDO::MYSQL_ATTR_INIT_COMMAND } diff --git a/tests/PHPUnit/Integration/DbTest.php b/tests/PHPUnit/Integration/DbTest.php index 5656d6ab627..56b0ca4bb7e 100644 --- a/tests/PHPUnit/Integration/DbTest.php +++ b/tests/PHPUnit/Integration/DbTest.php @@ -257,6 +257,43 @@ public function testGetRowCount($adapter, $expectedClass) $this->assertEquals(1, $db->rowCount($result)); } + /** + * @dataProvider getDbAdapter + */ + public function testConnectionCollationDefault(string $adapter, string $expectedClass): void + { + Db::destroyDatabaseObject(); + + $config = Config::getInstance(); + $config->database['adapter'] = $adapter; + $config->database['connection_collation'] = null; + + $db = Db::get(); + self::assertInstanceOf($expectedClass, $db); + + // exact value depends on database used + $currentCollation = $db->fetchOne('SELECT @@collation_connection'); + self::assertStringStartsWith('utf8', $currentCollation); + } + + /** + * @dataProvider getDbAdapter + */ + public function testConnectionCollationSetInConfig(string $adapter, string $expectedClass): void + { + Db::destroyDatabaseObject(); + + $config = Config::getInstance(); + $config->database['adapter'] = $adapter; + $config->database['connection_collation'] = $config->database['charset'] . '_swedish_ci'; + + $db = Db::get(); + self::assertInstanceOf($expectedClass, $db); + + $currentCollation = $db->fetchOne('SELECT @@collation_connection'); + self::assertSame($config->database['connection_collation'], $currentCollation); + } + public function getDbAdapter() { return array( diff --git a/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php b/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php new file mode 100644 index 00000000000..79d326304a9 --- /dev/null +++ b/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php @@ -0,0 +1,53 @@ +database['adapter'] = 'MYSQLI'; + } + + public function testConnectionThrowsOnInvalidCharset(): void + { + self::expectException(Tracker\Db\DbException::class); + self::expectExceptionMessageMatches('/Set Charset failed/'); + + $config = Config::getInstance(); + $config->database['charset'] = 'something really invalid'; + + Tracker\Db::connectPiwikTrackerDb(); + } + + public function testConnectionThrowsOnInvalidConnectionCollation(): void + { + self::expectException(Tracker\Db\DbException::class); + self::expectExceptionMessageMatches('/Set charset\/connection collation failed/'); + + $config = Config::getInstance(); + $config->database['connection_collation'] = 'something really invalid'; + + Tracker\Db::connectPiwikTrackerDb(); + } +} diff --git a/tests/PHPUnit/Integration/Tracker/DbTest.php b/tests/PHPUnit/Integration/Tracker/DbTest.php index df630f24d98..569f69bba18 100644 --- a/tests/PHPUnit/Integration/Tracker/DbTest.php +++ b/tests/PHPUnit/Integration/Tracker/DbTest.php @@ -192,6 +192,27 @@ public function testFetchAllNoMatch() $this->assertEquals(array(), $val); } + public function testConnectionCollationDefault(): void + { + $config = Config::getInstance(); + $config->database['connection_collation'] = null; + $db = Tracker\Db::connectPiwikTrackerDb(); + + // exact value depends on database used + $currentCollation = $db->fetchOne('SELECT @@collation_connection'); + self::assertStringStartsWith('utf8', $currentCollation); + } + + public function testConnectionCollationSetInConfig(): void + { + $config = Config::getInstance(); + $config->database['connection_collation'] = $config->database['charset'] . '_swedish_ci'; + $db = Tracker\Db::connectPiwikTrackerDb(); + + $currentCollation = $db->fetchOne('SELECT @@collation_connection'); + self::assertSame($config->database['connection_collation'], $currentCollation); + } + private function insertRowId($value = '1') { $db = Tracker::getDatabase(); From 1d7429a4e2bf798590f1f03c14058ed6842b2e0c Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Mon, 9 Sep 2024 20:11:06 +0200 Subject: [PATCH 02/10] Rename "connection_collation" config to "collation" --- config/global.ini.php | 2 +- core/Db.php | 2 +- core/Tracker/Db/Mysqli.php | 8 ++++---- core/Tracker/Db/Pdo/Mysql.php | 10 +++++----- libs/Zend/Db/Adapter/Mysqli.php | 4 ++-- libs/Zend/Db/Adapter/Pdo/Mysql.php | 4 ++-- tests/PHPUnit/Integration/DbTest.php | 6 +++--- tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php | 2 +- tests/PHPUnit/Integration/Tracker/DbTest.php | 6 +++--- 9 files changed, 22 insertions(+), 22 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index b4d5ba93367..d683536ce53 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -50,7 +50,7 @@ ; If you encounter "Illegal mix of collation" errors, setting this config to the value matching ; your existing database tables can help. ; This setting will only be used if "charset" is also set. -connection_collation = +collation = ; Database error codes to ignore during updates ; diff --git a/core/Db.php b/core/Db.php index 81d8f708c79..fe6aeb1515d 100644 --- a/core/Db.php +++ b/core/Db.php @@ -186,7 +186,7 @@ public static function createReaderDatabaseObject($dbConfig = null) $dbConfig['type'] = $masterDbConfig['type']; $dbConfig['tables_prefix'] = $masterDbConfig['tables_prefix']; $dbConfig['charset'] = $masterDbConfig['charset']; - $dbConfig['connection_collation'] = $masterDbConfig['connection_collation'] ?? null; + $dbConfig['collation'] = $masterDbConfig['collation'] ?? null; $db = @Adapter::factory($dbConfig['adapter'], $dbConfig); diff --git a/core/Tracker/Db/Mysqli.php b/core/Tracker/Db/Mysqli.php index 77d084b73bf..05ac1843f36 100644 --- a/core/Tracker/Db/Mysqli.php +++ b/core/Tracker/Db/Mysqli.php @@ -26,7 +26,7 @@ class Mysqli extends Db protected $username; protected $password; protected $charset; - protected $connectionCollation; + protected $collation; protected $activeTransaction = false; protected $enable_ssl; @@ -63,7 +63,7 @@ public function __construct($dbInfo, $driverName = 'mysql') $this->username = $dbInfo['username']; $this->password = $dbInfo['password']; $this->charset = $dbInfo['charset'] ?? null; - $this->connectionCollation = $dbInfo['connection_collation'] ?? null; + $this->collation = $dbInfo['collation'] ?? null; if (!empty($dbInfo['enable_ssl'])) { $this->enable_ssl = $dbInfo['enable_ssl']; @@ -135,9 +135,9 @@ public function connect() throw new DbException("Connect failed: " . mysqli_connect_error()); } - if ($this->charset && $this->connectionCollation) { + if ($this->charset && $this->collation) { // mysqli_set_charset does not support setting a collation - $query = "SET NAMES '" . $this->charset . "' COLLATE '" . $this->connectionCollation . "'"; + $query = "SET NAMES '" . $this->charset . "' COLLATE '" . $this->collation . "'"; if (!mysqli_query($this->connection, $query)) { throw new DbException("Set charset/connection collation failed: " . mysqli_error($this->connection)); diff --git a/core/Tracker/Db/Pdo/Mysql.php b/core/Tracker/Db/Pdo/Mysql.php index b8493fbf2d8..d4e0714c791 100644 --- a/core/Tracker/Db/Pdo/Mysql.php +++ b/core/Tracker/Db/Pdo/Mysql.php @@ -50,7 +50,7 @@ class Mysql extends Db /** * @var string|null */ - private $connectionCollation; + private $collation; protected $mysqlOptions = []; @@ -79,8 +79,8 @@ public function __construct($dbInfo, $driverName = 'mysql') $this->charset = $dbInfo['charset']; $this->dsn .= ';charset=' . $this->charset; - if (!empty($dbInfo['connection_collation'])) { - $this->connectionCollation = $dbInfo['connection_collation']; + if (!empty($dbInfo['collation'])) { + $this->collation = $dbInfo['collation']; } } @@ -433,8 +433,8 @@ private function establishConnection(): void if (!empty($this->charset)) { $sql = "SET NAMES '" . $this->charset . "'"; - if (!empty($this->connectionCollation)) { - $sql .= " COLLATE '" . $this->connectionCollation . "'"; + if (!empty($this->collation)) { + $sql .= " COLLATE '" . $this->collation . "'"; } $this->connection->exec($sql); diff --git a/libs/Zend/Db/Adapter/Mysqli.php b/libs/Zend/Db/Adapter/Mysqli.php index 4f0a1ad9c38..50631bd4de0 100644 --- a/libs/Zend/Db/Adapter/Mysqli.php +++ b/libs/Zend/Db/Adapter/Mysqli.php @@ -375,9 +375,9 @@ protected function _connect() throw new Zend_Db_Adapter_Mysqli_Exception(mysqli_connect_error()); } - if (!empty($this->_config['charset']) && !empty($this->_config['connection_collation'])) { + if (!empty($this->_config['charset']) && !empty($this->_config['collation'])) { // mysqli_set_charset does not support setting a collation - $query = "SET NAMES '" . $this->_config['charset'] . "' COLLATE '" . $this->_config['connection_collation'] . "'"; + $query = "SET NAMES '" . $this->_config['charset'] . "' COLLATE '" . $this->_config['collation'] . "'"; mysqli_query($this->_connection, $query); } elseif (!empty($this->_config['charset'])) { diff --git a/libs/Zend/Db/Adapter/Pdo/Mysql.php b/libs/Zend/Db/Adapter/Pdo/Mysql.php index 6c3665421a8..ff064e894e2 100644 --- a/libs/Zend/Db/Adapter/Pdo/Mysql.php +++ b/libs/Zend/Db/Adapter/Pdo/Mysql.php @@ -104,8 +104,8 @@ protected function _connect() if (!empty($this->_config['charset'])) { $initCommand = "SET NAMES '" . $this->_config['charset'] . "'"; - if (!empty($this->_config['connection_collation'])) { - $initCommand .= " COLLATE '" . $this->_config['connection_collation'] . "'"; + if (!empty($this->_config['collation'])) { + $initCommand .= " COLLATE '" . $this->_config['collation'] . "'"; } $this->_config['driver_options'][1002] = $initCommand; // 1002 = PDO::MYSQL_ATTR_INIT_COMMAND diff --git a/tests/PHPUnit/Integration/DbTest.php b/tests/PHPUnit/Integration/DbTest.php index 56b0ca4bb7e..37f85b6841c 100644 --- a/tests/PHPUnit/Integration/DbTest.php +++ b/tests/PHPUnit/Integration/DbTest.php @@ -266,7 +266,7 @@ public function testConnectionCollationDefault(string $adapter, string $expected $config = Config::getInstance(); $config->database['adapter'] = $adapter; - $config->database['connection_collation'] = null; + $config->database['collation'] = null; $db = Db::get(); self::assertInstanceOf($expectedClass, $db); @@ -285,13 +285,13 @@ public function testConnectionCollationSetInConfig(string $adapter, string $expe $config = Config::getInstance(); $config->database['adapter'] = $adapter; - $config->database['connection_collation'] = $config->database['charset'] . '_swedish_ci'; + $config->database['collation'] = $config->database['charset'] . '_swedish_ci'; $db = Db::get(); self::assertInstanceOf($expectedClass, $db); $currentCollation = $db->fetchOne('SELECT @@collation_connection'); - self::assertSame($config->database['connection_collation'], $currentCollation); + self::assertSame($config->database['collation'], $currentCollation); } public function getDbAdapter() diff --git a/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php b/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php index 79d326304a9..d6359887c7d 100644 --- a/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php +++ b/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php @@ -46,7 +46,7 @@ public function testConnectionThrowsOnInvalidConnectionCollation(): void self::expectExceptionMessageMatches('/Set charset\/connection collation failed/'); $config = Config::getInstance(); - $config->database['connection_collation'] = 'something really invalid'; + $config->database['collation'] = 'something really invalid'; Tracker\Db::connectPiwikTrackerDb(); } diff --git a/tests/PHPUnit/Integration/Tracker/DbTest.php b/tests/PHPUnit/Integration/Tracker/DbTest.php index 569f69bba18..9db9837bb9a 100644 --- a/tests/PHPUnit/Integration/Tracker/DbTest.php +++ b/tests/PHPUnit/Integration/Tracker/DbTest.php @@ -195,7 +195,7 @@ public function testFetchAllNoMatch() public function testConnectionCollationDefault(): void { $config = Config::getInstance(); - $config->database['connection_collation'] = null; + $config->database['collation'] = null; $db = Tracker\Db::connectPiwikTrackerDb(); // exact value depends on database used @@ -206,11 +206,11 @@ public function testConnectionCollationDefault(): void public function testConnectionCollationSetInConfig(): void { $config = Config::getInstance(); - $config->database['connection_collation'] = $config->database['charset'] . '_swedish_ci'; + $config->database['collation'] = $config->database['charset'] . '_swedish_ci'; $db = Tracker\Db::connectPiwikTrackerDb(); $currentCollation = $db->fetchOne('SELECT @@collation_connection'); - self::assertSame($config->database['connection_collation'], $currentCollation); + self::assertSame($config->database['collation'], $currentCollation); } private function insertRowId($value = '1') From 1907ff75963af97ec0df5b78d1db5c75fd03ad38 Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Mon, 9 Sep 2024 20:59:40 +0200 Subject: [PATCH 03/10] Pass configured database collation to table creation statements --- config/global.ini.php | 5 ++- core/Db/Schema/Mysql.php | 10 +++++ core/Db/Schema/Tidb.php | 9 +++- core/Db/Settings.php | 5 +++ .../Integration/Db/Schema/MariadbTest.php | 41 +++++++++++++++++++ .../Integration/Db/Schema/MysqlTest.php | 41 +++++++++++++++++++ .../Integration/Db/Schema/TidbTest.php | 41 +++++++++++++++++++ 7 files changed, 148 insertions(+), 4 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index d683536ce53..b8225832e2a 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -45,11 +45,12 @@ ; Matomo should work correctly without this setting but we recommend to have a charset set. charset = utf8 -; In some database setups the collation for the connection changes after a "SET NAMES" statement -; is issued, unless a "COLLATE" argument is added. +; In some database setups the collation used for queries and creating tables can have unexpected +; values, or change after a database version upgrade. ; If you encounter "Illegal mix of collation" errors, setting this config to the value matching ; your existing database tables can help. ; This setting will only be used if "charset" is also set. +; Matomo should work correctly without this setting but we recommend to have a collation set. collation = ; Database error codes to ignore during updates diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index 47c05756289..f5b24b4da39 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -678,10 +678,15 @@ public function getTableCreateOptions(): string { $engine = $this->getTableEngine(); $charset = $this->getUsedCharset(); + $collation = $this->getUsedCollation(); $rowFormat = $this->getTableRowFormat(); $options = "ENGINE=$engine DEFAULT CHARSET=$charset"; + if ('' !== $collation) { + $options .= " COLLATE=$collation"; + } + if ('' !== $rowFormat) { $options .= " $rowFormat"; } @@ -784,6 +789,11 @@ protected function getUsedCharset(): string return $this->getDbSettings()->getUsedCharset(); } + protected function getUsedCollation(): string + { + return $this->getDbSettings()->getUsedCollation(); + } + private function getTablePrefix() { return $this->getDbSettings()->getTablePrefix(); diff --git a/core/Db/Schema/Tidb.php b/core/Db/Schema/Tidb.php index 732b44f9315..cf020d71056 100644 --- a/core/Db/Schema/Tidb.php +++ b/core/Db/Schema/Tidb.php @@ -36,12 +36,17 @@ public function getTableCreateOptions(): string { $engine = $this->getTableEngine(); $charset = $this->getUsedCharset(); + $collation = $this->getUsedCollation(); $rowFormat = $this->getTableRowFormat(); + if ('utf8mb4' === $charset && '' === $collation) { + $collation = 'utf8mb4_0900_ai_ci'; + } + $options = "ENGINE=$engine DEFAULT CHARSET=$charset"; - if ('utf8mb4' === $charset) { - $options .= ' COLLATE=utf8mb4_0900_ai_ci'; + if ('' !== $collation) { + $options .= " COLLATE=$collation"; } if ('' !== $rowFormat) { diff --git a/core/Db/Settings.php b/core/Db/Settings.php index c9707fcce8c..e18a9dab8b6 100644 --- a/core/Db/Settings.php +++ b/core/Db/Settings.php @@ -33,6 +33,11 @@ public function getUsedCharset() return strtolower($this->getDbSetting('charset')); } + public function getUsedCollation() + { + return strtolower($this->getDbSetting('collation') ?? ''); + } + public function getRowFormat() { return $this->getUsedCharset() === 'utf8mb4' ? 'ROW_FORMAT=DYNAMIC' : ''; diff --git a/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php b/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php index d5e1fe92a63..78b230331a3 100644 --- a/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php @@ -68,4 +68,45 @@ public function testOptimize() // should optimize when forced $this->assertTrue($schema->optimizeTables(['table3', 'table4'], true), true); } + + /** + * @dataProvider getTableCreateOptionsTestData + */ + public function testTableCreateOptions(array $optionOverrides, string $expected): void + { + if (DatabaseConfig::getConfigValue('schema') !== 'Mariadb') { + self::markTestSkipped('Mariadb is not available'); + } + + foreach ($optionOverrides as $name => $value) { + DatabaseConfig::setConfigValue($name, $value); + } + + $schema = Db\Schema::getInstance(); + + self::assertSame($expected, $schema->getTableCreateOptions()); + } + + public function getTableCreateOptionsTestData(): iterable + { + yield 'defaults' => [ + [], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC' + ]; + + yield 'override charset' => [ + ['charset' => 'utf8mb3'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3' + ]; + + yield 'override collation' => [ + ['collation' => 'utf8mb4_general_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC' + ]; + + yield 'override charset and collation' => [ + ['charset' => 'utf8mb3', 'collation' => 'utf8mb3_general_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_general_ci' + ]; + } } diff --git a/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php b/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php index 69094eb4386..b1c455ff93b 100644 --- a/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php @@ -68,4 +68,45 @@ public function testOptimize() // should optimize when forced $this->assertTrue($schema->optimizeTables(['table3', 'table4'], true)); } + + /** + * @dataProvider getTableCreateOptionsTestData + */ + public function testTableCreateOptions(array $optionOverrides, string $expected): void + { + if (DatabaseConfig::getConfigValue('schema') !== 'Mysql') { + self::markTestSkipped('Mysql is not available'); + } + + foreach ($optionOverrides as $name => $value) { + DatabaseConfig::setConfigValue($name, $value); + } + + $schema = Db\Schema::getInstance(); + + self::assertSame($expected, $schema->getTableCreateOptions()); + } + + public function getTableCreateOptionsTestData(): iterable + { + yield 'defaults' => [ + [], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC' + ]; + + yield 'override charset' => [ + ['charset' => 'utf8mb3'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3' + ]; + + yield 'override collation' => [ + ['collation' => 'utf8mb4_general_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC' + ]; + + yield 'override charset and collation' => [ + ['charset' => 'utf8mb3', 'collation' => 'utf8mb3_general_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_general_ci' + ]; + } } diff --git a/tests/PHPUnit/Integration/Db/Schema/TidbTest.php b/tests/PHPUnit/Integration/Db/Schema/TidbTest.php index 3ed40f9039a..9ec52106479 100644 --- a/tests/PHPUnit/Integration/Db/Schema/TidbTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/TidbTest.php @@ -43,4 +43,45 @@ public function testOptimize() $this->assertFalse($schema->optimizeTables(['table3', 'table4'])); $this->assertFalse($schema->optimizeTables(['table3', 'table4'], true)); } + + /** + * @dataProvider getTableCreateOptionsTestData + */ + public function testTableCreateOptions(array $optionOverrides, string $expected): void + { + if (DatabaseConfig::getConfigValue('schema') !== 'Tidb') { + self::markTestSkipped('Tidb is not available'); + } + + foreach ($optionOverrides as $name => $value) { + DatabaseConfig::setConfigValue($name, $value); + } + + $schema = Db\Schema::getInstance(); + + self::assertSame($expected, $schema->getTableCreateOptions()); + } + + public function getTableCreateOptionsTestData(): iterable + { + yield 'defaults' => [ + [], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci ROW_FORMAT=DYNAMIC' + ]; + + yield 'override charset' => [ + ['charset' => 'utf8mb3'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb4_0900_ai_ci' + ]; + + yield 'override collation' => [ + ['collation' => 'utf8mb4_general_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC' + ]; + + yield 'override charset and collation' => [ + ['charset' => 'utf8mb3', 'collation' => 'utf8mb3_general_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_general_ci' + ]; + } } From 6ed33c3f8d93a4c597bafb0808d99aca6f9842ca Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Tue, 10 Sep 2024 16:33:27 +0200 Subject: [PATCH 04/10] Detect default collation to be used during database creation --- core/Db/Schema.php | 11 ++++++++ core/Db/Schema/Mysql.php | 28 ++++++++++++++++++- core/Db/Schema/Tidb.php | 26 ++++++++--------- core/Db/SchemaInterface.php | 9 ++++++ core/DbHelper.php | 15 +++++++++- .../Integration/Db/Schema/TidbTest.php | 14 ++++++++++ tests/PHPUnit/Integration/DbHelperTest.php | 19 +++++++++++++ 7 files changed, 106 insertions(+), 16 deletions(-) diff --git a/core/Db/Schema.php b/core/Db/Schema.php index 296fb2ea92b..36a76b5434f 100644 --- a/core/Db/Schema.php +++ b/core/Db/Schema.php @@ -89,6 +89,17 @@ private function getSchema(): SchemaInterface return $this->schema; } + /** + * Returns the default collation for a charset. + * + * @param string $charset + * @return string + */ + public function getDefaultCollationForCharset(string $charset): string + { + return $this->getSchema()->getDefaultCollationForCharset($charset); + } + /** * Get the table options to use for a CREATE TABLE statement. * diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index f5b24b4da39..44b66fc4ddb 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -669,6 +669,31 @@ public function supportsComplexColumnUpdates(): bool return true; } + /** + * Returns the default collation for a charset. + * + * @param string $charset + * + * @return string + * @throws Exception + */ + public function getDefaultCollationForCharset(string $charset): string + { + $result = $this->getDb()->fetchRow( + 'SHOW COLLATION WHERE `Default` = "Yes" AND `Charset` = ?', + [$charset] + ); + + if (!isset($result['Collation'])) { + throw new Exception(sprintf( + 'Failed to detect default collation for character set "%s"', + $charset + )); + } + + return $result['Collation']; + } + public function getDefaultPort(): int { return 3306; @@ -770,8 +795,9 @@ public function getSupportedReadIsolationTransactionLevel(): string protected function getDatabaseCreateOptions(): string { $charset = DbHelper::getDefaultCharset(); + $collation = $this->getDefaultCollationForCharset($charset); - return "DEFAULT CHARACTER SET $charset"; + return "DEFAULT CHARACTER SET $charset COLLATE $collation"; } protected function getTableEngine() diff --git a/core/Db/Schema/Tidb.php b/core/Db/Schema/Tidb.php index cf020d71056..cdf9b673f7b 100644 --- a/core/Db/Schema/Tidb.php +++ b/core/Db/Schema/Tidb.php @@ -9,8 +9,6 @@ namespace Piwik\Db\Schema; -use Piwik\DbHelper; - /** * Mariadb schema */ @@ -27,6 +25,18 @@ public function supportsComplexColumnUpdates(): bool return false; } + public function getDefaultCollationForCharset(string $charset): string + { + $collation = parent::getDefaultCollationForCharset($charset); + + if ('utf8mb4' === $charset && 'utf8mb4_bin' === $collation) { + // replace the TiDB default "utf8mb4_bin" with a better default + return 'utf8mb4_0900_ai_ci'; + } + + return $collation; + } + public function getDefaultPort(): int { return 4000; @@ -78,16 +88,4 @@ public function getSupportedReadIsolationTransactionLevel(): string // TiDB doesn't support READ UNCOMMITTED return 'READ COMMITTED'; } - - protected function getDatabaseCreateOptions(): string - { - $charset = DbHelper::getDefaultCharset(); - $options = "DEFAULT CHARACTER SET $charset"; - - if ('utf8mb4' === $charset) { - $options .= ' COLLATE=utf8mb4_0900_ai_ci'; - } - - return $options; - } } diff --git a/core/Db/SchemaInterface.php b/core/Db/SchemaInterface.php index 3a86e9b74af..66a4aac203c 100644 --- a/core/Db/SchemaInterface.php +++ b/core/Db/SchemaInterface.php @@ -135,6 +135,15 @@ public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): strin */ public function supportsComplexColumnUpdates(): bool; + /** + * Returns the default collation for a charset used by this database engine. + * + * @param string $charset + * + * @return string + */ + public function getDefaultCollationForCharset(string $charset): string; + /** * Return the default port used by this database engine * diff --git a/core/DbHelper.php b/core/DbHelper.php index 25aecfa99c6..fad8a0af1a5 100644 --- a/core/DbHelper.php +++ b/core/DbHelper.php @@ -210,7 +210,7 @@ public static function tableHasIndex($table, $indexName) * @return string * @throws Tracker\Db\DbException */ - public static function getDefaultCharset() + public static function getDefaultCharset(): string { $result = Db::get()->fetchRow("SHOW CHARACTER SET LIKE 'utf8mb4'"); @@ -233,6 +233,19 @@ public static function getDefaultCharset() return 'utf8mb4'; } + /** + * Returns the default collation for a charset. + * + * @param string $charset + * + * @return string + * @throws Exception + */ + public static function getDefaultCollationForCharset(string $charset): string + { + return Schema::getInstance()->getDefaultCollationForCharset($charset); + } + /** * Returns sql queries to convert all installed tables to utf8mb4 * diff --git a/tests/PHPUnit/Integration/Db/Schema/TidbTest.php b/tests/PHPUnit/Integration/Db/Schema/TidbTest.php index 9ec52106479..dff8133a388 100644 --- a/tests/PHPUnit/Integration/Db/Schema/TidbTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/TidbTest.php @@ -44,6 +44,20 @@ public function testOptimize() $this->assertFalse($schema->optimizeTables(['table3', 'table4'], true)); } + public function testGetDefaultCollationForCharsetReplacesUtf8mb4Binary(): void + { + if (DatabaseConfig::getConfigValue('schema') !== 'Tidb') { + self::markTestSkipped('Tidb is not available'); + } + + $schema = Db\Schema::getInstance(); + + self::assertSame( + 'utf8mb4_0900_ai_ci', + $schema->getDefaultCollationForCharset('utf8mb4') + ); + } + /** * @dataProvider getTableCreateOptionsTestData */ diff --git a/tests/PHPUnit/Integration/DbHelperTest.php b/tests/PHPUnit/Integration/DbHelperTest.php index d3f88c76a53..de20af974db 100644 --- a/tests/PHPUnit/Integration/DbHelperTest.php +++ b/tests/PHPUnit/Integration/DbHelperTest.php @@ -119,6 +119,25 @@ public function testAddOriginHintToQuery() self::assertEquals($expected, $result); } + public function testGetDefaultCollationForCharset(): void + { + $charset = 'utf8mb4'; + $collation = DbHelper::getDefaultCollationForCharset($charset); + $expectedPrefix = $charset . '_'; + + // exact collation depends on the database used + // but should always start with the charset + self::assertStringStartsWith($expectedPrefix, $collation); + } + + public function testGetDefaultCollationForCharsetThrowsForInvalidCharset(): void + { + self::expectException(\Exception::class); + self::expectExceptionMessage('Failed to detect default collation for character set "invalid"'); + + DbHelper::getDefaultCollationForCharset('invalid'); + } + private function assertDbExists($dbName) { $dbs = Db::fetchAll("SHOW DATABASES"); From e7f5bc498a9b86ca6dcf61c04aeeb41a7c670351 Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Tue, 10 Sep 2024 16:35:02 +0200 Subject: [PATCH 05/10] Save default database collation to config during installation --- plugins/Installation/Controller.php | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/Installation/Controller.php b/plugins/Installation/Controller.php index d7d40e5f277..151494ccb91 100644 --- a/plugins/Installation/Controller.php +++ b/plugins/Installation/Controller.php @@ -609,6 +609,7 @@ private function createConfigFile($dbInfos) $config->database = $dbInfos; $config->database['charset'] = DbHelper::getDefaultCharset(); + $config->database['collation'] = DbHelper::getDefaultCollationForCharset($config->database['charset']); $config->forceSave(); From e688f0de88d78a1f6bf465e4164377b928e07e2c Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Wed, 11 Sep 2024 20:51:51 +0200 Subject: [PATCH 06/10] Update database collection config if auto-detectable --- core/Updates/5.2.0-b2.php | 70 +++++++++++++++++++++++++++++++++++++++ core/Version.php | 2 +- 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 core/Updates/5.2.0-b2.php diff --git a/core/Updates/5.2.0-b2.php b/core/Updates/5.2.0-b2.php new file mode 100644 index 00000000000..faab0e9eba0 --- /dev/null +++ b/core/Updates/5.2.0-b2.php @@ -0,0 +1,70 @@ +migration = $factory; + } + + public function getMigrations(Updater $updater) + { + $migrations = []; + + $config = Config::getInstance(); + $dbConfig = $config->database; + + // only run migration if config is not set + if (empty($dbConfig['collation'])) { + try { + $db = Db::get(); + $userTable = Common::prefixTable('user'); + $userTableStatus = $db->fetchRow('SHOW TABLE STATUS WHERE Name = ?', [$userTable]); + $connectionCollation = $db->fetchOne('SELECT @@collation_connection'); + + // only update config if user table and connection report same collation + if ( + !empty($userTableStatus['Collation']) + && !empty($connectionCollation) + && $userTableStatus['Collation'] === $connectionCollation + ) { + $migrations[] = $this->migration->config->set( + 'database', + 'collation', + $connectionCollation + ); + } + } catch (\Exception $e) { + // rely on the system check if detection failed + } + } + + return $migrations; + } + + public function doUpdate(Updater $updater) + { + $updater->executeMigrations(__FILE__, $this->getMigrations($updater)); + } +} diff --git a/core/Version.php b/core/Version.php index e2d3f54a45b..78e14b60fc4 100644 --- a/core/Version.php +++ b/core/Version.php @@ -22,7 +22,7 @@ final class Version * The current Matomo version. * @var string */ - public const VERSION = '5.2.0-b1'; + public const VERSION = '5.2.0-b2'; public const MAJOR_VERSION = 5; From 22391527c81359bea5aec32c3e25b98c18d9102a Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Wed, 11 Sep 2024 21:21:22 +0200 Subject: [PATCH 07/10] Add database collation to diagnostics --- .../Diagnostic/DatabaseAbilitiesCheck.php | 24 +++++++++++++++++++ .../Diagnostic/DatabaseInformational.php | 3 ++- plugins/Diagnostics/lang/en.json | 3 +++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/plugins/Diagnostics/Diagnostic/DatabaseAbilitiesCheck.php b/plugins/Diagnostics/Diagnostic/DatabaseAbilitiesCheck.php index e764f1fe063..6465633b278 100644 --- a/plugins/Diagnostics/Diagnostic/DatabaseAbilitiesCheck.php +++ b/plugins/Diagnostics/Diagnostic/DatabaseAbilitiesCheck.php @@ -42,6 +42,7 @@ public function execute() $result = new DiagnosticResult($this->translator->translate('Installation_DatabaseAbilities')); $result->addItem($this->checkUtf8mb4Charset()); + $result->addItem($this->checkCollation()); if (Config::getInstance()->General['enable_load_data_infile']) { $result->addItem($this->checkLoadDataInfile()); @@ -92,6 +93,29 @@ protected function checkUtf8mb4Charset() ); } + protected function checkCollation(): DiagnosticResultItem + { + $dbSettings = new Db\Settings(); + $collation = $dbSettings->getUsedCollation(); + + if ('' !== $collation) { + return new DiagnosticResultItem(DiagnosticResult::STATUS_OK, 'Connection collation'); + } + + $collationConnection = Db::get()->fetchOne('SELECT @@collation_connection'); + $collationCharset = DbHelper::getDefaultCollationForCharset($dbSettings->getUsedCharset()); + + return new DiagnosticResultItem( + DiagnosticResult::STATUS_WARNING, + sprintf( + 'Connection collation

%s

%s
%s
', + $this->translator->translate('Diagnostics_DatabaseCollationNotConfigured'), + $this->translator->translate('Diagnostics_DatabaseCollationConnection', [$collationConnection]), + $this->translator->translate('Diagnostics_DatabaseCollationCharset', [$collationCharset]) + ) + ); + } + protected function checkLoadDataInfile() { $optionTable = Common::prefixTable('option'); diff --git a/plugins/Diagnostics/Diagnostic/DatabaseInformational.php b/plugins/Diagnostics/Diagnostic/DatabaseInformational.php index 1d2dfc81032..1dd4eb6c522 100644 --- a/plugins/Diagnostics/Diagnostic/DatabaseInformational.php +++ b/plugins/Diagnostics/Diagnostic/DatabaseInformational.php @@ -16,7 +16,7 @@ use Piwik\Translation\Translator; /** - * Informatation about the database. + * Information about the database. */ class DatabaseInformational implements Diagnostic { @@ -38,6 +38,7 @@ public function execute() $dbConfig = Config::getInstance()->database; $results[] = DiagnosticResult::informationalResult('DB Prefix', $dbConfig['tables_prefix']); $results[] = DiagnosticResult::informationalResult('DB Charset', $dbConfig['charset']); + $results[] = DiagnosticResult::informationalResult('DB Collation', $dbConfig['collation']); $results[] = DiagnosticResult::informationalResult('DB Adapter', $dbConfig['adapter']); $results[] = DiagnosticResult::informationalResult('MySQL Version', $this->getServerVersion()); $results[] = DiagnosticResult::informationalResult('Num Tables', $this->getNumMatomoTables()); diff --git a/plugins/Diagnostics/lang/en.json b/plugins/Diagnostics/lang/en.json index ee0a651ef8c..70855e531a4 100644 --- a/plugins/Diagnostics/lang/en.json +++ b/plugins/Diagnostics/lang/en.json @@ -12,6 +12,9 @@ "Sections": "Sections", "BrowserAndAutoArchivingEnabledLabel": "Browser and Auto-archiving enabled", "BrowserAndAutoArchivingEnabledComment": "It looks like both browser and auto archiving are enabled. Auto archiving last started %3$s ago. If %1$sauto archiving%2$s is enabled, you should disable browser archiving in \"General Settings\".", + "DatabaseCollationNotConfigured": "You database connection is configured without an explicit collation. Please update [database] collation = '' in the \"config/config.ini.php\" file with the collation to be used, to ensure all database features work as expected.", + "DatabaseCollationConnection": "Your currently used connection collation is: %1$s", + "DatabaseCollationCharset": "The default collation for your configured charset is: %1$s", "DatabaseReaderConnection": "Database Reader Connection", "DatabaseUtf8Requirement": "This is required to be able to store 4-byte UTF8 characters. Unless utf8mb4 is available special characters, such as emojis, less common characters of asian languages, various historic scripts or mathematical symbols will be replaced with %1$s. You can read more details about this topic in %2$sthis FAQ%3$s.", "DatabaseUtf8mb4CharsetRecommended": "Your database doesn't support utf8mb4 charset yet.", From 0e25f58083c948c9e5d28ab7b2351996ecef95b2 Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Thu, 12 Sep 2024 10:49:28 +0200 Subject: [PATCH 08/10] Configure default collation for test environment --- config/global.ini.php | 1 + .../Integration/Db/Schema/MariadbTest.php | 14 +++++++------- .../PHPUnit/Integration/Db/Schema/MysqlTest.php | 14 +++++++------- tests/PHPUnit/Integration/Db/Schema/TidbTest.php | 16 ++++++++-------- .../Integration/Tracker/Db/MysqliTest.php | 1 + 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index b8225832e2a..0e9cf1d4c97 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -92,6 +92,7 @@ type = InnoDB schema = Mysql charset = utf8mb4 +collation = utf8mb4_general_ci enable_ssl = 0 ssl_ca = ssl_cert = diff --git a/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php b/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php index 78b230331a3..65c68ceeece 100644 --- a/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/MariadbTest.php @@ -89,19 +89,19 @@ public function testTableCreateOptions(array $optionOverrides, string $expected) public function getTableCreateOptionsTestData(): iterable { - yield 'defaults' => [ - [], + yield 'default charset, empty collation' => [ + ['collation' => ''], 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC' ]; - yield 'override charset' => [ - ['charset' => 'utf8mb3'], + yield 'override charset, empty collation' => [ + ['charset' => 'utf8mb3', 'collation' => ''], 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3' ]; - yield 'override collation' => [ - ['collation' => 'utf8mb4_general_ci'], - 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC' + yield 'default charset, override collation' => [ + ['collation' => 'utf8mb4_swedish_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_swedish_ci ROW_FORMAT=DYNAMIC' ]; yield 'override charset and collation' => [ diff --git a/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php b/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php index b1c455ff93b..7a2fd9eeaa3 100644 --- a/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/MysqlTest.php @@ -89,19 +89,19 @@ public function testTableCreateOptions(array $optionOverrides, string $expected) public function getTableCreateOptionsTestData(): iterable { - yield 'defaults' => [ - [], + yield 'default charset, empty collation' => [ + ['collation' => ''], 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC' ]; - yield 'override charset' => [ - ['charset' => 'utf8mb3'], + yield 'override charset, empty collation' => [ + ['charset' => 'utf8mb3', 'collation' => ''], 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3' ]; - yield 'override collation' => [ - ['collation' => 'utf8mb4_general_ci'], - 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC' + yield 'default charset, override collation' => [ + ['collation' => 'utf8mb4_swedish_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_swedish_ci ROW_FORMAT=DYNAMIC' ]; yield 'override charset and collation' => [ diff --git a/tests/PHPUnit/Integration/Db/Schema/TidbTest.php b/tests/PHPUnit/Integration/Db/Schema/TidbTest.php index dff8133a388..50a3989aa01 100644 --- a/tests/PHPUnit/Integration/Db/Schema/TidbTest.php +++ b/tests/PHPUnit/Integration/Db/Schema/TidbTest.php @@ -78,19 +78,19 @@ public function testTableCreateOptions(array $optionOverrides, string $expected) public function getTableCreateOptionsTestData(): iterable { - yield 'defaults' => [ - [], + yield 'default charset, empty collation' => [ + ['collation' => ''], 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci ROW_FORMAT=DYNAMIC' ]; - yield 'override charset' => [ - ['charset' => 'utf8mb3'], - 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb4_0900_ai_ci' + yield 'override charset, empty collation' => [ + ['charset' => 'utf8mb3', 'collation' => ''], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb3' ]; - yield 'override collation' => [ - ['collation' => 'utf8mb4_general_ci'], - 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC' + yield 'default charset, override collation' => [ + ['collation' => 'utf8mb4_swedish_ci'], + 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_swedish_ci ROW_FORMAT=DYNAMIC' ]; yield 'override charset and collation' => [ diff --git a/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php b/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php index d6359887c7d..eed5f2baa68 100644 --- a/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php +++ b/tests/PHPUnit/Integration/Tracker/Db/MysqliTest.php @@ -35,6 +35,7 @@ public function testConnectionThrowsOnInvalidCharset(): void self::expectExceptionMessageMatches('/Set Charset failed/'); $config = Config::getInstance(); + $config->database['collation'] = null; $config->database['charset'] = 'something really invalid'; Tracker\Db::connectPiwikTrackerDb(); From d203ce8d8dff53546b4467da8b131f61b34f9592 Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Thu, 12 Sep 2024 13:26:04 +0200 Subject: [PATCH 09/10] Update expected screenshots --- .../tests/UI/expected-screenshots/Diagnostics_page.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png b/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png index 6c3fc1c22bc..132a013ed7e 100644 --- a/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png +++ b/plugins/Diagnostics/tests/UI/expected-screenshots/Diagnostics_page.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:91515f6ddd2de18326ce12e3880916607412a2c90e5cfcfa986464aa2bdcfd75 -size 446937 +oid sha256:f0e3c3c056d20d74b0cad5ef85faaef42de54945e96d8c0b568c02613c86391e +size 452383 From 104c2dd343107d8bc6d6e2eef3af9d59547d2b00 Mon Sep 17 00:00:00 2001 From: Marc Neudert Date: Thu, 12 Sep 2024 18:29:09 +0200 Subject: [PATCH 10/10] Look at most recent archive table for update collation detection --- core/Updates/5.2.0-b2.php | 78 +++++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 20 deletions(-) diff --git a/core/Updates/5.2.0-b2.php b/core/Updates/5.2.0-b2.php index faab0e9eba0..8514829bc0b 100644 --- a/core/Updates/5.2.0-b2.php +++ b/core/Updates/5.2.0-b2.php @@ -11,6 +11,7 @@ use Piwik\Common; use Piwik\Config; +use Piwik\DataAccess\ArchiveTableCreator; use Piwik\Db; use Piwik\Updater; use Piwik\Updater\Migration\Factory as MigrationFactory; @@ -37,26 +38,14 @@ public function getMigrations(Updater $updater) // only run migration if config is not set if (empty($dbConfig['collation'])) { - try { - $db = Db::get(); - $userTable = Common::prefixTable('user'); - $userTableStatus = $db->fetchRow('SHOW TABLE STATUS WHERE Name = ?', [$userTable]); - $connectionCollation = $db->fetchOne('SELECT @@collation_connection'); - - // only update config if user table and connection report same collation - if ( - !empty($userTableStatus['Collation']) - && !empty($connectionCollation) - && $userTableStatus['Collation'] === $connectionCollation - ) { - $migrations[] = $this->migration->config->set( - 'database', - 'collation', - $connectionCollation - ); - } - } catch (\Exception $e) { - // rely on the system check if detection failed + $collation = $this->detectCollationForMigration(); + + if (null !== $collation) { + $migrations[] = $this->migration->config->set( + 'database', + 'collation', + $collation + ); } } @@ -67,4 +56,53 @@ public function doUpdate(Updater $updater) { $updater->executeMigrations(__FILE__, $this->getMigrations($updater)); } + + private function detectCollationForMigration(): ?string + { + try { + $db = Db::get(); + $userTable = Common::prefixTable('user'); + $userTableStatus = $db->fetchRow('SHOW TABLE STATUS WHERE Name = ?', [$userTable]); + + if (empty($userTableStatus['Collation'])) { + // if there is no user table, or no collation for it, abort detection + // this table should always exist and something must be wrong in this case + return null; + } + + $userTableCollation = $userTableStatus['Collation']; + $connectionCollation = $db->fetchOne('SELECT @@collation_connection'); + + if ($userTableCollation === $connectionCollation) { + // if the connection is matching the user table + // we should be safe to assume we have already found a config value + return $userTableCollation; + } + + $archiveTables = ArchiveTableCreator::getTablesArchivesInstalled(ArchiveTableCreator::NUMERIC_TABLE); + + if (0 === count($archiveTables)) { + // skip if there is no archive table (yet) + return null; + } + + // sort tables so we have them in order of their date + rsort($archiveTables); + + $archiveTableStatus = $db->fetchRow('SHOW TABLE STATUS WHERE Name = ?', [$archiveTables[0]]); + + if ( + !empty($archiveTableStatus['Collation']) + && $archiveTableStatus['Collation'] === $userTableCollation + ) { + // the most recent numeric archive table is matching the collation + // of the users table, should be a good config value to choose + return $userTableCollation; + } + } catch (\Exception $e) { + // rely on the system check if detection failed + } + + return null; + } }