Skip to content

Mark ColumnDiff public properties as internal #5657

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

Merged
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
5 changes: 5 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ The following `Comparator` methods have been marked as internal:

The `diffColumn()` method has been deprecated. Use `diffTable()` instead.

## Marked `ColumnDiff` public properties as internal.

The `$fromColumn` and `$column` properties of the `ColumnDiff` class have been marked as internal. Use the
`getOldColumn()` and `getNewColumn()` methods instead.

## Deprecated `ColumnDiff::$changedProperties` and `::hasChanged()`.

The `ColumnDiff::$changedProperties` property and the `hasChanged()` method have been deprecated. Use one of the
Expand Down
37 changes: 20 additions & 17 deletions src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -618,11 +618,14 @@ public function getAlterTableSQL(TableDiff $diff)
continue;
}

$columnArray = array_merge($column->toArray(), [
$columnProperties = array_merge($column->toArray(), [
'comment' => $this->getColumnComment($column),
]);

$queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
$queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL(
$column->getQuotedName($this),
$columnProperties,
);
}

foreach ($diff->removedColumns as $column) {
Expand All @@ -638,31 +641,31 @@ public function getAlterTableSQL(TableDiff $diff)
continue;
}

$column = $columnDiff->column;
$columnArray = $column->toArray();
$newColumn = $columnDiff->getNewColumn();

$columnArray['comment'] = $this->getColumnComment($column);
$newColumnProperties = array_merge($newColumn->toArray(), [
'comment' => $this->getColumnComment($newColumn),
]);

if ($columnDiff->fromColumn !== null) {
$fromColumn = $columnDiff->fromColumn;
} else {
$fromColumn = $columnDiff->getOldColumnName();
}
$oldColumn = $columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName();

$queryParts[] = 'CHANGE ' . $fromColumn->getQuotedName($this) . ' '
. $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
$queryParts[] = 'CHANGE ' . $oldColumn->getQuotedName($this) . ' '
. $this->getColumnDeclarationSQL($newColumn->getQuotedName($this), $newColumnProperties);
}

foreach ($diff->renamedColumns as $oldColumnName => $column) {
if ($this->onSchemaAlterTableRenameColumn($oldColumnName, $column, $diff, $columnSql)) {
continue;
}

$oldColumnName = new Identifier($oldColumnName);
$columnArray = $column->toArray();
$columnArray['comment'] = $this->getColumnComment($column);
$queryParts[] = 'CHANGE ' . $oldColumnName->getQuotedName($this) . ' '
. $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
$oldColumnName = new Identifier($oldColumnName);

$columnProperties = array_merge($column->toArray(), [
'comment' => $this->getColumnComment($column),
]);

$queryParts[] = 'CHANGE ' . $oldColumnName->getQuotedName($this) . ' '
. $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnProperties);
}

