Skip to content

Commit

Permalink
Allow to distinguish between MySQL and MariaDB (also in installation) (
Browse files Browse the repository at this point in the history
…#18371)

* Allow to distinguish between MySQL and MariaDB (also in installation)

* updates expected UI test files

* add tests

* run tests with MariaDb

* adjust some tests to work with MariaDb

* submodule update

* ensure static order for custom dimensions

* fix userid tests

* fix max execution time tests

* adjust action param

* Use database engine instead of schema in frontend

* skip test on Mariadb

* Apply suggestions from code review

* Update .github/workflows/matomo-tests.yml

* updates expected UI test files

* fix diagnostic check

* Recreate OmniFixture after fixture changes

* updates expected UI test files

* improve ui test

* Apply suggestions from code review

Co-authored-by: Marc Neudert <[email protected]>

* improve mariadb diagnostic

* submodule update

---------

Co-authored-by: Michal Kleiner <[email protected]>
Co-authored-by: Marc Neudert <[email protected]>
  • Loading branch information
3 people authored May 17, 2024
1 parent 9deb8c7 commit 2743e52
Show file tree
Hide file tree
Showing 57 changed files with 1,253 additions and 1,122 deletions.
10 changes: 9 additions & 1 deletion .github/workflows/matomo-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,19 @@ jobs:
matrix:
type: [ 'UnitTests', 'SystemTestsPlugins', 'SystemTestsCore', 'IntegrationTestsCore', 'IntegrationTestsPlugins' ]
php: [ '7.2', '8.2', '8.3' ]
engine: [ 'Mysql', 'Mariadb' ]
adapter: [ 'PDO_MYSQL', 'MYSQLI' ]
exclude:
- php: '7.2'
engine: 'Mariadb'
- php: '8.2'
engine: 'Mysql'
- php: '8.3'
engine: 'Mariadb'
- php: '7.2'
adapter: 'MYSQLI'
- php: '8.2'
adapter: 'PDO_MYSQL'
adapter: 'MYSQLI'
- php: '8.3'
adapter: 'PDO_MYSQL'
steps:
Expand All @@ -58,6 +65,7 @@ jobs:
with:
test-type: ${{ matrix.type }}
mysql-driver: ${{ matrix.adapter }}
mysql-engine: ${{ matrix.engine }}
php-version: ${{ matrix.php }}
redis-service: true
artifacts-pass: ${{ secrets.ARTIFACTS_PASS }}
Expand Down
38 changes: 24 additions & 14 deletions core/Db/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
/**
* Schema abstraction
*
* Note: no relation to the ZF proposals for Zend_Db_Schema_Manager
*
* @method static \Piwik\Db\Schema getInstance()
*/
class Schema extends Singleton
Expand All @@ -26,7 +24,7 @@ class Schema extends Singleton
/**
* Type of database schema
*
* @var string
* @var SchemaInterface
*/
private $schema = null;

Expand All @@ -36,7 +34,7 @@ class Schema extends Singleton
* @param string $schemaName
* @return string
*/
private static function getSchemaClassName($schemaName)
private static function getSchemaClassName($schemaName): string
{
// Upgrade from pre 2.0.4
if (
Expand All @@ -54,7 +52,7 @@ private static function getSchemaClassName($schemaName)
/**
* Load schema
*/
private function loadSchema()
private function loadSchema(): void
{
$config = Config::getInstance();
$dbInfos = $config->database;
Expand All @@ -67,9 +65,9 @@ private function loadSchema()
/**
* Returns an instance that subclasses Schema
*
* @return \Piwik\Db\SchemaInterface
* @return SchemaInterface
*/
private function getSchema()
private function getSchema(): SchemaInterface
{
if ($this->schema === null) {
$this->loadSchema();
Expand Down Expand Up @@ -102,7 +100,7 @@ public function getTablesCreateSql()
/**
* Creates a new table in the database.
*
* @param string $nameWithoutPrefix The name of the table without any piwik prefix.
* @param string $nameWithoutPrefix The name of the table without any prefix.
* @param string $createDefinition The table create definition
*/
public function createTable($nameWithoutPrefix, $createDefinition)
Expand Down Expand Up @@ -131,23 +129,23 @@ public function dropDatabase($dbName = null)
/**
* Create all tables
*/
public function createTables()
public function createTables(): void
{
$this->getSchema()->createTables();
}

/**
* Creates an entry in the User table for the "anonymous" user.
*/
public function createAnonymousUser()
public function createAnonymousUser(): void
{
$this->getSchema()->createAnonymousUser();
}

/**
* Records the Matomo version a user used when installing this Matomo for the first time
*/
public function recordInstallVersion()
public function recordInstallVersion(): void
{
$this->getSchema()->recordInstallVersion();
}
Expand All @@ -163,13 +161,13 @@ public function getInstallVersion()
/**
* Truncate all tables
*/
public function truncateAllTables()
public function truncateAllTables(): void
{
$this->getSchema()->truncateAllTables();
}

/**
* Names of all the prefixed tables in piwik
* Names of all the prefixed tables in Matomo
* Doesn't use the DB
*
* @return array Table names
Expand Down Expand Up @@ -203,12 +201,24 @@ public function getTableColumns($tableName)
}

/**
* Returns true if Piwik tables exist
* Returns true if Matomo tables exist
*
* @return bool True if tables exist; false otherwise
*/
public function hasTables()
{
return $this->getSchema()->hasTables();
}

/**
* Adds a MAX_EXECUTION_TIME hint into a SELECT query if $limit is bigger than 0
*
* @param string $sql query to add hint to
* @param float $limit time limit in seconds
* @return string
*/
public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): string
{
return $this->getSchema()->addMaxExecutionTimeHintToQuery($sql, $limit);
}
}
40 changes: 40 additions & 0 deletions core/Db/Schema/Mariadb.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/**
* Matomo - free/libre analytics platform
*
* @link https://matomo.org
* @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*/

namespace Piwik\Db\Schema;

/**
* Mariadb schema
*/
class Mariadb extends Mysql
{
/**
* Adds a max_statement_time hint into a SELECT query if $limit is bigger than 0
*
* @param string $sql query to add hint to
* @param float $limit time limit in seconds
* @return string
*/
public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): string
{
if ($limit <= 0) {
return $sql;
}

$sql = trim($sql);
$pos = stripos($sql, 'SELECT');
$isMaxExecutionTimeoutAlreadyPresent = (stripos($sql, 'max_statement_time=') !== false);
if ($pos !== false && !$isMaxExecutionTimeoutAlreadyPresent) {
$maxExecutionTimeHint = 'SET STATEMENT max_statement_time=' . ceil($limit) . ' FOR ';
$sql = substr_replace($sql, $maxExecutionTimeHint . 'SELECT', $pos, strlen('SELECT'));
}

return $sql;
}
}
27 changes: 27 additions & 0 deletions core/Db/Schema/Mysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,33 @@ public function truncateAllTables()
}
}

/**
* Adds a MAX_EXECUTION_TIME hint into a SELECT query if $limit is bigger than 0
*
* @param string $sql query to add hint to
* @param float $limit time limit in seconds
* @return string
*/
public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): string
{
if ($limit <= 0) {
return $sql;
}

$sql = trim($sql);
$pos = stripos($sql, 'SELECT');
$isMaxExecutionTimeoutAlreadyPresent = (stripos($sql, 'MAX_EXECUTION_TIME(') !== false);
if ($pos !== false && !$isMaxExecutionTimeoutAlreadyPresent) {
$timeInMs = $limit * 1000;
$timeInMs = (int) $timeInMs;
$maxExecutionTimeHint = ' /*+ MAX_EXECUTION_TIME(' . $timeInMs . ') */ ';

$sql = substr_replace($sql, 'SELECT ' . $maxExecutionTimeHint, $pos, strlen('SELECT'));
}

return $sql;
}

