Improve MySQL charset and collation comparison

doctrine · Jul 20, 2022 · 941005a · 941005a
1 parent 9045f6d
commit 941005a
Show file tree

Hide file tree

Showing 12 changed files with 238 additions and 26 deletions.
diff --git a/src/Platforms/MySQL/CharsetMetadataProvider.php b/src/Platforms/MySQL/CharsetMetadataProvider.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Platforms\MySQL;
+
+/**
+ * @internal
+ */
+interface CharsetMetadataProvider
+{
+    public function getDefaultCharsetCollation(string $charset): ?string;
+}
diff --git a/src/Platforms/MySQL/CharsetMetadataProvider/CachingCharsetMetadataProvider.php b/src/Platforms/MySQL/CharsetMetadataProvider/CachingCharsetMetadataProvider.php
@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
+
+use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
+
+use function array_key_exists;
+
+/**
+ * @internal
+ */
+final class CachingCharsetMetadataProvider implements CharsetMetadataProvider
+{
+    private CharsetMetadataProvider $charsetMetadataProvider;
+
+    /** @var array<string,?string> */
+    private array $cache = [];
+
+    public function __construct(CharsetMetadataProvider $charsetMetadataProvider)
+    {
+        $this->charsetMetadataProvider = $charsetMetadataProvider;
+    }
+
+    public function getDefaultCharsetCollation(string $charset): ?string
+    {
+        if (array_key_exists($charset, $this->cache)) {
+            return $this->cache[$charset];
+        }
+
+        return $this->cache[$charset] = $this->charsetMetadataProvider->getDefaultCharsetCollation($charset);
+    }
+}
diff --git a/src/Platforms/MySQL/CharsetMetadataProvider/ConnectionCharsetMetadataProvider.php b/src/Platforms/MySQL/CharsetMetadataProvider/ConnectionCharsetMetadataProvider.php
@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
+
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Exception;
+use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
+
+/**
+ * @internal
+ */
+final class ConnectionCharsetMetadataProvider implements CharsetMetadataProvider
+{
+    private Connection $connection;
+
+    public function __construct(Connection $connection)
+    {
+        $this->connection = $connection;
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function getDefaultCharsetCollation(string $charset): ?string
+    {
+        $collation = $this->connection->fetchOne(
+            <<<'SQL'
+SELECT DEFAULT_COLLATE_NAME
+FROM information_schema.CHARACTER_SETS
+WHERE CHARACTER_SET_NAME = ?;
+SQL
+            ,
+            [$charset]
+        );
+
+        if ($collation !== false) {
+            return $collation;
+        }
+
+        return null;
+    }
+}
diff --git a/src/Platforms/MySQL/Comparator.php b/src/Platforms/MySQL/Comparator.php
@@ -10,7 +10,6 @@
 use Doctrine\DBAL\Schema\TableDiff;
 
 use function array_diff_assoc;
-use function array_intersect_key;
 
 /**
  * Compares schemas in the context of MySQL platform.
@@ -21,32 +20,54 @@
  */
 class Comparator extends BaseComparator
 {
+    private CharsetMetadataProvider $charsetMetadataProvider;
+
     private CollationMetadataProvider $collationMetadataProvider;
 
+    private DefaultTableOptions $defaultTableOptions;
+
     /**
      * @internal The comparator can be only instantiated by a schema manager.
      */
-    public function __construct(AbstractMySQLPlatform $platform, CollationMetadataProvider $collationMetadataProvider)
-    {
+    public function __construct(
+        AbstractMySQLPlatform $platform,
+        CharsetMetadataProvider $charsetMetadataProvider,
+        CollationMetadataProvider $collationMetadataProvider,
+        DefaultTableOptions $defaultTableOptions
+    ) {
         parent::__construct($platform);
 
+        $this->charsetMetadataProvider   = $charsetMetadataProvider;
         $this->collationMetadataProvider = $collationMetadataProvider;
+        $this->defaultTableOptions       = $defaultTableOptions;
     }
 
     public function diffTable(Table $fromTable, Table $toTable): ?TableDiff
     {
         return parent::diffTable(
-            $this->normalizeColumns($fromTable),
-            $this->normalizeColumns($toTable)
+            $this->normalizeTable($fromTable),
+            $this->normalizeTable($toTable)
         );
     }
 
-    private function normalizeColumns(Table $table): Table
+    private function normalizeTable(Table $table): Table
     {
-        $tableOptions = array_intersect_key($table->getOptions(), [
-            'charset'   => null,
-            'collation' => null,
-        ]);
+        $charset   = $table->getOption('charset');
+        $collation = $table->getOption('collation');
+
+        if ($charset === null && $collation !== null) {
+            $charset = $this->collationMetadataProvider->getCollationCharset($collation);
+        } elseif ($charset !== null && $collation === null) {
+            $collation = $this->charsetMetadataProvider->getDefaultCharsetCollation($charset);
+        } elseif ($charset === null && $collation === null) {
+            $charset   = $this->defaultTableOptions->getCharset();
+            $collation = $this->defaultTableOptions->getCollation();
+        }
+
+        $tableOptions = [
+            'charset'   => $charset,
+            'collation' => $collation,
+        ];
 
         $table = clone $table;
 
@@ -69,16 +90,14 @@ private function normalizeColumns(Table $table): Table
     /**
      * @param array<string,string> $options
      *
-     * @return array<string,string>
+     * @return array<string,string|null>
      */
     private function normalizeOptions(array $options): array
     {
-        if (isset($options['collation']) && ! isset($options['charset'])) {
-            $charset = $this->collationMetadataProvider->getCollationCharset($options['collation']);
-
-            if ($charset !== null) {
-                $options['charset'] = $charset;
-            }
+        if (isset($options['charset']) && ! isset($options['collation'])) {
+            $options['collation'] = $this->charsetMetadataProvider->getDefaultCharsetCollation($options['charset']);
+        } elseif (isset($options['collation']) && ! isset($options['charset'])) {
+            $options['charset'] = $this->collationMetadataProvider->getCollationCharset($options['collation']);
         }
 
         return $options;

diff --git a/src/Platforms/MySQL/DefaultTableOptions.php b/src/Platforms/MySQL/DefaultTableOptions.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Platforms\MySQL;
+
+/**
+ * @internal
+ */
+final class DefaultTableOptions
+{
+    private string $charset;
+
+    private string $collation;
+
+    public function __construct(string $charset, string $collation)
+    {
+        $this->charset   = $charset;
+        $this->collation = $collation;
+    }
+
+    public function getCharset(): string
+    {
+        return $this->charset;
+    }
+
+    public function getCollation(): string
+    {
+        return $this->collation;
+    }
+}
diff --git a/src/Schema/MySQLSchemaManager.php b/src/Schema/MySQLSchemaManager.php
@@ -4,11 +4,15 @@
 
 namespace Doctrine\DBAL\Schema;
 
+use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
 use Doctrine\DBAL\Platforms\MariaDBPlatform;
 use Doctrine\DBAL\Platforms\MySQL;
+use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider\CachingCharsetMetadataProvider;
+use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider\ConnectionCharsetMetadataProvider;
 use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider\CachingCollationMetadataProvider;
 use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider\ConnectionCollationMetadataProvider;
+use Doctrine\DBAL\Platforms\MySQL\DefaultTableOptions;
 use Doctrine\DBAL\Result;
 use Doctrine\DBAL\Types\Type;
 
@@ -52,6 +56,8 @@ class MySQLSchemaManager extends AbstractSchemaManager
         "''" => "'",
     ];
 
+    private ?DefaultTableOptions $defaultTableOptions = null;
+
     /**
      * {@inheritdoc}
      */
@@ -315,13 +321,20 @@ protected function _getPortableTableForeignKeyDefinition(array $tableForeignKey)
         );
     }
 
+    /**
+     * @throws Exception
+     */
     public function createComparator(): Comparator
     {
         return new MySQL\Comparator(
             $this->_platform,
+            new CachingCharsetMetadataProvider(
+                new ConnectionCharsetMetadataProvider($this->_conn)
+            ),
             new CachingCollationMetadataProvider(
                 new ConnectionCollationMetadataProvider($this->_conn)
-            )
+            ),
+            $this->getDefaultTableOptions()
         );
     }
 
@@ -510,4 +523,22 @@ private function parseCreateOptions(?string $string): array
 
         return $options;
     }
+
+    /**
+     * @throws Exception
+     */
+    private function getDefaultTableOptions(): DefaultTableOptions
+    {
+        if ($this->defaultTableOptions === null) {
+            $row = $this->_conn->fetchNumeric(
+                'SELECT @@character_set_database, @@collation_database',
+            );
+
+            assert($row !== false);
+
+            $this->defaultTableOptions = new DefaultTableOptions(...$row);
+        }
+
+        return $this->defaultTableOptions;
+    }
 }