if (isset($diff->addedIndexes['primary'])) {
Expand Down
20 changes: 10 additions & 10 deletions src/Platforms/DB2Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,8 @@ public function getAlterTableSQL(TableDiff $diff)
if ($columnDiff->hasCommentChanged()) {
$commentsSQL[] = $this->getCommentOnColumnSQL(
$diff->getName($this)->getQuotedName($this),
$columnDiff->column->getQuotedName($this),
$this->getColumnComment($columnDiff->column),
$columnDiff->getNewColumn()->getQuotedName($this),
$this->getColumnComment($columnDiff->getNewColumn()),
);
}

Expand Down Expand Up @@ -702,12 +702,12 @@ private function gatherAlterColumnSQL(
*/
private function getAlterColumnClausesSQL(ColumnDiff $columnDiff): array
{
$column = $columnDiff->column->toArray();
$newColumn = $columnDiff->getNewColumn()->toArray();

$alterClause = 'ALTER COLUMN ' . $columnDiff->column->getQuotedName($this);
$alterClause = 'ALTER COLUMN ' . $columnDiff->getNewColumn()->getQuotedName($this);

if ($column['columnDefinition'] !== null) {
return [$alterClause . ' ' . $column['columnDefinition']];
if ($newColumn['columnDefinition'] !== null) {
return [$alterClause . ' ' . $newColumn['columnDefinition']];
}

$clauses = [];
Expand All @@ -719,16 +719,16 @@ private function getAlterColumnClausesSQL(ColumnDiff $columnDiff): array
$columnDiff->hasScaleChanged() ||
$columnDiff->hasFixedChanged()
) {
$clauses[] = $alterClause . ' SET DATA TYPE ' . $column['type']->getSQLDeclaration($column, $this);
$clauses[] = $alterClause . ' SET DATA TYPE ' . $newColumn['type']->getSQLDeclaration($newColumn, $this);
}

if ($columnDiff->hasNotNullChanged()) {
$clauses[] = $column['notnull'] ? $alterClause . ' SET NOT NULL' : $alterClause . ' DROP NOT NULL';
$clauses[] = $newColumn['notnull'] ? $alterClause . ' SET NOT NULL' : $alterClause . ' DROP NOT NULL';
}

if ($columnDiff->hasDefaultChanged()) {
if (isset($column['default'])) {
$defaultClause = $this->getDefaultValueDeclarationSQL($column);
if (isset($newColumn['default'])) {
$defaultClause = $this->getDefaultValueDeclarationSQL($newColumn);

if ($defaultClause !== '') {
$clauses[] = $alterClause . ' SET' . $defaultClause;
Expand Down
14 changes: 7 additions & 7 deletions src/Platforms/OraclePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -900,13 +900,13 @@ public function getAlterTableSQL(TableDiff $diff)
continue;
}

$column = $columnDiff->column;
$newColumn = $columnDiff->getNewColumn();

// Do not generate column alteration clause if type is binary and only fixed property has changed.
// Oracle only supports binary type columns with variable length.
// Avoids unnecessary table alteration statements.
if (
$column->getType() instanceof BinaryType &&
$newColumn->getType() instanceof BinaryType &&
$columnDiff->hasFixedChanged() &&
count($columnDiff->changedProperties) === 1
) {
Expand All @@ -919,13 +919,13 @@ public function getAlterTableSQL(TableDiff $diff)
* Do not add query part if only comment has changed
*/
if (! ($columnHasChangedComment && count($columnDiff->changedProperties) === 1)) {
$columnInfo = $column->toArray();
$newColumnProperties = $newColumn->toArray();

if (! $columnDiff->hasNotNullChanged()) {
unset($columnInfo['notnull']);
unset($newColumnProperties['notnull']);
}

$fields[] = $column->getQuotedName($this) . $this->getColumnDeclarationSQL('', $columnInfo);
$fields[] = $newColumn->getQuotedName($this) . $this->getColumnDeclarationSQL('', $newColumnProperties);
}

if (! $columnHasChangedComment) {
Expand All @@ -934,8 +934,8 @@ public function getAlterTableSQL(TableDiff $diff)

$commentsSQL[] = $this->getCommentOnColumnSQL(
$diff->getName($this)->getQuotedName($this),
$column->getQuotedName($this),
$this->getColumnComment($column),
$newColumn->getQuotedName($this),
$this->getColumnComment($newColumn),
);
}

Expand Down
76 changes: 39 additions & 37 deletions src/Platforms/PostgreSQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -522,33 +522,33 @@ public function getAlterTableSQL(TableDiff $diff)
$commentsSQL = [];
$columnSql = [];

foreach ($diff->addedColumns as $column) {
if ($this->onSchemaAlterTableAddColumn($column, $diff, $columnSql)) {
foreach ($diff->addedColumns as $newColumn) {
if ($this->onSchemaAlterTableAddColumn($newColumn, $diff, $columnSql)) {
continue;
}

$query = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
$query = 'ADD ' . $this->getColumnDeclarationSQL($newColumn->getQuotedName($this), $newColumn->toArray());
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;

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

if ($comment === null || $comment === '') {
continue;
}

$commentsSQL[] = $this->getCommentOnColumnSQL(
$diff->getName($this)->getQuotedName($this),
$column->getQuotedName($this),
$newColumn->getQuotedName($this),
$comment,
);
}

foreach ($diff->removedColumns as $column) {
if ($this->onSchemaAlterTableRemoveColumn($column, $diff, $columnSql)) {
foreach ($diff->removedColumns as $newColumn) {
if ($this->onSchemaAlterTableRemoveColumn($newColumn, $diff, $columnSql)) {
continue;
}

$query = 'DROP ' . $column->getQuotedName($this);
$query = 'DROP ' . $newColumn->getQuotedName($this);
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
}

Expand All @@ -561,26 +561,21 @@ public function getAlterTableSQL(TableDiff $diff)
continue;
}

if ($columnDiff->fromColumn !== null) {
$fromColumn = $columnDiff->fromColumn;
} else {
$fromColumn = $columnDiff->getOldColumnName();
}

$oldColumnName = $fromColumn->getQuotedName($this);
$oldColumn = $columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName();
$newColumn = $columnDiff->getNewColumn();

$column = $columnDiff->column;
$oldColumnName = $oldColumn->getQuotedName($this);

if (
$columnDiff->hasTypeChanged()
|| $columnDiff->hasPrecisionChanged()
|| $columnDiff->hasScaleChanged()
|| $columnDiff->hasFixedChanged()
) {
$type = $column->getType();
$type = $newColumn->getType();

// SERIAL/BIGSERIAL are not "real" types and we can't alter a column to that type
$columnDefinition = $column->toArray();
$columnDefinition = $newColumn->toArray();
$columnDefinition['autoincrement'] = false;

// here was a server version check before, but DBAL API does not support this anymore.
Expand All @@ -589,45 +584,46 @@ public function getAlterTableSQL(TableDiff $diff)
}

if ($columnDiff->hasDefaultChanged()) {
$defaultClause = $column->getDefault() === null
$defaultClause = $newColumn->getDefault() === null
? ' DROP DEFAULT'
: ' SET' . $this->getDefaultValueDeclarationSQL($column->toArray());
$query = 'ALTER ' . $oldColumnName . $defaultClause;
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
: ' SET' . $this->getDefaultValueDeclarationSQL($newColumn->toArray());

$query = 'ALTER ' . $oldColumnName . $defaultClause;
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
}

if ($columnDiff->hasNotNullChanged()) {
$query = 'ALTER ' . $oldColumnName . ' ' . ($column->getNotnull() ? 'SET' : 'DROP') . ' NOT NULL';
$query = 'ALTER ' . $oldColumnName . ' ' . ($newColumn->getNotnull() ? 'SET' : 'DROP') . ' NOT NULL';
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
}

if ($columnDiff->hasAutoIncrementChanged()) {
if ($column->getAutoincrement()) {
if ($newColumn->getAutoincrement()) {
// add autoincrement
$seqName = $this->getIdentitySequenceName($diff->name, $oldColumnName);

$sql[] = 'CREATE SEQUENCE ' . $seqName;
$sql[] = "SELECT setval('" . $seqName . "', (SELECT MAX(" . $oldColumnName . ') FROM '
. $diff->getName($this)->getQuotedName($this) . '))';
$query = 'ALTER ' . $oldColumnName . " SET DEFAULT nextval('" . $seqName . "')";
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
} else {
// Drop autoincrement, but do NOT drop the sequence. It might be re-used by other tables or have
$query = 'ALTER ' . $oldColumnName . ' DROP DEFAULT';
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
}

$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
}

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

if (
$columnDiff->hasCommentChanged()
|| ($columnDiff->fromColumn !== null && $oldComment !== $newComment)
|| ($columnDiff->getOldColumn() !== null && $oldComment !== $newComment)
) {
$commentsSQL[] = $this->getCommentOnColumnSQL(
$diff->getName($this)->getQuotedName($this),
$column->getQuotedName($this),
$newColumn->getQuotedName($this),
$newComment,
);
}
Expand All @@ -637,7 +633,7 @@ public function getAlterTableSQL(TableDiff $diff)
}

$query = 'ALTER ' . $oldColumnName . ' TYPE '
. $column->getType()->getSQLDeclaration($column->toArray(), $this);
. $newColumn->getType()->getSQLDeclaration($newColumn->toArray(), $this);
$sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
}

Expand Down Expand Up @@ -688,18 +684,18 @@ public function getAlterTableSQL(TableDiff $diff)
*/
private function isUnchangedBinaryColumn(ColumnDiff $columnDiff): bool
{
$columnType = $columnDiff->column->getType();
$newColumnType = $columnDiff->getNewColumn()->getType();

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

$fromColumn = $columnDiff->fromColumn instanceof Column ? $columnDiff->fromColumn : null;
$oldColumn = $columnDiff->getOldColumn() instanceof Column ? $columnDiff->getOldColumn() : null;

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

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

Expand Down Expand Up @@ -1326,7 +1322,13 @@ public function getJsonTypeDeclarationSQL(array $column)

private function getOldColumnComment(ColumnDiff $columnDiff): ?string
{
return $columnDiff->fromColumn !== null ? $this->getColumnComment($columnDiff->fromColumn) : null;
$oldColumn = $columnDiff->getOldColumn();

if ($oldColumn !== null) {
return $this->getColumnComment($oldColumn);
}

return null;
}

/** @deprecated The SQL used for schema introspection is an implementation detail and should not be relied upon. */
Expand Down
Loading