[POC] Run tests with TiDb

.github/workflows/matomo-tests.yml

@@ -38,7 +38,7 @@ jobs:
       matrix:
         type: [ 'UnitTests', 'SystemTestsPlugins', 'SystemTestsCore', 'IntegrationTestsCore', 'IntegrationTestsPlugins' ]
         php: [ '7.2', '8.2', '8.3' ]
-        engine: [ 'Mysql', 'Mariadb' ]
+        engine: [ 'Tidb' ]
         adapter: [ 'PDO_MYSQL', 'MYSQLI' ]
         exclude:
           - php: '7.2'
@@ -124,6 +124,7 @@ jobs:
           test-type: 'UI'
           php-version: '7.2'
           node-version: '16'
+          mysql-engine: 'Tidb'
           redis-service: true
           artifacts-pass: ${{ secrets.ARTIFACTS_PASS }}
           upload-artifacts: true

CHANGELOG.md

@@ -10,6 +10,10 @@ The Product Changelog at **[matomo.org/changelog](https://matomo.org/changelog)*
 
 * The dependency `jQuery.dotdotdot` has been removed. Please use pure CSS instead or include the library in your plugin if needed.
 
+## Deprecations
+
+The methods `Db::isOptimizeInnoDBSupported` and `Db::optimizeTables` have been deprecated. Use `Db\Schema::getInstance()->isOptimizeInnoDBSupported` and `Db\Schema::getInstance()->optimizeTables` instead
+
 ## Matomo 5.1.0
 
 ### Breaking Changes

config/global.ini.php

@@ -770,7 +770,7 @@
 
 ; In some rare cases it may be useful to explicitly tell Matomo not to use LOAD DATA INFILE
 ; This may for example be useful when doing Mysql AWS replication
-enable_load_data_infile = 1
+enable_load_data_infile = 0
 
 ; By setting this option to 0:
 ; - links to Enable/Disable/Uninstall plugins will be hidden and disabled

core/DataAccess/LogAggregator.php

@@ -1236,9 +1236,9 @@ public function queryConversionsByPageView(string $linkField, int $idGoal)
             " . sprintf("ROUND(SUM(1 / log_conversion.pageviews_before * log_conversion.revenue_tax),2) AS `%d`,", Metrics::INDEX_GOAL_ECOMMERCE_REVENUE_TAX) . "
             " . sprintf("ROUND(SUM(1 / log_conversion.pageviews_before * log_conversion.revenue_shipping),2) AS `%d`,", Metrics::INDEX_GOAL_ECOMMERCE_REVENUE_SHIPPING) . "
             " . sprintf("ROUND(SUM(1 / log_conversion.pageviews_before * log_conversion.revenue_discount),2) AS `%d`,", Metrics::INDEX_GOAL_ECOMMERCE_REVENUE_DISCOUNT) . "
-            " . sprintf("SUM(1 / log_conversion.pageviews_before * log_conversion.items) AS `%d`,", Metrics::INDEX_GOAL_ECOMMERCE_ITEMS) . "
+            " . sprintf("SUM(ROUND(1 / log_conversion.pageviews_before * log_conversion.items, 4)) AS `%d`,", Metrics::INDEX_GOAL_ECOMMERCE_ITEMS) . "
             " . sprintf("log_conversion.pageviews_before AS `%d`,", Metrics::INDEX_GOAL_NB_PAGES_UNIQ_BEFORE) . "
-            " . sprintf("SUM(1 / log_conversion.pageviews_before) AS `%d`,", Metrics::INDEX_GOAL_NB_CONVERSIONS_ATTRIB) . "
+            " . sprintf("SUM(ROUND(1 / log_conversion.pageviews_before, 4)) AS `%d`,", Metrics::INDEX_GOAL_NB_CONVERSIONS_ATTRIB) . "
             " . sprintf("COUNT(*) AS `%d`,", Metrics::INDEX_GOAL_NB_CONVERSIONS_PAGE_UNIQ) . "
             " . sprintf("ROUND(SUM(1 / log_conversion.pageviews_before * log_conversion.revenue),2) AS `%d`", Metrics::INDEX_GOAL_REVENUE_ATTRIB);
 

core/DataAccess/Model.php

@@ -62,7 +62,7 @@ public function getInvalidatedArchiveIdsSafeToDelete($archiveTable, $setGroupCon
         }
 
         $sql = "SELECT idsite, date1, date2, period, name,
-                       GROUP_CONCAT(idarchive, '.', value ORDER BY ts_archived DESC) as archives
+                       GROUP_CONCAT(idarchive, '.', value ORDER BY ts_archived DESC, idarchive DESC) as archives
                   FROM `$archiveTable`
                  WHERE name LIKE 'done%'
                    AND `value` NOT IN (" . ArchiveWriter::DONE_ERROR . ")

core/DataTable/Filter/ReplaceColumnNames.php

@@ -162,6 +162,8 @@ protected function getRenamedColumns($columns)
     protected function flattenGoalColumns($columnValue)
     {
         $newSubColumns = array();
+        // sort by key (idgoal) to ensure a static result
+        ksort($columnValue);
         foreach ($columnValue as $idGoal => $goalValues) {
             $mapping = Metrics::$mappingFromIdToNameGoal;
             if ($idGoal == GoalManager::IDGOAL_CART) {

core/DataTable/Row.php

@@ -521,7 +521,13 @@ private function getColumnValuesMerged($operation, $thisColumnValue, $columnToSu
                 $newValue = null;
                 break;
             case 'max':
-                $newValue = max($thisColumnValue, $columnToSumValue);
+                if (!$thisColumnValue) {
+                    $newValue = $columnToSumValue;
+                } elseif (!$columnToSumValue) {
+                    $newValue = $thisColumnValue;
+                } else {
+                    $newValue = max($thisColumnValue, $columnToSumValue);
+                }
                 break;
             case 'min':
                 if (!$thisColumnValue) {

core/Db.php

@@ -11,6 +11,7 @@
 
 use Exception;
 use Piwik\Db\Adapter;
+use Piwik\Db\Schema;
 
 /**
  * Contains SQL related helper functions for Piwik's MySQL database.
@@ -466,63 +467,13 @@ public static function deleteAllRows($table, $where, $orderBy, $maxRowsPerQuery
      *                             Table names must be prefixed (see {@link Piwik\Common::prefixTable()}).
      * @param bool $force If true, the `OPTIMIZE TABLE` query will be run even if InnoDB tables are being used.
      * @return bool
+     * @deprecated  will be removed in Matomo 6
+     *              use Schema::getInstance()->optimizeTables() instead
      */
     public static function optimizeTables($tables, $force = false)
     {
-        $optimize = Config::getInstance()->General['enable_sql_optimize_queries'];
-
-        if (
-            empty($optimize)
-            && !$force
-        ) {
-            return false;
-        }
-
-        if (empty($tables)) {
-            return false;
-        }
-
-        if (!is_array($tables)) {
-            $tables = array($tables);
-        }
-
-        if (
-            !self::isOptimizeInnoDBSupported()
-            && !$force
-        ) {
-            // filter out all InnoDB tables
-            $myisamDbTables = array();
-            foreach (self::getTableStatus() as $row) {
-                if (
-                    strtolower($row['Engine']) == 'myisam'
-                    && in_array($row['Name'], $tables)
-                ) {
-                    $myisamDbTables[] = $row['Name'];
-                }
-            }
-
-            $tables = $myisamDbTables;
-        }
-
-        if (empty($tables)) {
-            return false;
-        }
-
-        // optimize the tables
-        $success = true;
-        foreach ($tables as &$t) {
-            $ok = self::query('OPTIMIZE TABLE ' . $t);
-            if (!$ok) {
-                $success = false;
-            }
-        }
-
-        return $success;
-    }
-
-    private static function getTableStatus()
-    {
-        return Db::fetchAll("SHOW TABLE STATUS");
+        $tables = !is_array($tables) ? [$tables] : $tables;
+        return Schema::getInstance()->optimizeTables($tables, (bool) $force);
     }
 
     /**
@@ -912,19 +863,12 @@ public static function isQueryLogEnabled()
         return self::$logQueries;
     }
 
+    /**
+     * @deprecated will be removed with Matomo 6
+     *             use Schema::getInstance()->isOptimizeInnoDBSupported() instead
+     */
     public static function isOptimizeInnoDBSupported($version = null)
     {
-        if ($version === null) {
-            $version = Db::fetchOne("SELECT VERSION()");
-        }
-
-        $version = strtolower($version);
-
-        if (strpos($version, "mariadb") === false) {
-            return false;
-        }
-
-        $semanticVersion = strstr($version, '-', $beforeNeedle = true);
-        return version_compare($semanticVersion, '10.1.1', '>=');
+        return Db\Schema::getInstance()->isOptimizeInnoDBSupported();
     }
 }

core/Db/Schema.php

@@ -254,4 +254,30 @@ public function supportsComplexColumnUpdates(): bool
     {
         return $this->getSchema()->supportsComplexColumnUpdates();
     }
+
+    /**
+     * Returns if the schema supports `OPTIMIZE TABLE` statements for innodb tables
+     *
+     * @return bool
+     */
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        return $this->getSchema()->isOptimizeInnoDBSupported();
+    }
+
+    /**
+     * Runs an `OPTIMIZE TABLE` query on the supplied table or tables.
+     *
+     * Tables will only be optimized if the `[General] enable_sql_optimize_queries` INI config option is
+     * set to **1**.
+     *
+     * @param string|array $tables The name of the table to optimize or an array of tables to optimize.
+     *                             Table names must be prefixed (see {@link Piwik\Common::prefixTable()}).
+     * @param bool $force If true, the `OPTIMIZE TABLE` query will be run even if InnoDB tables are being used.
+     * @return bool
+     */
+    public function optimizeTables(array $tables, bool $force = false): bool
+    {
+        return $this->getSchema()->optimizeTables($tables, $force);
+    }
 }

core/Db/Schema/Mariadb.php

@@ -37,4 +37,11 @@ public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): strin
 
         return $sql;
     }
+
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        $version = strtolower($this->getVersion());
+        $semanticVersion = strstr($version, '-', $beforeNeedle = true);
+        return version_compare($semanticVersion, '10.1.1', '>=');
+    }
 }