private function getTablePrefix()
{
return $this->getDbSettings()->getTablePrefix();
Expand Down
18 changes: 14 additions & 4 deletions core/Db/SchemaInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
interface SchemaInterface
{
/**
* Get the SQL to create a specific Piwik table
* Get the SQL to create a specific Matomo table
*
* @param string $tableName
* @return string SQL
*/
public function getTableCreateSql($tableName);

/**
* Get the SQL to create Piwik tables
* Get the SQL to create Matomo tables
*
* @return array array of strings containing SQL
*/
Expand All @@ -32,7 +32,7 @@ public function getTablesCreateSql();
/**
* Creates a new table in the database.
*
* @param string $nameWithoutPrefix The name of the table without any piwik prefix.
* @param string $nameWithoutPrefix The name of the table without any prefix.
* @param string $createDefinition The table create definition
*/
public function createTable($nameWithoutPrefix, $createDefinition);
Expand Down Expand Up @@ -75,7 +75,7 @@ public function getInstallVersion();
public function truncateAllTables();

/**
* Names of all the prefixed tables in piwik
* Names of all the prefixed tables in Matomo
* Doesn't use the DB
*
* @return array Table names
Expand Down Expand Up @@ -105,4 +105,14 @@ public function getTableColumns($tableName);
* @return bool True if tables exist; false otherwise
*/
public function hasTables();

/**
* Adds a max execution time query hint into a SELECT query if $limit is bigger than 0
* (floating values for limit might be rounded to full seconds depending on DB support)
*
* @param string $sql query to add hint to
* @param float $limit time limit in seconds
* @return string
*/
public function addMaxExecutionTimeHintToQuery(string $sql, float $limit): string;
}
23 changes: 4 additions & 19 deletions core/DbHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -287,30 +287,15 @@ public static function deleteArchiveTables()
}

