Skip to content

Commit

Permalink
Revert the rename column feature (#6276)
Browse files Browse the repository at this point in the history 
This PR reverts PR #6080 and its follow-up #6273. Merging this feature
up to 4.0 is more difficult that I anticipated and I don't want to
postpone the release of 3.8 and 4.0 any further.

@Tofandel: Please take all the time that you need to redo your PR on
4.x. As soon as you did that, I'd be more than happy to restore this
change and do another 3.x feature release.
  • Loading branch information
@derrabus
derrabus committed Jan 25, 2024
1 parent 06d9e9f commit d244f2e
Show file tree
Hide file tree
Showing 25 changed files with 274 additions and 718 deletions.
7 changes: 0 additions & 7 deletions psalm.xml.dist
View file
Expand Up @@ -517,13 +517,6 @@
<!-- TODO for PHPUnit 10 -->
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::withConsecutive"/>

<!--
TODO: remove in 4.0.0
See https://github.com/doctrine/dbal/pull/6080
-->
<referencedMethod name="Doctrine\DBAL\Schema\TableDiff::getModifiedColumns"/>
<referencedMethod name="Doctrine\DBAL\Schema\TableDiff::getRenamedColumns"/>

<!-- TODO: remove in 4.0.0 -->
<referencedMethod name="Doctrine\DBAL\Platforms\DB2Platform::getForUpdateSQL"/>
</errorLevel>
Expand Down
34 changes: 23 additions & 11 deletions src/Platforms/AbstractMySQLPlatform.php
View file
Expand Up @@ -6,6 +6,7 @@
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Schema\AbstractAsset;
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
use Doctrine\DBAL\Schema\Identifier;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\MySQLSchemaManager;
use Doctrine\DBAL\Schema\Table;
Expand Down Expand Up @@ -683,26 +684,36 @@ public function getAlterTableSQL(TableDiff $diff)
$queryParts[] = 'DROP ' . $column->getQuotedName($this);
}

foreach ($diff->getChangedColumns() as $columnDiff) {
foreach ($diff->getModifiedColumns() as $columnDiff) {
if ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
continue;
}

$newColumn = $columnDiff->getNewColumn();

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

$oldColumn = $columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName();
$oldColumnName = $oldColumn->getName();
$oldColumn = $columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName();

if ($columnDiff->hasNameChanged()) {
if ($this->onSchemaAlterTableRenameColumn($oldColumnName, $newColumn, $diff, $columnSql)) {
continue;
}
} elseif ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
$queryParts[] = 'CHANGE ' . $oldColumn->getQuotedName($this) . ' '
. $this->getColumnDeclarationSQL($newColumn->getQuotedName($this), $newColumnProperties);
}

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

$queryParts[] = 'CHANGE ' . $oldColumn->getQuotedName($this) . ' '
. $this->getColumnDeclarationSQL($newColumn->getQuotedName($this), $newColumnProperties);
$oldColumnName = new Identifier($oldColumnName);

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

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

$addedIndexes = $this->indexAssetsByLowerCaseName($diff->getAddedIndexes());
Expand Down Expand Up @@ -734,7 +745,7 @@ public function getAlterTableSQL(TableDiff $diff)
$diff = new TableDiff(
$diff->name,
$diff->getAddedColumns(),
$diff->getChangedColumns(),
$diff->getModifiedColumns(),
$diff->getDroppedColumns(),
array_values($addedIndexes),
array_values($modifiedIndexes),
Expand All @@ -743,6 +754,7 @@ public function getAlterTableSQL(TableDiff $diff)
$diff->getAddedForeignKeys(),
$diff->getModifiedForeignKeys(),
$diff->getDroppedForeignKeys(),
$diff->getRenamedColumns(),
$diff->getRenamedIndexes(),
);
}
Expand Down
14 changes: 0 additions & 14 deletions src/Platforms/AbstractPlatform.php
View file
Expand Up @@ -2940,20 +2940,6 @@ protected function getRenameIndexSQL($oldIndexName, Index $index, $tableName)
];
}