core/Db/Schema/Mysql.php

@@ -12,6 +12,7 @@
 use Exception;
 use Piwik\Common;
 use Piwik\Concurrency\Lock;
+use Piwik\Config;
 use Piwik\Date;
 use Piwik\Db\SchemaInterface;
 use Piwik\Db;
@@ -688,6 +689,69 @@ public function getTableCreateOptions(): string
         return $options;
     }
 
+    public function optimizeTables(array $tables, bool $force = false): bool
+    {
+        $optimize = Config::getInstance()->General['enable_sql_optimize_queries'];
+
+        if (
+            empty($optimize)
+            && !$force
+        ) {
+            return false;
+        }
+
+        if (empty($tables)) {
+            return false;
+        }
+
+        if (
+            !$this->isOptimizeInnoDBSupported()
+            && !$force
+        ) {
+            // filter out all InnoDB tables
+            $myisamDbTables = array();
+            foreach ($this->getTableStatus() as $row) {
+                if (
+                    strtolower($row['Engine']) == 'myisam'
+                    && in_array($row['Name'], $tables)
+                ) {
+                    $myisamDbTables[] = $row['Name'];
+                }
+            }
+
+            $tables = $myisamDbTables;
+        }
+
+        if (empty($tables)) {
+            return false;
+        }
+
+        // optimize the tables
+        $success = true;
+        foreach ($tables as &$t) {
+            $ok = Db::query('OPTIMIZE TABLE ' . $t);
+            if (!$ok) {
+                $success = false;
+            }
+        }
+
+        return $success;
+    }
+
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        $version = strtolower($this->getVersion());
+
+        // Note: This check for MariaDb is here on purpose, so it's working correctly for people
+        // having MySQL still configured, when using MariaDb
+        if (strpos($version, "mariadb") === false) {
+            return false;
+        }
+
+        $semanticVersion = strstr($version, '-', $beforeNeedle = true);
+        return version_compare($semanticVersion, '10.1.1', '>=');
+    }
+
     protected function getDatabaseCreateOptions(): string
     {
         $charset = DbHelper::getDefaultCharset();
@@ -715,6 +779,16 @@ private function getTablePrefix()
         return $this->getDbSettings()->getTablePrefix();
     }
 
