Skip to content

Commit

Permalink
Merge pull request #5916 from cgknx/native-mariadb-json
Browse files Browse the repository at this point in the history
Use Native JSON type instead of LONGTEXT for MariaDB
  • Loading branch information
derrabus committed Mar 6, 2023
2 parents f4ba78c + 04c3330 commit 42877bf
Show file tree
Hide file tree
Showing 13 changed files with 590 additions and 6 deletions.
6 changes: 6 additions & 0 deletions src/Driver/AbstractMySQLDriver.php
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
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
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
@@ -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
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
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
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

0 comments on commit 42877bf

Please sign in to comment.