/**
* Returns the SQL for renaming a column
*
* @param string $tableName The table to rename the column on.
* @param string $oldColumnName The name of the column we want to rename.
* @param string $newColumnName The name we should rename it to.
*
* @return string[] The sequence of SQL statements for renaming the given column.
*/
protected function getRenameColumnSQL(string $tableName, string $oldColumnName, string $newColumnName): array
{
return [sprintf('ALTER TABLE %s RENAME COLUMN %s TO %s', $tableName, $oldColumnName, $newColumnName)];
}

/**
* Gets declaration of a number of columns in bulk.
*
Expand Down
65 changes: 24 additions & 41 deletions src/Platforms/DB2Platform.php
View file
Expand Up @@ -591,8 +591,6 @@ public function getAlterTableSQL(TableDiff $diff)
$tableNameSQL = ($diff->getOldTable() ?? $diff->getName($this))->getQuotedName($this);

$queryParts = [];
$needsReorg = false;

foreach ($diff->getAddedColumns() as $column) {
if ($this->onSchemaAlterTableAddColumn($column, $diff, $columnSql)) {
continue;
Expand All @@ -612,8 +610,7 @@ public function getAlterTableSQL(TableDiff $diff)

$queryParts[] = $queryPart;

$comment = $this->getColumnComment($column);
$needsReorg = true;
$comment = $this->getColumnComment($column);

if ($comment === null || $comment === '') {
continue;
Expand All @@ -632,20 +629,10 @@ public function getAlterTableSQL(TableDiff $diff)
}

$queryParts[] = 'DROP COLUMN ' . $column->getQuotedName($this);
$needsReorg = true;
}

foreach ($diff->getChangedColumns() as $columnDiff) {
$newColumn = $columnDiff->getNewColumn();
$oldColumn = $columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName();

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

if ($columnDiff->hasNameChanged()) {
if ($this->onSchemaAlterTableRenameColumn($oldColumnName, $newColumn, $diff, $columnSql)) {
continue;
}
} elseif ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
foreach ($diff->getModifiedColumns() as $columnDiff) {
if ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
continue;
}

Expand All @@ -662,10 +649,20 @@ public function getAlterTableSQL(TableDiff $diff)
$columnDiff,
$sql,
$queryParts,
$needsReorg,
);
}

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

$oldColumnName = new Identifier($oldColumnName);

$queryParts[] = 'RENAME COLUMN ' . $oldColumnName->getQuotedName($this) .
' TO ' . $column->getQuotedName($this);
}

$tableSql = [];

if (! $this->onSchemaAlterTable($diff, $tableSql)) {
Expand All @@ -674,7 +671,7 @@ public function getAlterTableSQL(TableDiff $diff)
}

// Some table alteration operations require a table reorganization.
if ($needsReorg) {
if (count($diff->getDroppedColumns()) > 0 || count($diff->getModifiedColumns()) > 0) {
$sql[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE " . $tableNameSQL . "')";
}

Expand Down Expand Up @@ -729,12 +726,11 @@ private function gatherAlterColumnSQL(
string $table,
ColumnDiff $columnDiff,
array &$sql,
array &$queryParts,
bool &$needsReorg
array &$queryParts
): void {
$alterColumnClauses = $this->getAlterColumnClausesSQL($columnDiff, $needsReorg);
$alterColumnClauses = $this->getAlterColumnClausesSQL($columnDiff);

if (count($alterColumnClauses) < 1) {
if (empty($alterColumnClauses)) {
return;
}

Expand All @@ -758,54 +754,41 @@ private function gatherAlterColumnSQL(
*
* @return string[]
*/
private function getAlterColumnClausesSQL(ColumnDiff $columnDiff, bool &$needsReorg): array
private function getAlterColumnClausesSQL(ColumnDiff $columnDiff): array
{
$newColumn = $columnDiff->getNewColumn()->toArray();

$newName = $columnDiff->getNewColumn()->getQuotedName($this);
$oldName = ($columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName())->getQuotedName($this);

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

if ($newColumn['columnDefinition'] !== null) {
$needsReorg = true;

return [$alterClause . ' ' . $newColumn['columnDefinition']];
}

$clauses = [];

if ($columnDiff->hasNameChanged()) {
$clauses[] = 'RENAME COLUMN ' . $oldName . ' TO ' . $newName;
}

if (
$columnDiff->hasTypeChanged() ||
$columnDiff->hasLengthChanged() ||
$columnDiff->hasPrecisionChanged() ||
$columnDiff->hasScaleChanged() ||
$columnDiff->hasFixedChanged()
) {
$needsReorg = true;
$clauses[] = $alterClause . ' SET DATA TYPE ' . $newColumn['type']->getSQLDeclaration($newColumn, $this);
$clauses[] = $alterClause . ' SET DATA TYPE ' . $newColumn['type']->getSQLDeclaration($newColumn, $this);
}

if ($columnDiff->hasNotNullChanged()) {
$needsReorg = true;
$clauses[] = $newColumn['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($newColumn['default'])) {
$defaultClause = $this->getDefaultValueDeclarationSQL($newColumn);

if ($defaultClause !== '') {
$needsReorg = true;
$clauses[] = $alterClause . ' SET' . $defaultClause;
$clauses[] = $alterClause . ' SET' . $defaultClause;
}
} else {
$needsReorg = true;
$clauses[] = $alterClause . ' DROP DEFAULT';
$clauses[] = $alterClause . ' DROP DEFAULT';
}
}

Expand Down
35 changes: 16 additions & 19 deletions src/Platforms/OraclePlatform.php
View file
Expand Up @@ -897,27 +897,13 @@ public function getAlterTableSQL(TableDiff $diff)
}

