From f2ff1add89c1f98ae40f167feadbc380866a57d8 Mon Sep 17 00:00:00 2001 From: Colin Knox <124388865+cgknx@users.noreply.github.com> Date: Sat, 4 Mar 2023 13:54:07 +0000 Subject: [PATCH] MariaDb1043. Detect the need to migrate JSON columns to native JSON. 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. --- src/Platforms/AbstractPlatform.php | 14 ++++++++ src/Platforms/MariaDb1043Platform.php | 14 ++++++++ src/Schema/MySQLSchemaManager.php | 34 ++++++++++++++++++- .../Schema/MySQL/ComparatorTest.php | 19 +++++++++++ tests/Platforms/AbstractPlatformTestCase.php | 7 ++++ tests/Platforms/MariaDb1043PlatformTest.php | 17 ++++++++++ 6 files changed, 104 insertions(+), 1 deletion(-) diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 28e0782e8a9..11d21ced01d 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -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; @@ -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. diff --git a/src/Platforms/MariaDb1043Platform.php b/src/Platforms/MariaDb1043Platform.php index a9b7b1de5ba..6fe607f506b 100644 --- a/src/Platforms/MariaDb1043Platform.php +++ b/src/Platforms/MariaDb1043Platform.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Platforms; +use Doctrine\DBAL\Schema\Column; use Doctrine\DBAL\Types\JsonType; use function sprintf; @@ -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') + ); + } } diff --git a/src/Schema/MySQLSchemaManager.php b/src/Schema/MySQLSchemaManager.php index 5b43ab07463..f5cf0f87e18 100644 --- a/src/Schema/MySQLSchemaManager.php +++ b/src/Schema/MySQLSchemaManager.php @@ -183,7 +183,7 @@ 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'])) { @@ -191,6 +191,12 @@ protected function _getPortableTableColumnDefinition($tableColumn) $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': @@ -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. * diff --git a/tests/Functional/Schema/MySQL/ComparatorTest.php b/tests/Functional/Schema/MySQL/ComparatorTest.php index 8665e50ec47..d799d0ac3b7 100644 --- a/tests/Functional/Schema/MySQL/ComparatorTest.php +++ b/tests/Functional/Schema/MySQL/ComparatorTest.php @@ -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; @@ -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} * diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php index 1fff8734a01..8e64cb5cd49 100644 --- a/tests/Platforms/AbstractPlatformTestCase.php +++ b/tests/Platforms/AbstractPlatformTestCase.php @@ -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)) { diff --git a/tests/Platforms/MariaDb1043PlatformTest.php b/tests/Platforms/MariaDb1043PlatformTest.php index e3c8a1aed16..d6bfdbd56e6 100644 --- a/tests/Platforms/MariaDb1043PlatformTest.php +++ b/tests/Platforms/MariaDb1043PlatformTest.php @@ -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 @@ -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)); + } }