diff --git a/src/Schema/Table.php b/src/Schema/Table.php
@@ -598,7 +598,7 @@ public function hasOption(string $name): bool
 
     public function getOption(string $name): mixed
     {
-        return $this->_options[$name];
+        return $this->_options[$name] ?? null;
     }
 
     /**

diff --git a/tests/Functional/Schema/MySQL/ComparatorTest.php b/tests/Functional/Schema/MySQL/ComparatorTest.php
@@ -136,13 +136,20 @@ public function testChangeColumnCollation(): void
         ComparatorTestUtils::assertDiffNotEmpty($this->connection, $this->comparator, $table);
     }
 
-    public function testImplicitColumnCharset(): void
+    /**
+     * @param array<string,string> $tableOptions
+     * @param array<string,string> $columnOptions
+     *
+     * @dataProvider tableAndColumnOptionsProvider
+     */
+    public function testTableAndColumnOptions(array $tableOptions, array $columnOptions): void
     {
-        $table = new Table('comparator_test');
+        $table = new Table('comparator_test', [], [], [], [], $tableOptions);
         $table->addColumn('name', Types::STRING, [
             'length' => 32,
-            'platformOptions' => ['collation' => 'ascii_general_ci'],
+            'platformOptions' => $columnOptions,
         ]);
+
         $this->dropAndCreateTable($table);
 
         self::assertNull(ComparatorTestUtils::diffFromActualToDesiredTable(
@@ -158,6 +165,27 @@ public function testImplicitColumnCharset(): void
         ));
     }
 
