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.

Sign up for GitHub

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

Require the original column to be set on the column diff #4786

Merged
merged 1 commit into from
Sep 11, 2021
Merged

Require the original column to be set on the column diff #4786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions UPGRADE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ awareness about deprecated code.

# Upgrade to 4.0

## BC BREAK: The `$fromColumn` parameter of the `ColumnDiff` constructor made required

## BC BREAK: Changes in the return value of `Table::getColumns()`

1. The columns are returned as a list, not as an associative array.
Expand Down
36 changes: 5 additions & 31 deletions src/Platforms/PostgreSQLPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,12 +449,9 @@ public function getAlterTableSQL(TableDiff $diff): array
}

$newComment = $this->getColumnComment($column);
$oldComment = $this->getOldColumnComment($columnDiff);
$oldComment = $this->getColumnComment($columnDiff->fromColumn);

if (
$columnDiff->hasChanged('comment')
|| ($columnDiff->fromColumn !== null && $oldComment !== $newComment)
) {
if ($columnDiff->hasChanged('comment') || $oldComment !== $newComment) {
$commentsSQL[] = $this->getCommentOnColumnSQL(
$diff->getName($this)->getQuotedName($this),
$column->getQuotedName($this),
Expand Down Expand Up @@ -528,23 +525,13 @@ private function isUnchangedBinaryColumn(ColumnDiff $columnDiff): bool
return false;
}

$fromColumn = $columnDiff->fromColumn;

if ($fromColumn !== null) {
$fromColumnType = $fromColumn->getType();

if (! $fromColumnType instanceof BinaryType && ! $fromColumnType instanceof BlobType) {
return false;
}

return count(array_diff($columnDiff->changedProperties, ['type', 'length', 'fixed'])) === 0;
}
$fromColumnType = $columnDiff->fromColumn->getType();

if ($columnDiff->hasChanged('type')) {
if (! $fromColumnType instanceof BinaryType && ! $fromColumnType instanceof BlobType) {
return false;
}

return count(array_diff($columnDiff->changedProperties, ['length', 'fixed'])) === 0;
return count(array_diff($columnDiff->changedProperties, ['type', 'length', 'fixed'])) === 0;
}

/**
Expand Down Expand Up @@ -1051,10 +1038,6 @@ private function isSerialColumn(array $column): bool
*/
private function typeChangeBreaksDefaultValue(ColumnDiff $columnDiff): bool
{
if ($columnDiff->fromColumn === null) {
return $columnDiff->hasChanged('type');
}

$oldTypeIsNumeric = $this->isNumericType($columnDiff->fromColumn->getType());
$newTypeIsNumeric = $this->isNumericType($columnDiff->column->getType());

Expand All @@ -1068,15 +1051,6 @@ private function isNumericType(Type $type): bool
return $type instanceof IntegerType || $type instanceof BigIntType;
}

private function getOldColumnComment(ColumnDiff $columnDiff): ?string
{
if ($columnDiff->fromColumn === null) {
return null;
}

return $this->getColumnComment($columnDiff->fromColumn);
}

public function getListTableMetadataSQL(string $table, ?string $schema = null): string
{
if ($schema !== null) {
Expand Down
34 changes: 9 additions & 25 deletions src/Platforms/SQLServerPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,25 +515,15 @@ public function getAlterTableSQL(TableDiff $diff): array
$comment = $this->getColumnComment($column);
$hasComment = $comment !== '';

if ($columnDiff->fromColumn instanceof Column) {
$fromComment = $this->getColumnComment($columnDiff->fromColumn);
$hasFromComment = $fromComment !== '';

if ($hasFromComment && $hasComment && $fromComment !== $comment) {
$commentsSql[] = $this->getAlterColumnCommentSQL(
$diff->name,
$column->getQuotedName($this),
$comment
);
} elseif ($hasFromComment && ! $hasComment) {
$commentsSql[] = $this->getDropColumnCommentSQL($diff->name, $column->getQuotedName($this));
} elseif (! $hasFromComment && $hasComment) {
$commentsSql[] = $this->getCreateColumnCommentSQL(
$diff->name,
$column->getQuotedName($this),
$comment
);
}
$fromComment = $this->getColumnComment($columnDiff->fromColumn);
$hasFromComment = $fromComment !== '';

if ($hasFromComment && $hasComment && $fromComment !== $comment) {
$commentsSql[] = $this->getAlterColumnCommentSQL($diff->name, $column->getQuotedName($this), $comment);
} elseif ($hasFromComment && ! $hasComment) {
$commentsSql[] = $this->getDropColumnCommentSQL($diff->name, $column->getQuotedName($this));
} elseif (! $hasFromComment && $hasComment) {
$commentsSql[] = $this->getCreateColumnCommentSQL($diff->name, $column->getQuotedName($this), $comment);
}

// Do not add query part if only comment has changed.
Expand Down Expand Up @@ -671,12 +661,6 @@ private function getAlterTableDropDefaultConstraintClause(string $tableName, str
*/
private function alterColumnRequiresDropDefaultConstraint(ColumnDiff $columnDiff): bool
{
// We can only decide whether to drop an existing default constraint
// if we know the original default value.
if (! $columnDiff->fromColumn instanceof Column) {
return false;
}

// We only need to drop an existing default constraint if we know the
// column was defined with a default value before.
if ($columnDiff->fromColumn->getDefault() === null) {
Expand Down
1 change: 0 additions & 1 deletion src/Platforms/SqlitePlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,6 @@ private function getSimpleAlterTableSQL(TableDiff $diff)
// Suppress changes on integer type autoincrement columns.
foreach ($diff->changedColumns as $oldColumnName => $columnDiff) {
if (
$columnDiff->fromColumn === null ||
! $columnDiff->column->getAutoincrement() ||
! $columnDiff->column->getType() instanceof Types\IntegerType
) {
Expand Down
12 changes: 5 additions & 7 deletions src/Schema/ColumnDiff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ class ColumnDiff
public Column $column;

/** @var array<int, string> */
public array $changedProperties = [];
public array $changedProperties;

public ?Column $fromColumn = null;
public Column $fromColumn;

/**
* @param array<string> $changedProperties
*/
public function __construct(
string $oldColumnName,
Column $column,
array $changedProperties = [],
?Column $fromColumn = null
array $changedProperties,
Column $fromColumn
) {
$this->oldColumnName = $oldColumnName;
$this->column = $column;
Expand All @@ -42,8 +42,6 @@ public function hasChanged(string $propertyName): bool

public function getOldColumnName(): Identifier
{
$quote = $this->fromColumn !== null && $this->fromColumn->isQuoted();

return new Identifier($this->oldColumnName, $quote);
return new Identifier($this->oldColumnName, $this->fromColumn->isQuoted());
}
}
2 changes: 1 addition & 1 deletion src/Schema/Comparator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public function diffTable(Table $fromTable, Table $toTable): ?TableDiff

$changedProperties = $this->diffColumn($column, $toColumn);

$columnDiff = new ColumnDiff($column->getName(), $toColumn, $changedProperties);
$columnDiff = new ColumnDiff($column->getName(), $toColumn, $changedProperties, $column);

$columnDiff->fromColumn = $column;
$tableDifferences->changedColumns[$column->getName()] = $columnDiff;
Expand Down
37 changes: 19 additions & 18 deletions tests/Platforms/AbstractPlatformTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ public function testGeneratesTableAlterationSql(): void
'default' => 'def',
]
),
['type', 'notnull', 'default']
['type', 'notnull', 'default'],
$table->getColumn('bar')
);

$tableDiff->changedColumns['bloo'] = new ColumnDiff(
Expand All @@ -370,7 +371,8 @@ public function testGeneratesTableAlterationSql(): void
Type::getType('boolean'),
['default' => false]
),
['type', 'notnull', 'default']
['type', 'notnull', 'default'],
$table->getColumn('bloo')
);

$sql = $this->platform->getAlterTableSQL($tableDiff);
Expand Down Expand Up @@ -468,7 +470,9 @@ public function testGetAlterTableSqlDispatchEvent(): void
$tableDiff->removedColumns['removed'] = new Column('removed', Type::getType('integer'), []);
$tableDiff->changedColumns['changed'] = new ColumnDiff(
'changed',
new Column('changed2', Type::getType('string'), ['length' => 255])
new Column('changed2', Type::getType('string'), ['length' => 255]),
[],
$table->getColumn('changed')
);
$tableDiff->renamedColumns['renamed'] = new Column('renamed2', Type::getType('integer'));

Expand All @@ -486,21 +490,16 @@ public function testCreateTableColumnComments(): void

public function testAlterTableColumnComments(): void
{
$foo = new Column('foo', Type::getType('string'), ['length' => 255]);
$bar = new Column('baz', Type::getType('string'), [
'length' => 255,
'comment' => 'B comment',
]);

$tableDiff = new TableDiff('mytable');
$tableDiff->addedColumns['quota'] = new Column('quota', Type::getType('integer'), ['comment' => 'A comment']);
$tableDiff->changedColumns['foo'] = new ColumnDiff(
'foo',
new Column('foo', Type::getType('string'), ['length' => 255]),
['comment']
);
$tableDiff->changedColumns['bar'] = new ColumnDiff(
'bar',
new Column('baz', Type::getType('string'), [
'length' => 255,
'comment' => 'B comment',
]),
['comment']
);
$tableDiff->changedColumns['foo'] = new ColumnDiff('foo', $foo, ['comment'], $foo);
$tableDiff->changedColumns['bar'] = new ColumnDiff('bar', $bar, ['comment'], $bar);

self::assertEquals($this->getAlterTableColumnCommentsSQL(), $this->platform->getAlterTableSQL($tableDiff));
}
Expand Down Expand Up @@ -791,7 +790,8 @@ public function testAlterTableChangeQuotedColumn(): void
Type::getType('string'),
['length' => 255]
),
['type']
['type'],
$table->getColumn('select')
);

self::assertStringContainsString(
Expand Down Expand Up @@ -1440,7 +1440,8 @@ public function testAlterStringToFixedString(): void
Type::getType('string'),
['fixed' => true, 'length' => 2]
),
['fixed']
['fixed'],
$table->getColumn('name')
);

$sql = $this->platform->getAlterTableSQL($tableDiff);
Expand Down
12 changes: 8 additions & 4 deletions tests/Platforms/AbstractPostgreSQLPlatformTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ public function testAlterDecimalPrecisionScale(): void
Type::getType('decimal'),
['precision' => 16, 'scale' => 6]
),
['precision']
['precision'],
$table->getColumn('dfoo1')
);
$tableDiff->changedColumns['dloo2'] = new ColumnDiff(
'dloo2',
Expand All @@ -518,7 +519,8 @@ public function testAlterDecimalPrecisionScale(): void
Type::getType('decimal'),
['precision' => 10, 'scale' => 4]
),
['scale']
['scale'],
$table->getColumn('dfoo2')
);
$tableDiff->changedColumns['dloo3'] = new ColumnDiff(
'dloo3',
Expand All @@ -527,7 +529,8 @@ public function testAlterDecimalPrecisionScale(): void
Type::getType('decimal'),
['precision' => 10, 'scale' => 6]
),
[]
[],
$table->getColumn('dfoo3')
);
$tableDiff->changedColumns['dloo4'] = new ColumnDiff(
'dloo4',
Expand All @@ -536,7 +539,8 @@ public function testAlterDecimalPrecisionScale(): void
Type::getType('decimal'),
['precision' => 16, 'scale' => 8]
),
['precision', 'scale']
['precision', 'scale'],
$table->getColumn('dfoo4')
);

