Drop type comments entirely

This feature is only useful for reverse-engineering purposes since we
have platform-aware comparison. Reverse-engineering is not enough to
justify having that feature, and ORM metadata reverse-engineered from
the database can still be adjusted afterwards.

UPGRADE.md

@@ -8,6 +8,14 @@ awareness about deprecated code.
 
 # Upgrade to 4.0
 
+## BC BREAK: Deployed database schema no longer contains the information about abstract data types
+
+Database column comments no longer contain type comments added by DBAL.
+If you use `doctrine/migrations`, it should generate a migration dropping those
+comments from all columns that have them.
+As a consequence, introspecting a table no longer guarantees getting the same
+column types that were used when creating that table.
+
 ## BC BREAK: Removed `AbstractPlatform::prefersIdentityColumns()`
 
 The `AbstractPlatform::prefersIdentityColumns()` method has been removed.

docs/en/sidebar.rst

@@ -20,5 +20,4 @@
     reference/upgrading
     reference/testing
 
-    explanation/dc2type-comments.rst
     explanation/implicit-indexes

src/Platforms/AbstractMySQLPlatform.php

@@ -411,7 +411,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             }
 
             $columnArray = array_merge($column->toArray(), [
-                'comment' => $this->getColumnComment($column),
+                'comment' => $column->getComment(),
             ]);
 
             $queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
@@ -442,7 +442,7 @@ public function getAlterTableSQL(TableDiff $diff): array
                 continue;
             }
 
-            $columnArray['comment'] = $this->getColumnComment($column);
+            $columnArray['comment'] = $column->getComment();
             $queryParts[]           =  'CHANGE ' . ($columnDiff->getOldColumnName()->getQuotedName($this)) . ' '
                 . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
         }
@@ -454,7 +454,7 @@ public function getAlterTableSQL(TableDiff $diff): array
 
             $oldColumnName          = new Identifier($oldColumnName);
             $columnArray            = $column->toArray();
-            $columnArray['comment'] = $this->getColumnComment($column);
+            $columnArray['comment'] = $column->getComment();
             $queryParts[]           =  'CHANGE ' . $oldColumnName->getQuotedName($this) . ' '
                 . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
         }

src/Platforms/AbstractPlatform.php

@@ -375,28 +375,6 @@ public function hasDoctrineTypeMappingFor(string $dbType): bool
         return isset($this->doctrineTypeMapping[$dbType]);
     }
 
-    /**
-     * Gets the comment to append to a column comment that helps parsing this type in reverse engineering.
-     */
-    public function getDoctrineTypeComment(Type $doctrineType): string
-    {
-        return '(DC2Type:' . $doctrineType->getName() . ')';
-    }
-
-    /**
-     * Gets the comment of a passed column modified by potential doctrine type comment hints.
-     */
-    protected function getColumnComment(Column $column): string
-    {
-        $comment = $column->getComment();
-
-        if ($column->getType()->requiresSQLCommentHint($this)) {
-            $comment .= $this->getDoctrineTypeComment($column->getType());
-        }
-
-        return $comment;
-    }
-
     /**
      * Gets the character used for identifier quoting.
      */
@@ -971,7 +949,7 @@ public function getCreateTableSQL(Table $table, int $createFlags = self::CREATE_
             }
 
             foreach ($table->getColumns() as $column) {
-                $comment = $this->getColumnComment($column);
+                $comment = $column->getComment();
 
                 if ($comment === '') {
                     continue;
@@ -2510,7 +2488,7 @@ private function columnToArray(Column $column): array
         return array_merge($column->toArray(), [
             'name' => $column->getQuotedName($this),
             'version' => $column->hasPlatformOption('version') ? $column->getPlatformOption('version') : false,
-            'comment' => $this->getColumnComment($column),
+            'comment' => $column->getComment(),
         ]);
     }
 
@@ -2546,10 +2524,6 @@ public function columnsEqual(Column $column1, Column $column2): bool
             return true;
         }
 
-        if ($column1->getComment() !== $column2->getComment()) {
-            return false;
-        }
-
-        return $column1->getType() === $column2->getType();
+        return $column1->getComment() === $column2->getComment();
     }
 }

src/Platforms/DB2Platform.php