+    /**
+     * @return iterable<string,array{array<string,string>,array<string,string>}>
+     */
+    public static function tableAndColumnOptionsProvider(): iterable
+    {
+        yield "Column collation explicitly set to its table's default" => [
+            [],
+            ['collation' => 'utf8mb4_unicode_ci'],
+        ];
+
+        yield "Column charset implicitly set to a value matching its table's charset" => [
+            ['charset' => 'utf8mb4'],
+            ['collation' => 'utf8mb4_unicode_ci'],
+        ];
+
+        yield "Column collation reset to the collation's default matching its table's charset" => [
+            ['collation' => 'utf8mb4_unicode_ci'],
+            ['charset' => 'utf8mb4'],
+        ];
+    }
+
     /**
      * @return array{Table,Column}
      *

diff --git a/tests/Functional/Schema/MySQLSchemaManagerTest.php b/tests/Functional/Schema/MySQLSchemaManagerTest.php
@@ -246,7 +246,7 @@ public function testColumnCharsetChange(): void
 
         self::assertContains(
             'ALTER TABLE test_column_charset_change CHANGE col_string'
-                . ' col_string VARCHAR(100) CHARACTER SET ascii NOT NULL',
+                . ' col_string VARCHAR(100) CHARACTER SET ascii NOT NULL COLLATE `ascii_general_ci`',
             $diff
         );
     }

diff --git a/tests/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Platforms/AbstractMySQLPlatformTestCase.php
@@ -7,7 +7,9 @@
 use Doctrine\DBAL\Exception\ColumnLengthRequired;
 use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
 use Doctrine\DBAL\Platforms\MySQL;
+use Doctrine\DBAL\Platforms\MySQL\CharsetMetadataProvider;
 use Doctrine\DBAL\Platforms\MySQL\CollationMetadataProvider;
+use Doctrine\DBAL\Platforms\MySQL\DefaultTableOptions;
 use Doctrine\DBAL\Schema\Comparator;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Table;
@@ -808,7 +810,9 @@ protected function createComparator(): Comparator
     {
         return new MySQL\Comparator(
             $this->platform,
-            $this->createStub(CollationMetadataProvider::class)
+            $this->createStub(CharsetMetadataProvider::class),
+            $this->createStub(CollationMetadataProvider::class),
+            new DefaultTableOptions('utf8mb4', 'utf8mb4_general_ci')
         );
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -598,7 +598,7 @@ public function hasOption(string $name): bool @@
         public function getOption(string $name): mixed
         {
-            return $this->_options[$name];
+            return $this->_options[$name] ?? null;
         }
         /**
@@ Expand Down @@