/**
* Adds a MAX_EXECUTION_TIME hint into a SELECT query if $limit is bigger than 1
* Adds a MAX_EXECUTION_TIME hint into a SELECT query if $limit is bigger than 0
*
* @param string $sql query to add hint to
* @param int $limit time limit in seconds
* @param float $limit time limit in seconds
* @return string
*/
public static function addMaxExecutionTimeHintToQuery($sql, $limit)
public static function addMaxExecutionTimeHintToQuery(string $sql, float $limit): string
{
if ($limit <= 0) {
return $sql;
}

$sql = trim($sql);
$pos = stripos($sql, 'SELECT');
$isMaxExecutionTimeoutAlreadyPresent = (stripos($sql, 'MAX_EXECUTION_TIME(') !== false);
if ($pos !== false && !$isMaxExecutionTimeoutAlreadyPresent) {
$timeInMs = $limit * 1000;
$timeInMs = (int) $timeInMs;
$maxExecutionTimeHint = ' /*+ MAX_EXECUTION_TIME(' . $timeInMs . ') */ ';

$sql = substr_replace($sql, 'SELECT ' . $maxExecutionTimeHint, $pos, strlen('SELECT'));
}

return $sql;
return Schema::getInstance()->addMaxExecutionTimeHintToQuery($sql, $limit);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions core/Updater/Migration/Db.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ abstract class Db extends Migration
*/
public const ERROR_CODE_MAX_EXECUTION_TIME_EXCEEDED_QUERY_INTERRUPTED = 3024;

/**
* MariaDb only: Query execution was interrupted (max_statement_time exceeded)
*/
public const ERROR_CODE_MAX_STATEMENT_TIME_EXCEEDED_QUERY_INTERRUPTED = 1969;

/**
* Sort aborted: Query execution was interrupted, maximum statement execution time exceeded
*/
Expand Down
Loading

0 comments on commit 2743e52

Please sign in to comment.