$sql = $this->platform->getAlterTableSQL($tableDiff);
Expand Down
2 changes: 1 addition & 1 deletion tests/Platforms/DB2PlatformTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ public function testGeneratesAlterColumnSQL(
): void {
$tableDiff = new TableDiff('foo');
$tableDiff->fromTable = new Table('foo');
$tableDiff->changedColumns['bar'] = new ColumnDiff('bar', $column, [$changedProperty]);
$tableDiff->changedColumns['bar'] = new ColumnDiff('bar', $column, [$changedProperty], $column);

$expectedSQL = [];

Expand Down
78 changes: 46 additions & 32 deletions tests/Platforms/OraclePlatformTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,45 +474,59 @@ public function testAlterTableNotNULL(): void
{
$tableDiff = new TableDiff('mytable');

$tableDiff->changedColumns['foo'] = new ColumnDiff(
$foo = new Column(
'foo',
new Column(
'foo',
Type::getType('string'),
[
'length' => 255,
'default' => 'bla',
'notnull' => true,
]
),
['type']
Type::getType('string'),
[
'length' => 255,
'default' => 'bla',
'notnull' => true,
]
);

$tableDiff->changedColumns['bar'] = new ColumnDiff(
'bar',
new Column(
'baz',
Type::getType('string'),
[
'length' => 255,
'default' => 'bla',
'notnull' => true,
]
),
['type', 'notnull']
$bar = new Column(
'baz',
Type::getType('string'),
[
'length' => 255,
'default' => 'bla',
'notnull' => true,
]
);

$baz = new Column(
'baz',
Type::getType('string'),
[
'length' => 255,
'default' => 'bla',
'notnull' => true,
]
);

$metar = new Column(
'metar',
Type::getType('string'),
[
'length' => 2000,
'notnull' => false,
]
);

$tableDiff->changedColumns['foo'] = new ColumnDiff(
'foo',
$foo,
['type'],
$foo
);

$tableDiff->changedColumns['bar'] = new ColumnDiff('bar', $bar, ['type', 'notnull'], $baz);

$tableDiff->changedColumns['metar'] = new ColumnDiff(
'metar',
new Column(
'metar',
Type::getType('string'),
[
'length' => 2000,
'notnull' => false,
]
),
['notnull']
$metar,
['notnull'],
$metar
);

$expectedSql = [
Expand Down
Loading