Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix MariaDB list columns performance #6202

Merged
merged 5 commits into from
Nov 5, 2023
Merged

Fix MariaDB list columns performance #6202

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f921da9
Fix MariaDB list columns performance
ausi Oct 19, 2023
d0ac1da
Fix backwards compatibility
ausi Nov 3, 2023
b7017b1
Fix PHPStan and Psalm
ausi Nov 3, 2023
82d8266
Deprecate and replace getColumnTypeSQLSnippets()
ausi Nov 5, 2023
f2758ff
Revert deprecation
ausi Nov 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Platforms/AbstractMySQLPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ public function getListTableColumnsSQL($table, $database = null)
*
* @return array{string, string}
*/
public function getColumnTypeSQLSnippets(string $tableAlias = 'c'): array
public function getColumnTypeSQLSnippets(string $tableAlias = 'c', ?string $databaseName = null): array
ausi marked this conversation as resolved.
Show resolved Hide resolved
{
return [$tableAlias . '.COLUMN_TYPE', ''];
}
Expand Down
31 changes: 17 additions & 14 deletions src/Platforms/MariaDb1043Platform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
*/
public function getListTableColumnsSQL($table, $database = null): string
{
[$columnTypeSQL, $joinCheckConstraintSQL] = $this->getColumnTypeSQLSnippets();
[$columnTypeSQL, $joinCheckConstraintSQL] = $this->getColumnTypeSQLSnippets('c', $database);

return sprintf(
<<<SQL
Expand Down Expand Up @@ -76,30 +76,33 @@
*
* @return array{string, string}
*/
public function getColumnTypeSQLSnippets(string $tableAlias = 'c'): array
public function getColumnTypeSQLSnippets(string $tableAlias = 'c', ?string $databaseName = null): array
{
if ($this->getJsonTypeDeclarationSQL([]) !== 'JSON') {
return parent::getColumnTypeSQLSnippets($tableAlias);
return parent::getColumnTypeSQLSnippets($tableAlias, $databaseName);

Check warning on line 82 in src/Platforms/MariaDb1043Platform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/MariaDb1043Platform.php#L82 

Added line #L82 was not covered by tests
}

$databaseName = $this->getDatabaseNameSQL($databaseName);

$columnTypeSQL = <<<SQL
IF(
x.CHECK_CLAUSE IS NOT NULL AND $tableAlias.COLUMN_TYPE = 'longtext',
$tableAlias.COLUMN_TYPE = 'longtext'
AND EXISTS(
SELECT * from information_schema.CHECK_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = $databaseName
ausi marked this conversation as resolved.
Show resolved Hide resolved
AND TABLE_NAME = $tableAlias.TABLE_NAME
AND CHECK_CLAUSE = CONCAT(
'json_valid(`',
$tableAlias.COLUMN_NAME,
'`)'
)
),
'json',
$tableAlias.COLUMN_TYPE
)
SQL;

$joinCheckConstraintSQL = <<<SQL
LEFT JOIN information_schema.CHECK_CONSTRAINTS x
ON (
$tableAlias.TABLE_SCHEMA = x.CONSTRAINT_SCHEMA
AND $tableAlias.TABLE_NAME = x.TABLE_NAME
AND x.CHECK_CLAUSE = CONCAT('json_valid(`', $tableAlias.COLUMN_NAME , '`)')
)
SQL;

return [$columnTypeSQL, $joinCheckConstraintSQL];
return [$columnTypeSQL, ''];
}

/** {@inheritDoc} */
Expand Down
2 changes: 1 addition & 1 deletion src/Schema/MySQLSchemaManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ protected function selectTableNames(string $databaseName): Result

protected function selectTableColumns(string $databaseName, ?string $tableName = null): Result
{
[$columnTypeSQL, $joinCheckConstraintSQL] = $this->_platform->getColumnTypeSQLSnippets();
[$columnTypeSQL, $joinCheckConstraintSQL] = $this->_platform->getColumnTypeSQLSnippets('c', $databaseName);

$sql = 'SELECT';

Expand Down
6 changes: 6 additions & 0 deletions tests/Functional/Schema/MySQLSchemaManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\MariaDb1027Platform;
use Doctrine\DBAL\Platforms\MariaDb1043Platform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Tests\Functional\Schema\MySQL\PointType;
Expand Down Expand Up @@ -401,6 +402,11 @@ public function testJsonColumnType(): void
$table->addColumn('col_json', Types::JSON);
$this->dropAndCreateTable($table);

// Remove the comment from the column to ensure the type is detected correctly from the check constraints.
if ($this->connection->getDatabasePlatform() instanceof MariaDb1043Platform) {
$this->connection->executeStatement('ALTER TABLE test_mysql_json CHANGE COLUMN col_json col_json JSON');
}
derrabus marked this conversation as resolved.
Show resolved Hide resolved

$columns = $this->schemaManager->listTableColumns('test_mysql_json');

self::assertSame(Types::JSON, $columns['col_json']->getType()->getName());
Expand Down