+    protected function getVersion(): string
+    {
+        return Db::fetchOne("SELECT VERSION()");
+    }
+
+    protected function getTableStatus()
+    {
+        return Db::fetchAll("SHOW TABLE STATUS");
+    }
+
     private function getDb()
     {
         return Db::get();

core/Db/Schema/Tidb.php

@@ -51,6 +51,17 @@ public function getTableCreateOptions(): string
         return $options;
     }
 
+    public function isOptimizeInnoDBSupported(): bool
+    {
+        return false;
+    }
+
+    public function optimizeTables(array $tables, bool $force = false): bool
+    {
+        // OPTIMIZE TABLE not supported for TiDb
+        return false;
+    }
+
     protected function getDatabaseCreateOptions(): string
     {
         $charset = DbHelper::getDefaultCharset();

core/Db/SchemaInterface.php

@@ -138,4 +138,24 @@ public function getDefaultPort(): int;
      * @return string
      */
     public function getTableCreateOptions(): string;
+
+    /**
+     * Returns if performing on `OPTIMIZE TABLE` is supported for InnoDb tables
+     *
+     * @return bool
+     */
+    public function isOptimizeInnoDBSupported(): bool;
+
+    /**
+     * Runs an `OPTIMIZE TABLE` query on the supplied table or tables.
+     *
+     * Tables will only be optimized if the `[General] enable_sql_optimize_queries` INI config option is
+     * set to **1**.
+     *
+     * @param array $tables The name of the table to optimize or an array of tables to optimize.
+     *                      Table names must be prefixed (see {@link Piwik\Common::prefixTable()}).
+     * @param bool $force If true, the `OPTIMIZE TABLE` query will be run even if InnoDB tables are being used.
+     * @return bool
+     */
+    public function optimizeTables(array $tables, bool $force = false): bool;
 }