@@ -393,7 +393,7 @@ public function getAlterTableSQL(TableDiff $diff): array
 
             $queryParts[] = $queryPart;
 
-            $comment = $this->getColumnComment($column);
+            $comment = $column->getComment();
 
             if ($comment === '') {
                 continue;
@@ -423,7 +423,7 @@ public function getAlterTableSQL(TableDiff $diff): array
                 $commentsSQL[] = $this->getCommentOnColumnSQL(
                     $diff->getName($this)->getQuotedName($this),
                     $columnDiff->column->getQuotedName($this),
-                    $this->getColumnComment($columnDiff->column)
+                    $columnDiff->column->getComment()
                 );
 
                 if (count($columnDiff->changedProperties) === 1) {

src/Platforms/OraclePlatform.php

@@ -671,7 +671,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             }
 
             $fields[] = $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
-            $comment  = $this->getColumnComment($column);
+            $comment  = $column->getComment();
 
             if ($comment === '') {
                 continue;
@@ -730,7 +730,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             $commentsSQL[] = $this->getCommentOnColumnSQL(
                 $diff->getName($this)->getQuotedName($this),
                 $column->getQuotedName($this),
-                $this->getColumnComment($column)
+                $column->getComment()
             );
         }
 

src/Platforms/PostgreSQLPlatform.php

@@ -355,7 +355,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             $query = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
             $sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
 
-            $comment = $this->getColumnComment($column);
+            $comment = $column->getComment();
 
             if ($comment === '') {
                 continue;
@@ -436,8 +436,8 @@ public function getAlterTableSQL(TableDiff $diff): array
                 }
             }
 
