Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop type comments entirely #5107

Merged
merged 1 commit into from
Jan 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 0 additions & 39 deletions docs/en/explanation/dc2type-comments.rst

This file was deleted.

1 change: 0 additions & 1 deletion docs/en/sidebar.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,4 @@
reference/upgrading
reference/testing

explanation/dc2type-comments.rst
explanation/implicit-indexes
6 changes: 3 additions & 3 deletions src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
32 changes: 3 additions & 29 deletions src/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
]);
}

Expand Down Expand Up @@ -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();
}
}
4 changes: 2 additions & 2 deletions src/Platforms/DB2Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ public function getAlterTableSQL(TableDiff $diff): array

$queryParts[] = $queryPart;

$comment = $this->getColumnComment($column);
$comment = $column->getComment();

if ($comment === '') {
continue;
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/Platforms/OraclePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/Platforms/PostgreSQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions src/Platforms/SQLServerPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ public function getAlterTableSQL(TableDiff $diff): array

$queryParts[] = $addColumnSql;

$comment = $this->getColumnComment($column);
$comment = $column->getComment();

if ($comment === '') {
continue;
Expand All @@ -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) {
Expand Down
18 changes: 0 additions & 18 deletions src/Schema/AbstractSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
use function array_values;
use function assert;
use function count;
use function preg_match;
use function strtolower;

/**
Expand Down Expand Up @@ -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
*/
Expand Down
3 changes: 1 addition & 2 deletions src/Schema/DB2SchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
3 changes: 1 addition & 2 deletions src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
3 changes: 1 addition & 2 deletions src/Schema/OracleSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
3 changes: 1 addition & 2 deletions src/Schema/PostgreSQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
3 changes: 1 addition & 2 deletions src/Schema/SQLServerSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions src/Schema/SqliteSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
12 changes: 0 additions & 12 deletions tests/Functional/Schema/MySQLSchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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();
Expand Down
27 changes: 0 additions & 27 deletions tests/Functional/Schema/SchemaManagerFunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down
Loading