$fields = [];
foreach ($diff->getChangedColumns() as $columnDiff) {
$newColumn = $columnDiff->getNewColumn();
$newColumnName = $newColumn->getQuotedName($this);
$oldColumn = $columnDiff->getOldColumn() ?? $columnDiff->getOldColumnName();

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

// Column names in Oracle are case insensitive and automatically uppercased on the server.
if ($columnDiff->hasNameChanged()) {
if ($this->onSchemaAlterTableRenameColumn($oldColumnName, $newColumn, $diff, $columnSql)) {
continue;
}

$sql = array_merge(
$sql,
$this->getRenameColumnSQL($tableNameSQL, $oldColumnName, $newColumnName),
);
} elseif ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
foreach ($diff->getModifiedColumns() as $columnDiff) {
if ($this->onSchemaAlterTableChangeColumn($columnDiff, $diff, $columnSql)) {
continue;
}

$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.
Expand All @@ -934,7 +920,7 @@ public function getAlterTableSQL(TableDiff $diff)
/**
* Do not add query part if only comment has changed
*/
if (count($columnDiff->changedProperties) > ($columnHasChangedComment ? 1 : 0)) {
if (! ($columnHasChangedComment && count($columnDiff->changedProperties) === 1)) {
$newColumnProperties = $newColumn->toArray();

if (! $columnDiff->hasNotNullChanged()) {
Expand All @@ -959,6 +945,17 @@ public function getAlterTableSQL(TableDiff $diff)
$sql[] = 'ALTER TABLE ' . $tableNameSQL . ' MODIFY (' . implode(', ', $fields) . ')';
}

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

$oldColumnName = new Identifier($oldColumnName);

$sql[] = 'ALTER TABLE ' . $tableNameSQL . ' RENAME COLUMN ' . $oldColumnName->getQuotedName($this)
. ' TO ' . $column->getQuotedName($this);
}

$fields = [];
foreach ($diff->getDroppedColumns() as $column) {
if ($this->onSchemaAlterTableRemoveColumn($column, $diff, $columnSql)) {
Expand Down

0 comments on commit d244f2e

Please sign in to comment.