Skip to content

Use Native JSON type instead of LONGTEXT for MariaDB #5916

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.

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

Merged
merged 4 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions src/Driver/AbstractMySQLDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\MariaDb1027Platform;
use Doctrine\DBAL\Platforms\MariaDb1043Platform;
use Doctrine\DBAL\Platforms\MariaDb1052Platform;
use Doctrine\DBAL\Platforms\MySQL57Platform;
use Doctrine\DBAL\Platforms\MySQL80Platform;
Expand All @@ -35,12 +36,17 @@ abstract class AbstractMySQLDriver implements VersionAwarePlatformDriver
public function createDatabasePlatformForVersion($version)
{
$mariadb = stripos($version, 'mariadb') !== false;

if ($mariadb) {
$mariaDbVersion = $this->getMariaDbMysqlVersionNumber($version);
if (version_compare($mariaDbVersion, '10.5.2', '>=')) {
return new MariaDb1052Platform();
}

if (version_compare($mariaDbVersion, '10.4.3', '>=')) {
return new MariaDb1043Platform();
}

if (version_compare($mariaDbVersion, '10.2.7', '>=')) {
return new MariaDb1027Platform();
}
Expand Down
14 changes: 13 additions & 1 deletion src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,18 @@ public function getListTableColumnsSQL($table, $database = null)
' ORDER BY ORDINAL_POSITION ASC';
}

/**
* The SQL snippets required to elucidate a column type
*
* Returns an array of the form [column type SELECT snippet, additional JOIN statement snippet]
*
* @return array{string, string}
*/
public function getColumnTypeSQLSnippets(string $tableAlias = 'c'): array
{
return [$tableAlias . '.COLUMN_TYPE', ''];
}

/** @deprecated The SQL used for schema introspection is an implementation detail and should not be relied upon. */
public function getListTableMetadataSQL(string $table, ?string $database = null): string
{
Expand Down Expand Up @@ -1390,7 +1402,7 @@ public function supportsColumnLengthIndexes(): bool
return true;
}

private function getDatabaseNameSQL(?string $databaseName): string
protected function getDatabaseNameSQL(?string $databaseName): string
{
if ($databaseName !== null) {
return $this->quoteStringLiteral($databaseName);
Expand Down
15 changes: 15 additions & 0 deletions src/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -4626,6 +4626,10 @@ public function columnsEqual(Column $column1, Column $column2): bool
return false;
}

if (! $this->columnDeclarationsMatch($column1, $column2)) {
return false;
}

// If the platform supports inline comments, all comparison is already done above
if ($this->supportsInlineColumnComments()) {
return true;
Expand All @@ -4638,6 +4642,17 @@ public function columnsEqual(Column $column1, Column $column2): bool
return $column1->getType() === $column2->getType();
}

/**
* Whether the database data type matches that expected for the doctrine type for the given colunms.
*/
private function columnDeclarationsMatch(Column $column1, Column $column2): bool
{
return ! (
$column1->hasPlatformOption('declarationMismatch') ||
$column2->hasPlatformOption('declarationMismatch')
);
}

/**
* Creates the schema manager that can be used to inspect and change the underlying
* database schema according to the dialect of the platform.
Expand Down
114 changes: 114 additions & 0 deletions src/Platforms/MariaDb1043Platform.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php

namespace Doctrine\DBAL\Platforms;

use Doctrine\DBAL\Types\JsonType;

use function sprintf;

/**
* Provides the behavior, features and SQL dialect of the MariaDB 10.4 (10.4.6 GA) database platform.
*
* Extend deprecated MariaDb1027Platform to ensure correct functions used in MySQLSchemaManager which
* tests for MariaDb1027Platform not MariaDBPlatform.
*/
class MariaDb1043Platform extends MariaDb1027Platform
{
/**
* Use JSON rather than LONGTEXT for json columns. Since it is not a true native type, do not override
* hasNativeJsonType() so the DC2Type comment will still be set.
*
* {@inheritdoc}
*/
public function getJsonTypeDeclarationSQL(array $column): string
{
return 'JSON';
}

/**
* {@inheritdoc}
*
* From version 10.4.3, MariaDb aliases JSON to LONGTEXT and adds a constraint CHECK (json_valid). Reverse
* this process when introspecting tables.
*
* @see https://mariadb.com/kb/en/information-schema-check_constraints-table/
* @see https://mariadb.com/kb/en/json-data-type/
* @see https://jira.mariadb.org/browse/MDEV-13916
*/
public function getListTableColumnsSQL($table, $database = null): string
{
[$columnTypeSQL, $joinCheckConstraintSQL] = $this->getColumnTypeSQLSnippets();

return sprintf(
<<<SQL
SELECT c.COLUMN_NAME AS Field,
$columnTypeSQL AS Type,
c.IS_NULLABLE AS `Null`,
c.COLUMN_KEY AS `Key`,
c.COLUMN_DEFAULT AS `Default`,
c.EXTRA AS Extra,
c.COLUMN_COMMENT AS Comment,
c.CHARACTER_SET_NAME AS CharacterSet,
c.COLLATION_NAME AS Collation
FROM information_schema.COLUMNS c
$joinCheckConstraintSQL
WHERE c.TABLE_SCHEMA = %s
AND c.TABLE_NAME = %s
ORDER BY ORDINAL_POSITION ASC;
SQL
,
$this->getDatabaseNameSQL($database),
$this->quoteStringLiteral($table),
);
}

/**
* Generate SQL snippets to reverse the aliasing of JSON to LONGTEXT.
*
* MariaDb aliases columns specified as JSON to LONGTEXT and sets a CHECK constraint to ensure the column
* is valid json. This function generates the SQL snippets which reverse this aliasing i.e. report a column
* as JSON where it was originally specified as such instead of LONGTEXT.
*
* The CHECK constraints are stored in information_schema.CHECK_CONSTRAINTS so JOIN that table.
*
* @return array{string, string}
*/
public function getColumnTypeSQLSnippets(string $tableAlias = 'c'): array
{
if ($this->getJsonTypeDeclarationSQL([]) !== 'JSON') {
return parent::getColumnTypeSQLSnippets($tableAlias);
}

$columnTypeSQL = <<<SQL
IF(
x.CHECK_CLAUSE IS NOT NULL AND $tableAlias.COLUMN_TYPE = 'longtext',
'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];
}

/** {@inheritDoc} */
public function getColumnDeclarationSQL($name, array $column)
{
// MariaDb forces column collation to utf8mb4_bin where the column was declared as JSON so ignore
// collation and character set for json columns as attempting to set them can cause an error.
if ($this->getJsonTypeDeclarationSQL([]) === 'JSON' && ($column['type'] ?? null) instanceof JsonType) {
unset($column['collation']);
unset($column['charset']);
}

return parent::getColumnDeclarationSQL($name, $column);
}
}
2 changes: 1 addition & 1 deletion src/Platforms/MariaDb1052Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*
* Note: Should not be used with versions prior to 10.5.2.
*/
class MariaDb1052Platform extends MariaDb1027Platform
class MariaDb1052Platform extends MariaDb1043Platform
{
/**
* {@inheritdoc}
Expand Down
41 changes: 38 additions & 3 deletions src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,20 @@ protected function _getPortableTableColumnDefinition($tableColumn)
$scale = null;
$precision = null;

$type = $this->_platform->getDoctrineTypeMapping($dbType);
$type = $origType = $this->_platform->getDoctrineTypeMapping($dbType);

// In cases where not connected to a database DESCRIBE $table does not return 'Comment'
if (isset($tableColumn['comment'])) {
$type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type);
$tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type);
}

// Check underlying database type where doctrine type is inferred from DC2Type comment
// and set a flag if it is not as expected.
if ($origType !== $type && $this->expectedDbType($type, $tableColumn) !== $dbType) {
$tableColumn['declarationMismatch'] = true;
}

switch ($dbType) {
case 'char':
case 'binary':
Expand Down Expand Up @@ -286,9 +292,35 @@ protected function _getPortableTableColumnDefinition($tableColumn)
$column->setPlatformOption('collation', $tableColumn['collation']);
}

if (isset($tableColumn['declarationMismatch'])) {
$column->setPlatformOption('declarationMismatch', $tableColumn['declarationMismatch']);
}

return $column;
}

/**
* Returns the database data type for a given doctrine type and column
*
* Note that for data types that depend on length where length is not part of the column definition
* and therefore the $tableColumn['length'] will not be set, for example TEXT (which could be LONGTEXT,
* MEDIUMTEXT) or BLOB (LONGBLOB or TINYBLOB), the expectedDbType cannot be inferred exactly, merely
* the default type.
*
* This method is intended to be used to determine underlying database type where doctrine type is
* inferred from a DC2Type comment.
*
* @param mixed[] $tableColumn
*/
private function expectedDbType(string $type, array $tableColumn): string
{
$_type = Type::getType($type);
$expectedDbType = strtolower($_type->getSQLDeclaration($tableColumn, $this->_platform));
$expectedDbType = strtok($expectedDbType, '(), ');

return $expectedDbType === false ? '' : $expectedDbType;
}

/**
* Return Doctrine/Mysql-compatible column default values for MariaDB 10.2.7+ servers.
*
Expand Down Expand Up @@ -405,15 +437,17 @@ protected function selectTableNames(string $databaseName): Result

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

$sql = 'SELECT';

if ($tableName === null) {
$sql .= ' c.TABLE_NAME,';
}

$sql .= <<<'SQL'
$sql .= <<<SQL
c.COLUMN_NAME AS field,
c.COLUMN_TYPE AS type,
$columnTypeSQL AS type,
c.IS_NULLABLE AS `null`,
c.COLUMN_KEY AS `key`,
c.COLUMN_DEFAULT AS `default`,
Expand All @@ -424,6 +458,7 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
FROM information_schema.COLUMNS c
INNER JOIN information_schema.TABLES t
ON t.TABLE_NAME = c.TABLE_NAME
$joinCheckConstraintSQL
SQL;

// The schema name is passed multiple times as a literal in the WHERE clause instead of using a JOIN condition
Expand Down
19 changes: 19 additions & 0 deletions tests/Functional/Schema/MySQL/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\MariaDb1043Platform;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\Comparator;
Expand Down Expand Up @@ -147,6 +148,24 @@ public function testImplicitColumnCharset(): void
));
}

public function testMariaDb1043NativeJsonUpgradeDetected(): void
{
if (! $this->platform instanceof MariaDb1043Platform) {
self::markTestSkipped();
}

$table = new Table('mariadb_json_upgrade');

$table->addColumn('json_col', 'json');
$this->dropAndCreateTable($table);

// Revert column to old LONGTEXT declaration
$sql = 'ALTER TABLE mariadb_json_upgrade CHANGE json_col json_col LONGTEXT NOT NULL COMMENT \'(DC2Type:json)\'';
$this->connection->executeStatement($sql);

ComparatorTestUtils::assertDiffNotEmpty($this->connection, $this->comparator, $table);
}

/**
* @return array{Table,Column}
*
Expand Down
Loading