core/Updates/1.2-rc1.php

@@ -47,9 +47,11 @@ public function getMigrations(Updater $updater)
         return array(
             // Various performance improvements schema updates
             $this->migration->db->sql('ALTER TABLE `' . Common::prefixTable('log_visit') . '`
-                DROP `visit_server_date`,
                 DROP INDEX `index_idsite_date_config`,
-                DROP INDEX `index_idsite_datetime_config`,
+                DROP INDEX `index_idsite_datetime_config`
+               ', array(Updater\Migration\Db::ERROR_CODE_UNKNOWN_COLUMN, Updater\Migration\Db::ERROR_CODE_COLUMN_NOT_EXISTS)),
+            $this->migration->db->sql('ALTER TABLE `' . Common::prefixTable('log_visit') . '`
+                DROP `visit_server_date`,
                 ADD `idvisitor` BINARY(8) NOT NULL AFTER `idsite`,
                 ADD `config_id` BINARY(8) NOT NULL AFTER `config_md5config`
                ', array(Updater\Migration\Db::ERROR_CODE_UNKNOWN_COLUMN, Updater\Migration\Db::ERROR_CODE_COLUMN_NOT_EXISTS)),

plugins/Actions/API.php

@@ -513,11 +513,13 @@ protected function doFilterPageDatatableSearch($callBackParameters, $table, $sea
      */
     private function filterActionsDataTable($dataTable, $isPageTitleType)
     {
+        $dataTable->filter(function ($dataTable) {
+            $dataTable->setMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME, Metrics::getColumnsAggregationOperation());
+        });
         // Must be applied before Sort in this case, since the DataTable can contain both int and strings indexes
         // (in the transition period between pre 1.2 and post 1.2 datatable structure)
         $dataTable->filter('Piwik\Plugins\Actions\DataTable\Filter\Actions', [$isPageTitleType]);
         $dataTable->filter('Piwik\Plugins\Goals\DataTable\Filter\CalculateConversionPageRate');
-
         return $dataTable;
     }