-            $newComment = $this->getColumnComment($column);
-            $oldComment = $this->getColumnComment($columnDiff->fromColumn);
+            $newComment = $column->getComment();
+            $oldComment = $columnDiff->fromColumn->getComment();
 
             if ($columnDiff->hasChanged('comment') || $oldComment !== $newComment) {
                 $commentsSQL[] = $this->getCommentOnColumnSQL(

src/Platforms/SQLServerPlatform.php

@@ -381,7 +381,7 @@ public function getAlterTableSQL(TableDiff $diff): array
 
             $queryParts[] = $addColumnSql;
 
-            $comment = $this->getColumnComment($column);
+            $comment = $column->getComment();
 
             if ($comment === '') {
                 continue;
@@ -408,10 +408,10 @@ public function getAlterTableSQL(TableDiff $diff): array
             }
 
             $column     = $columnDiff->column;
-            $comment    = $this->getColumnComment($column);
+            $comment    = $column->getComment();
             $hasComment = $comment !== '';
 
-            $fromComment    = $this->getColumnComment($columnDiff->fromColumn);
+            $fromComment    = $columnDiff->fromColumn->getComment();
             $hasFromComment = $fromComment !== '';
 
             if ($hasFromComment && $hasComment && $fromComment !== $comment) {

src/Schema/AbstractSchemaManager.php

@@ -20,7 +20,6 @@
 use function array_values;
 use function assert;
 use function count;
-use function preg_match;
 use function strtolower;
 
 /**
@@ -845,23 +844,6 @@ public function createSchemaConfig(): SchemaConfig
         return $schemaConfig;
     }
 
-    /**
-     * Given a table comment this method tries to extract a type hint for Doctrine Type. If the type hint is found,
-     * it's removed from the comment.
-     *
-     * @return string|null The extracted Doctrine type or NULL of the type hint was not found.
-     */
-    final protected function extractDoctrineTypeFromComment(?string &$comment): ?string
-    {
-        if ($comment === null || preg_match('/(.*)\(DC2Type:(((?!\)).)+)\)(.*)/', $comment, $match) === 0) {
-            return null;
-        }
-
-        $comment = $match[1] . $match[4];
-
-        return $match[2];
-    }
-
     /**
      * @throws DatabaseRequired
      */

src/Schema/DB2SchemaManager.php

@@ -61,8 +61,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             }
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($tableColumn['typename']);
+        $type = $this->_platform->getDoctrineTypeMapping($tableColumn['typename']);
 
         switch (strtolower($tableColumn['typename'])) {
             case 'varchar':

src/Schema/MySQLSchemaManager.php

@@ -127,8 +127,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
         $scale     = 0;
         $precision = null;
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         switch ($dbType) {
             case 'char':

src/Schema/OracleSchemaManager.php

@@ -139,8 +139,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $scale = (int) $tableColumn['data_scale'];
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comments'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         switch ($dbType) {
             case 'number':

src/Schema/PostgreSQLSchemaManager.php

@@ -321,8 +321,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $tableColumn['complete_type'] = $tableColumn['domain_complete_type'];
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         switch ($dbType) {
             case 'smallint':

src/Schema/SQLServerSchemaManager.php

@@ -108,8 +108,7 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $fixed = true;
         }
 
-        $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'])
-            ?? $this->_platform->getDoctrineTypeMapping($dbType);
+        $type = $this->_platform->getDoctrineTypeMapping($dbType);
 
         $options = [
             'fixed'         => $fixed,

src/Schema/SqliteSchemaManager.php

@@ -260,12 +260,6 @@ protected function _getPortableTableColumnList(string $table, string $database,
 
             $comment = $this->parseColumnCommentFromSQL($columnName, $createSql);
 
-            $type = $this->extractDoctrineTypeFromComment($comment);
-
-            if ($type !== null) {
-                $column->setType(Type::getType($type));
-            }
-
             $column->setComment($comment);
         }
 

tests/Functional/Schema/MySQLSchemaManagerTest.php

@@ -17,7 +17,6 @@
 use Doctrine\DBAL\Tests\TestUtil;
 use Doctrine\DBAL\Types\BlobType;
 use Doctrine\DBAL\Types\Type;
-use Doctrine\DBAL\Types\Types;
 
 class MySQLSchemaManagerTest extends SchemaManagerFunctionalTestCase
 {
@@ -391,17 +390,6 @@ public function testListFloatTypeColumns(): void
         self::assertTrue($columns['col_unsigned']->getUnsigned());
     }
 
-    public function testJsonColumnType(): void
-    {
-        $table = new Table('test_mysql_json');
-        $table->addColumn('col_json', 'json');
-        $this->dropAndCreateTable($table);
-
-        $columns = $this->schemaManager->listTableColumns('test_mysql_json');
-
-        self::assertSame(Types::JSON, $columns['col_json']->getType()->getName());
-    }
-
     public function testColumnDefaultCurrentTimestamp(): void
     {
         $platform = $this->schemaManager->getDatabasePlatform();

tests/Functional/Schema/SchemaManagerFunctionalTestCase.php

@@ -33,7 +33,6 @@
 use Doctrine\DBAL\Types\TextType;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
-use ReflectionMethod;
 
 use function array_filter;
 use function array_keys;
@@ -1128,32 +1127,6 @@ public function testComparatorShouldNotAddCommentToJsonTypeSinceItIsTheDefaultNo
         self::assertNull($tableDiff);
     }
 
-    /**
-     * @dataProvider commentsProvider
-     */
-    public function testExtractDoctrineTypeFromComment(string $comment, ?string $expectedType): void
-    {
-        $re = new ReflectionMethod($this->schemaManager, 'extractDoctrineTypeFromComment');
-        $re->setAccessible(true);
-
-        self::assertSame($expectedType, $re->invokeArgs($this->schemaManager, [&$comment]));
-    }
-
-    /**
-     * @return mixed[][]
-     */
-    public static function commentsProvider(): iterable
-    {
-        return [
-            'invalid custom type comments'      => ['should.return.null', null],
-            'valid doctrine type'               => ['(DC2Type:guid)', 'guid'],
-            'valid with dots'                   => ['(DC2Type:type.should.return)', 'type.should.return'],
-            'valid with namespace'              => ['(DC2Type:Namespace\Class)', 'Namespace\Class'],
-            'valid with extra closing bracket'  => ['(DC2Type:should.stop)).before)', 'should.stop'],
-            'valid with extra opening brackets' => ['(DC2Type:should((.stop)).before)', 'should((.stop'],
-        ];
-    }
-
     public function testCreateAndListSequences(): void
     {
         if (! $this->schemaManager->getDatabasePlatform()->supportsSequences()) {