Skip to content

Commit

Permalink
Fix table name in unsigned key usage warning
Browse files Browse the repository at this point in the history
  • Loading branch information
cedric-anne authored and trasher committed Jan 17, 2022
1 parent 25a730d commit b5907d0
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 34 deletions.
9 changes: 8 additions & 1 deletion src/DBmysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -1957,7 +1957,14 @@ public function executeStatement(mysqli_stmt $stmt): void
private function checkForDeprecatedTableOptions(string $query): void
{
$table_matches = [];
if (preg_match('/(ALTER|CREATE)\s+TABLE\s*(\s|`)(?<table>[^`\s]+)(\s|`)/i', $query, $table_matches) !== 1) {
$table_pattern = '/'
. '(ALTER|CREATE)'
. '(\s+TEMPORARY)?'
. '\s+TABLE'
. '(\s+IF\s+NOT\s+EXISTS)?'
. '\s*(\s|`)(?<table>[^`\s]+)(\s|`)'
. '/i';
if (preg_match($table_pattern, $query, $table_matches) !== 1) {
return;
}

Expand Down
130 changes: 97 additions & 33 deletions tests/units/DB.php
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,12 @@ public function testRemoveSqlRemarks($sql, $expected)
protected function tableOptionProvider(): iterable
{
yield [
'table_options' => '',
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
)
SQL,
'db_properties' => [],
'warning' => null
];
Expand All @@ -484,17 +488,25 @@ protected function tableOptionProvider(): iterable

foreach ($myisam_declarations as $table_options) {
yield [
'table_options' => $table_options,
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
){$table_options}
SQL,
'db_properties' => [
'allow_myisam' => true
],
'warning' => null
];

yield [
'table_options' => $table_options,
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
){$table_options}
SQL,
'db_properties' => [
'allow_myisam' => false
],
Expand All @@ -504,16 +516,26 @@ protected function tableOptionProvider(): iterable

// Warnings related to datetime fields
yield [
'table_options' => '',
'extra_fields' => '`date` datetime NOT NULL,',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
`date` datetime NOT NULL,
UNIQUE KEY (`nameid`)
)
SQL,
'db_properties' => [
'allow_datetime' => true
],
'warning' => null
];
yield [
'table_options' => '',
'extra_fields' => '`date` datetime NOT NULL,',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
`date` datetime NOT NULL,
UNIQUE KEY (`nameid`)
)
SQL,
'db_properties' => [
'allow_datetime' => false
],
Expand All @@ -522,16 +544,24 @@ protected function tableOptionProvider(): iterable

// Warnings related to 'utf8mb4' usage when DB not yet migrated to 'utf8mb4'
yield [
'table_options' => 'ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8 COLLATE = utf8_unicode_ci',
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
) ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8 COLLATE = utf8_unicode_ci
SQL,
'db_properties' => [
'use_utf8mb4' => false
],
'warning' => null
];
yield [
'table_options' => 'ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8mb4 COLLATE = utf8mb4_unicode_ci',
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
) ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8mb4 COLLATE = utf8mb4_unicode_ci
SQL,
'db_properties' => [
'use_utf8mb4' => false
],
Expand All @@ -540,16 +570,24 @@ protected function tableOptionProvider(): iterable

// Warnings related to 'utf8' usage when DB has been migrated to 'utf8mb4'
yield [
'table_options' => 'ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8 COLLATE = utf8_unicode_ci',
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
) ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8 COLLATE = utf8_unicode_ci
SQL,
'db_properties' => [
'use_utf8mb4' => true
],
'warning' => 'Usage of "utf8" charset/collation detected, should be "utf8mb4"'
];
yield [
'table_options' => 'ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8mb4 COLLATE = utf8mb4_unicode_ci',
'extra_fields' => '',
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
UNIQUE KEY (`nameid`)
) ENGINE = InnoDB ROW_FORMAT = DYNAMIC DEFAULT CHARSET = utf8mb4 COLLATE = utf8mb4_unicode_ci
SQL,
'db_properties' => [
'use_utf8mb4' => true
],
Expand All @@ -573,16 +611,26 @@ protected function tableOptionProvider(): iterable
];
foreach ($int_declarations as $int_declaration => $warning_field) {
yield [
'table_options' => '',
'extra_fields' => $int_declaration,
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
{$int_declaration}
UNIQUE KEY (`nameid`)
)
SQL,
'db_properties' => [
'allow_signed_keys' => true
],
'warning' => null // No warning as we allow signed keys
];
yield [
'table_options' => '',
'extra_fields' => $int_declaration,
'sql' => <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
{$int_declaration}
UNIQUE KEY (`nameid`)
)
SQL,
'db_properties' => [
'allow_signed_keys' => false
],
Expand All @@ -591,26 +639,42 @@ protected function tableOptionProvider(): iterable
: null,
];
}

// Check table name extracted in warnings
$table_declarations = [
'CREATE TEMPORARY TABLE `%s`', // temporary table
'CREATE TABLE IF NOT EXISTS `%s`', // if not exists
'CREATE TABLE`%s`', // no space before table name
'CREATE TABLE %s', // no quotes
'CREATE TEMPORARY TABLE IF NOT EXISTS`%s`', // random spacing
];
foreach ($table_declarations as $table_declaration) {
yield [
'sql' => <<<SQL
{$table_declaration} (
`id` int NOT NULL AUTO_INCREMENT,
PRIMARY KEY (`id`)
)
SQL,
'db_properties' => [
'allow_signed_keys' => false
],
'warning' => sprintf('Usage of signed integers in primary or foreign keys is discouraged, please use unsigned integers instead in `{$table}`.`id`.'),
];
}
}

/**
* @dataProvider tableOptionProvider
*/
public function testAlterOrCreateTableWarnings(
string $table_options,
string $extra_fields,
string $sql,
array $db_properties,
?string $warning = null
) {
$db = new \mock\DB();

$create_query_template = <<<SQL
CREATE TABLE `%s` (
`nameid` varchar(100) NOT NULL,
%s
UNIQUE KEY (`nameid`)
)%s
SQL;
$create_query_template = $sql;
$drop_query_template = 'DROP TABLE `%s`';

$db->log_deprecation_warnings = false; // Prevent deprecation warning from MySQL server
Expand All @@ -622,8 +686,8 @@ public function testAlterOrCreateTableWarnings(

$table = sprintf('glpitests_%s', uniqid());
$this->when(
function () use ($db, $create_query_template, $drop_query_template, $table, $extra_fields, $table_options) {
$db->query(sprintf($create_query_template, $table, $extra_fields, $table_options));
function () use ($db, $create_query_template, $drop_query_template, $table) {
$db->query(sprintf($create_query_template, $table));
$db->query(sprintf($drop_query_template, $table));
}
)->error()
Expand Down

0 comments on commit b5907d0

Please sign in to comment.