Skip to content

Commit

Permalink
MariaDb1043. Detect the need to migrate JSON columns to native JSON.
Browse files Browse the repository at this point in the history
MariaDb JSON columns are marked as such with a DC2Type comment hint.

The schema manager maps this to a json doctrine type which matches
expectations whether or not the column was declared as LONGTEXT (by the
previous MariaDb1027 platform) or JSON. This means the comparator will
not detect that a column should be upgraded from LONGTEXT to native JSON
and no migration would be generated.

Therefore, during column introspection check that the data type of the
introspected column matches that which should be set for the doctrine
type where that doctrine type was inferred from a DC2Type comment and
set a flag (a platformOption for the relevant column) where it does not.

Check whether the flag is set when the comparator diffs the expected
and actual tables and mark a column type difference where the flag is
set so a migration is generated.
  • Loading branch information
cgknx committed Mar 4, 2023
1 parent fed472c commit f2ff1ad
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 1 deletion.
14 changes: 14 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,16 @@ 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.
*
* To be implemented in specific platforms as necessary.
*/
public function columnDeclarationsMatch(Column $column1, ?Column $column2 = null): bool
{
return true;
}

/**
* 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
14 changes: 14 additions & 0 deletions src/Platforms/MariaDb1043Platform.php
Expand Up @@ -2,6 +2,7 @@

namespace Doctrine\DBAL\Platforms;

use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Types\JsonType;

use function sprintf;
Expand Down Expand Up @@ -111,4 +112,17 @@ public function getColumnDeclarationSQL($name, array $column)

return parent::getColumnDeclarationSQL($name, $column);
}

/**
* Whether the database data type matches that expected for the doctrine type
*/
public function columnDeclarationsMatch(Column $column1, ?Column $column2 = null): bool
{
$column2 ??= $column1;

return ! (
$column1->hasPlatformOption('declarationMismatch') ||
$column2->hasPlatformOption('declarationMismatch')
);
}
}
34 changes: 33 additions & 1 deletion 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
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
7 changes: 7 additions & 0 deletions tests/Platforms/AbstractPlatformTestCase.php
Expand Up @@ -1399,6 +1399,13 @@ public function testEmptySchemaDiff(): void
self::assertSame([], $this->platform->getAlterSchemaSQL($diff));
}

public function testColumnDeclarationsMatch(): void
{
$column1 = new Column('col', Type::getType('string'));

self::assertTrue($this->platform->columnDeclarationsMatch($column1));
}

public function tearDown(): void
{
if (! isset($this->backedUpType)) {
Expand Down
17 changes: 17 additions & 0 deletions tests/Platforms/MariaDb1043PlatformTest.php
Expand Up @@ -4,6 +4,8 @@

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\MariaDb1043Platform;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;

class MariaDb1043PlatformTest extends AbstractMySQLPlatformTestCase
Expand Down Expand Up @@ -39,4 +41,19 @@ public function testIgnoresDifferenceInDefaultValuesForUnsupportedColumnTypes():
{
self::markTestSkipped('MariaDb1043Platform supports default values for BLOB and TEXT columns');
}

public function testColumnDeclarationsMatch(): void
{
$column1 = new Column('col', Type::getType('string'));
$column2 = new Column('col', Type::getType('string'));

self::assertTrue($this->platform->columnDeclarationsMatch($column1, $column2));

$column2->setPlatformOption('declarationMismatch', true);

self::assertFalse($this->platform->columnDeclarationsMatch($column1, $column2));
self::assertFalse($this->platform->columnDeclarationsMatch($column2, $column1));
self::assertFalse($this->platform->columnDeclarationsMatch($column2));
self::assertTrue($this->platform->columnDeclarationsMatch($column1));
}
}

0 comments on commit f2ff1ad

Please sign in to comment.