From 5678b6d9f43490b61c6182b6975f2b5f14a024ce Mon Sep 17 00:00:00 2001 From: Tofandel Date: Fri, 26 Jan 2024 14:35:14 +0100 Subject: [PATCH] Add explicit renameColumn method (4.x) --- psalm.xml.dist | 1 + src/Platforms/AbstractMySQLPlatform.php | 45 ++++------- src/Platforms/AbstractPlatform.php | 23 ++++-- src/Platforms/DB2Platform.php | 68 +++++++++------- src/Platforms/OraclePlatform.php | 68 ++++++++++------ src/Platforms/PostgreSQLPlatform.php | 48 +++++------- src/Platforms/SQLServerPlatform.php | 75 +++++++++++------- src/Platforms/SQLitePlatform.php | 38 ++------- src/Schema/ColumnDiff.php | 25 ++++++ src/Schema/Comparator.php | 78 ++++++++++++------- src/Schema/SQLiteSchemaManager.php | 4 +- src/Schema/Table.php | 47 +++++++++++ src/Schema/TableDiff.php | 59 +++++++++----- .../Functional/Platform/RenameColumnTest.php | 31 +++++++- tests/Functional/Schema/ComparatorTest.php | 53 +++++++++++++ .../Schema/CustomIntrospectionTest.php | 2 +- .../AbstractMySQLPlatformTestCase.php | 9 +-- tests/Platforms/AbstractPlatformTestCase.php | 57 ++++---------- tests/Platforms/DB2PlatformTest.php | 12 ++- tests/Platforms/SQLServerPlatformTest.php | 32 ++++---- tests/Platforms/SQLitePlatformTest.php | 41 ++++++---- tests/Schema/AbstractComparatorTestCase.php | 13 ++-- tests/Schema/TableTest.php | 47 +++++++++++ 23 files changed, 560 insertions(+), 316 deletions(-) diff --git a/psalm.xml.dist b/psalm.xml.dist index 9fabad3812d..34c2365ac45 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -53,6 +53,7 @@ + diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 770d86341ef..b16d1ab05ec 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -10,7 +10,6 @@ use Doctrine\DBAL\Platforms\Keywords\MySQLKeywords; 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\TableDiff; @@ -335,7 +334,6 @@ private function buildTableOptions(array $options): string */ public function getAlterTableSQL(TableDiff $diff): array { - $columnSql = []; $queryParts = []; foreach ($diff->getAddedColumns() as $column) { @@ -353,7 +351,7 @@ public function getAlterTableSQL(TableDiff $diff): array $queryParts[] = 'DROP ' . $column->getQuotedName($this); } - foreach ($diff->getModifiedColumns() as $columnDiff) { + foreach ($diff->getChangedColumns() as $columnDiff) { $newColumn = $columnDiff->getNewColumn(); $newColumnProperties = array_merge($newColumn->toArray(), [ @@ -366,17 +364,6 @@ public function getAlterTableSQL(TableDiff $diff): array . $this->getColumnDeclarationSQL($newColumn->getQuotedName($this), $newColumnProperties); } - foreach ($diff->getRenamedColumns() as $oldColumnName => $column) { - $oldColumnName = new Identifier($oldColumnName); - - $columnProperties = array_merge($column->toArray(), [ - 'comment' => $column->getComment(), - ]); - - $queryParts[] = 'CHANGE ' . $oldColumnName->getQuotedName($this) . ' ' - . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnProperties); - } - $addedIndexes = $this->indexAssetsByLowerCaseName($diff->getAddedIndexes()); $modifiedIndexes = $this->indexAssetsByLowerCaseName($diff->getModifiedIndexes()); $diffModified = false; @@ -405,35 +392,31 @@ public function getAlterTableSQL(TableDiff $diff): array if ($diffModified) { $diff = new TableDiff( $diff->getOldTable(), - $diff->getAddedColumns(), - $diff->getModifiedColumns(), - $diff->getDroppedColumns(), - $diff->getRenamedColumns(), - array_values($addedIndexes), - array_values($modifiedIndexes), - $diff->getDroppedIndexes(), - $diff->getRenamedIndexes(), - $diff->getAddedForeignKeys(), - $diff->getModifiedForeignKeys(), - $diff->getDroppedForeignKeys(), + addedColumns: $diff->getAddedColumns(), + changedColumns: $diff->getChangedColumns(), + droppedColumns: $diff->getDroppedColumns(), + addedIndexes: array_values($addedIndexes), + modifiedIndexes: array_values($modifiedIndexes), + droppedIndexes: $diff->getDroppedIndexes(), + renamedIndexes: $diff->getRenamedIndexes(), + addedForeignKeys: $diff->getAddedForeignKeys(), + modifiedForeignKeys: $diff->getModifiedForeignKeys(), + droppedForeignKeys: $diff->getDroppedForeignKeys(), ); } - $sql = []; $tableSql = []; if (count($queryParts) > 0) { - $sql[] = 'ALTER TABLE ' . $diff->getOldTable()->getQuotedName($this) . ' ' + $tableSql[] = 'ALTER TABLE ' . $diff->getOldTable()->getQuotedName($this) . ' ' . implode(', ', $queryParts); } - $sql = array_merge( + return array_merge( $this->getPreAlterTableIndexForeignKeySQL($diff), - $sql, + $tableSql, $this->getPostAlterTableIndexForeignKeySQL($diff), ); - - return array_merge($sql, $tableSql, $columnSql); } /** diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 3959e2fa295..0db25e1b5c2 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -815,8 +815,7 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr } } - $columnSql = []; - $columns = []; + $columns = []; foreach ($table->getColumns() as $column) { $columnData = $this->columnToArray($column); @@ -846,7 +845,7 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr } } - return array_merge($sql, $columnSql); + return $sql; } /** @@ -944,7 +943,7 @@ public function getInlineColumnCommentSQL(string $comment): string * @param mixed[][] $columns * @param mixed[] $options * - * @return array + * @return list */ protected function _getCreateTableSQL(string $name, array $columns, array $options = []): array { @@ -961,7 +960,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio } if (isset($options['indexes']) && ! empty($options['indexes'])) { - foreach ($options['indexes'] as $index => $definition) { + foreach ($options['indexes'] as $definition) { $columnListSql .= ', ' . $this->getIndexDeclarationSQL($definition); } } @@ -1290,6 +1289,20 @@ protected function getRenameIndexSQL(string $oldIndexName, Index $index, string ]; } + /** + * 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 list 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. * diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php index a00b24bb702..35aca254f94 100644 --- a/src/Platforms/DB2Platform.php +++ b/src/Platforms/DB2Platform.php @@ -16,6 +16,7 @@ use Doctrine\DBAL\SQL\Builder\DefaultSelectSQLBuilder; use Doctrine\DBAL\SQL\Builder\SelectSQLBuilder; use Doctrine\DBAL\TransactionIsolationLevel; +use Doctrine\DBAL\Types\DateTimeType; use Doctrine\DBAL\Types\Types; use function array_merge; @@ -262,7 +263,6 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio public function getAlterTableSQL(TableDiff $diff): array { $sql = []; - $columnSql = []; $commentsSQL = []; $tableNameSQL = $diff->getOldTable()->getQuotedName($this); @@ -296,11 +296,13 @@ public function getAlterTableSQL(TableDiff $diff): array ); } + $needsReorg = false; foreach ($diff->getDroppedColumns() as $column) { $queryParts[] = 'DROP COLUMN ' . $column->getQuotedName($this); + $needsReorg = true; } - foreach ($diff->getModifiedColumns() as $columnDiff) { + foreach ($diff->getChangedColumns() as $columnDiff) { if ($columnDiff->hasCommentChanged()) { $newColumn = $columnDiff->getNewColumn(); $commentsSQL[] = $this->getCommentOnColumnSQL( @@ -315,33 +317,25 @@ public function getAlterTableSQL(TableDiff $diff): array $columnDiff, $sql, $queryParts, + $needsReorg, ); } - foreach ($diff->getRenamedColumns() as $oldColumnName => $column) { - $oldColumnName = new Identifier($oldColumnName); - - $queryParts[] = 'RENAME COLUMN ' . $oldColumnName->getQuotedName($this) . - ' TO ' . $column->getQuotedName($this); - } - if (count($queryParts) > 0) { $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . implode(' ', $queryParts); } // Some table alteration operations require a table reorganization. - if (count($diff->getDroppedColumns()) > 0 || count($diff->getModifiedColumns()) > 0) { + if ($needsReorg) { $sql[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE " . $tableNameSQL . "')"; } - $sql = array_merge( + return array_merge( $this->getPreAlterTableIndexForeignKeySQL($diff), $sql, $commentsSQL, $this->getPostAlterTableIndexForeignKeySQL($diff), ); - - return array_merge($sql, $columnSql); } public function getRenameTableSQL(string $oldName, string $newName): string @@ -362,10 +356,11 @@ private function gatherAlterColumnSQL( ColumnDiff $columnDiff, array &$sql, array &$queryParts, + bool &$needsReorg, ): void { - $alterColumnClauses = $this->getAlterColumnClausesSQL($columnDiff); + $alterColumnClauses = $this->getAlterColumnClausesSQL($columnDiff, $needsReorg); - if (empty($alterColumnClauses)) { + if (count($alterColumnClauses) < 1) { return; } @@ -389,18 +384,28 @@ private function gatherAlterColumnSQL( * * @return string[] */ - private function getAlterColumnClausesSQL(ColumnDiff $columnDiff): array + private function getAlterColumnClausesSQL(ColumnDiff $columnDiff, bool &$needsReorg): array { - $newColumn = $columnDiff->getNewColumn()->toArray(); + $newColumn = $columnDiff->getNewColumn(); + $columnArray = $newColumn->toArray(); + + $newName = $columnDiff->getNewColumn()->getQuotedName($this); + $oldName = $columnDiff->getOldColumn()->getQuotedName($this); - $alterClause = 'ALTER COLUMN ' . $columnDiff->getNewColumn()->getQuotedName($this); + $alterClause = 'ALTER COLUMN ' . $newName; - if ($newColumn['columnDefinition'] !== null) { - return [$alterClause . ' ' . $newColumn['columnDefinition']]; + if ($newColumn->getColumnDefinition() !== null) { + $needsReorg = true; + + return [$alterClause . ' ' . $newColumn->getColumnDefinition()]; } $clauses = []; + if ($columnDiff->hasNameChanged()) { + $clauses[] = 'RENAME COLUMN ' . $oldName . ' TO ' . $newName; + } + if ( $columnDiff->hasTypeChanged() || $columnDiff->hasLengthChanged() || @@ -408,22 +413,27 @@ private function getAlterColumnClausesSQL(ColumnDiff $columnDiff): array $columnDiff->hasScaleChanged() || $columnDiff->hasFixedChanged() ) { - $clauses[] = $alterClause . ' SET DATA TYPE ' . $newColumn['type']->getSQLDeclaration($newColumn, $this); + $needsReorg = true; + $clauses[] = $alterClause . ' SET DATA TYPE ' . $newColumn->getType() + ->getSQLDeclaration($columnArray, $this); } if ($columnDiff->hasNotNullChanged()) { - $clauses[] = $newColumn['notnull'] ? $alterClause . ' SET NOT NULL' : $alterClause . ' DROP NOT NULL'; + $needsReorg = true; + $clauses[] = $newColumn->getNotnull() ? $alterClause . ' SET NOT NULL' : $alterClause . ' DROP NOT NULL'; } if ($columnDiff->hasDefaultChanged()) { - if (isset($newColumn['default'])) { - $defaultClause = $this->getDefaultValueDeclarationSQL($newColumn); + if ($newColumn->getDefault() !== null) { + $defaultClause = $this->getDefaultValueDeclarationSQL($columnArray); if ($defaultClause !== '') { - $clauses[] = $alterClause . ' SET' . $defaultClause; + $needsReorg = true; + $clauses[] = $alterClause . ' SET' . $defaultClause; } } else { - $clauses[] = $alterClause . ' DROP DEFAULT'; + $needsReorg = true; + $clauses[] = $alterClause . ' DROP DEFAULT'; } } @@ -485,12 +495,12 @@ protected function getRenameIndexSQL(string $oldIndexName, Index $index, string */ public function getDefaultValueDeclarationSQL(array $column): string { - if (! empty($column['autoincrement'])) { + if (isset($column['autoincrement']) && $column['autoincrement'] === true) { return ''; } - if (! empty($column['version'])) { - if ((string) $column['type'] !== 'DateTime') { + if (isset($column['version']) && $column['version'] === true) { + if ($column['type'] instanceof DateTimeType) { $column['default'] = '1'; } } diff --git a/src/Platforms/OraclePlatform.php b/src/Platforms/OraclePlatform.php index 314f6ee7d40..1e37cbbba63 100644 --- a/src/Platforms/OraclePlatform.php +++ b/src/Platforms/OraclePlatform.php @@ -15,6 +15,7 @@ use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\TableDiff; use Doctrine\DBAL\TransactionIsolationLevel; +use Doctrine\DBAL\Types\BinaryType; use Doctrine\DBAL\Types\Types; use InvalidArgumentException; @@ -322,7 +323,7 @@ protected function _getCreateTableSQL(string $name, array $columns, array $optio } if ( - empty($column['autoincrement']) + ! isset($column['autoincrement']) || $column['autoincrement'] === false ) { continue; } @@ -523,10 +524,8 @@ public function getDropDatabaseSQL(string $name): string */ public function getAlterTableSQL(TableDiff $diff): array { - $sql = []; - $commentsSQL = []; - $columnSql = []; - + $sql = []; + $commentsSQL = []; $addColumnSQL = []; $tableNameSQL = $diff->getOldTable()->getQuotedName($this); @@ -551,23 +550,52 @@ public function getAlterTableSQL(TableDiff $diff): array } $modifyColumnSQL = []; - foreach ($diff->getModifiedColumns() as $columnDiff) { + foreach ($diff->getChangedColumns() as $columnDiff) { $newColumn = $columnDiff->getNewColumn(); $oldColumn = $columnDiff->getOldColumn(); - $newColumnProperties = $newColumn->toArray(); - $oldColumnProperties = $oldColumn->toArray(); + // Column names in Oracle are case insensitive and automatically uppercased on the server. + if ($columnDiff->hasNameChanged()) { + $newColumnName = $newColumn->getQuotedName($this); + $oldColumnName = $oldColumn->getQuotedName($this); - $oldSQL = $this->getColumnDeclarationSQL('', $oldColumnProperties); - $newSQL = $this->getColumnDeclarationSQL('', $newColumnProperties); + $sql = array_merge( + $sql, + $this->getRenameColumnSQL($tableNameSQL, $oldColumnName, $newColumnName), + ); + } - if ($newSQL !== $oldSQL) { - if (! $columnDiff->hasNotNullChanged()) { - unset($newColumnProperties['notnull']); - $newSQL = $this->getColumnDeclarationSQL('', $newColumnProperties); - } + $countChangedProperties = $columnDiff->countChangedProperties(); + // 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 ( + $newColumn->getType() instanceof BinaryType && + $columnDiff->hasFixedChanged() && + $countChangedProperties === 1 + ) { + continue; + } + + $columnHasChangedComment = $columnDiff->hasCommentChanged(); - $modifyColumnSQL[] = $newColumn->getQuotedName($this) . $newSQL; + /** + * Do not add query part if only comment has changed + */ + if ($countChangedProperties > ($columnHasChangedComment ? 1 : 0)) { + $newColumnProperties = $newColumn->toArray(); + + $oldSQL = $this->getColumnDeclarationSQL('', $oldColumn->toArray()); + $newSQL = $this->getColumnDeclarationSQL('', $newColumnProperties); + + if ($newSQL !== $oldSQL) { + if (! $columnDiff->hasNotNullChanged()) { + unset($newColumnProperties['notnull']); + $newSQL = $this->getColumnDeclarationSQL('', $newColumnProperties); + } + + $modifyColumnSQL[] = $newColumn->getQuotedName($this) . $newSQL; + } } if (! $columnDiff->hasCommentChanged()) { @@ -585,13 +613,6 @@ public function getAlterTableSQL(TableDiff $diff): array $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' MODIFY (' . implode(', ', $modifyColumnSQL) . ')'; } - foreach ($diff->getRenamedColumns() as $oldColumnName => $column) { - $oldColumnName = new Identifier($oldColumnName); - - $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' RENAME COLUMN ' . $oldColumnName->getQuotedName($this) - . ' TO ' . $column->getQuotedName($this); - } - $dropColumnSQL = []; foreach ($diff->getDroppedColumns() as $column) { $dropColumnSQL[] = $column->getQuotedName($this); @@ -606,7 +627,6 @@ public function getAlterTableSQL(TableDiff $diff): array $sql, $commentsSQL, $this->getPostAlterTableIndexForeignKeySQL($diff), - $columnSql, ); } diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php index 166ee7547fb..dce99c656d8 100644 --- a/src/Platforms/PostgreSQLPlatform.php +++ b/src/Platforms/PostgreSQLPlatform.php @@ -202,7 +202,6 @@ public function getAlterTableSQL(TableDiff $diff): array { $sql = []; $commentsSQL = []; - $columnSql = []; $table = $diff->getOldTable(); @@ -234,11 +233,19 @@ public function getAlterTableSQL(TableDiff $diff): array $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . $query; } - foreach ($diff->getModifiedColumns() as $columnDiff) { + foreach ($diff->getChangedColumns() as $columnDiff) { $oldColumn = $columnDiff->getOldColumn(); $newColumn = $columnDiff->getNewColumn(); $oldColumnName = $oldColumn->getQuotedName($this); + $newColumnName = $newColumn->getQuotedName($this); + + if ($columnDiff->hasNameChanged()) { + $sql = array_merge( + $sql, + $this->getRenameColumnSQL($tableNameSQL, $oldColumnName, $newColumnName), + ); + } if ( $columnDiff->hasTypeChanged() @@ -253,7 +260,7 @@ public function getAlterTableSQL(TableDiff $diff): array $columnDefinition['autoincrement'] = false; // here was a server version check before, but DBAL API does not support this anymore. - $query = 'ALTER ' . $oldColumnName . ' TYPE ' . $type->getSQLDeclaration($columnDefinition, $this); + $query = 'ALTER ' . $newColumnName . ' TYPE ' . $type->getSQLDeclaration($columnDefinition, $this); $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . $query; } @@ -262,12 +269,12 @@ public function getAlterTableSQL(TableDiff $diff): array ? ' DROP DEFAULT' : ' SET' . $this->getDefaultValueDeclarationSQL($newColumn->toArray()); - $query = 'ALTER ' . $oldColumnName . $defaultClause; + $query = 'ALTER ' . $newColumnName . $defaultClause; $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . $query; } if ($columnDiff->hasNotNullChanged()) { - $query = 'ALTER ' . $oldColumnName . ' ' . ($newColumn->getNotnull() ? 'SET' : 'DROP') . ' NOT NULL'; + $query = 'ALTER ' . $newColumnName . ' ' . ($newColumn->getNotnull() ? 'SET' : 'DROP') . ' NOT NULL'; $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . $query; } @@ -278,34 +285,18 @@ public function getAlterTableSQL(TableDiff $diff): array $query = 'DROP IDENTITY'; } - $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ALTER ' . $oldColumnName . ' ' . $query; - } - - $newComment = $newColumn->getComment(); - $oldComment = $columnDiff->getOldColumn()->getComment(); - - if ($columnDiff->hasCommentChanged() || $oldComment !== $newComment) { - $commentsSQL[] = $this->getCommentOnColumnSQL( - $tableNameSQL, - $newColumn->getQuotedName($this), - $newComment, - ); + $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ALTER ' . $newColumnName . ' ' . $query; } - if (! $columnDiff->hasLengthChanged()) { + if (! $columnDiff->hasCommentChanged()) { continue; } - $query = 'ALTER ' . $oldColumnName . ' TYPE ' - . $newColumn->getType()->getSQLDeclaration($newColumn->toArray(), $this); - $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ' . $query; - } - - foreach ($diff->getRenamedColumns() as $oldColumnName => $column) { - $oldColumnName = new Identifier($oldColumnName); - - $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' RENAME COLUMN ' . $oldColumnName->getQuotedName($this) - . ' TO ' . $column->getQuotedName($this); + $commentsSQL[] = $this->getCommentOnColumnSQL( + $tableNameSQL, + $newColumn->getQuotedName($this), + $newColumn->getComment(), + ); } return array_merge( @@ -313,7 +304,6 @@ public function getAlterTableSQL(TableDiff $diff): array $sql, $commentsSQL, $this->getPostAlterTableIndexForeignKeySQL($diff), - $columnSql, ); } diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php index 506fe52bbea..ce5d71a471d 100644 --- a/src/Platforms/SQLServerPlatform.php +++ b/src/Platforms/SQLServerPlatform.php @@ -358,7 +358,6 @@ public function getAlterTableSQL(TableDiff $diff): array { $queryParts = []; $sql = []; - $columnSql = []; $commentsSql = []; $table = $diff->getOldTable(); @@ -397,16 +396,31 @@ public function getAlterTableSQL(TableDiff $diff): array $queryParts[] = 'DROP COLUMN ' . $column->getQuotedName($this); } - foreach ($diff->getModifiedColumns() as $columnDiff) { + $tableNameSQL = $table->getQuotedName($this); + + foreach ($diff->getChangedColumns() as $columnDiff) { $newColumn = $columnDiff->getNewColumn(); + $newColumnName = $newColumn->getQuotedName($this); + + $oldColumn = $columnDiff->getOldColumn(); + $oldColumnName = $oldColumn->getQuotedName($this); + $nameChanged = $columnDiff->hasNameChanged(); + + // Column names in SQL server are case insensitive and automatically uppercased on the server. + if ($nameChanged) { + $sql = array_merge( + $sql, + $this->getRenameColumnSQL($tableNameSQL, $oldColumnName, $newColumnName), + ); + } + $newComment = $newColumn->getComment(); $hasNewComment = $newComment !== ''; - $oldColumn = $columnDiff->getOldColumn(); - $oldComment = $oldColumn->getComment(); - $hasOldComment = $oldComment !== ''; + $oldComment = $oldColumn->getComment(); + $hasOldComment = $oldComment !== ''; - if ($hasOldComment && $hasNewComment && $newComment !== $oldComment) { + if ($hasOldComment && $hasNewComment && $oldComment !== $newComment) { $commentsSql[] = $this->getAlterColumnCommentSQL( $tableName, $newColumn->getQuotedName($this), @@ -425,19 +439,19 @@ public function getAlterTableSQL(TableDiff $diff): array ); } - $columnNameSQL = $newColumn->getQuotedName($this); + $columnNameSQL = $newColumn->getQuotedName($this); - $oldDeclarationSQL = $this->getColumnDeclarationSQL($columnNameSQL, $oldColumn->toArray()); - $newDeclarationSQL = $this->getColumnDeclarationSQL($columnNameSQL, $newColumn->toArray()); + $newDeclarationSQL = $this->getColumnDeclarationSQL($columnNameSQL, $newColumn->toArray()); + $oldDeclarationSQL = $this->getColumnDeclarationSQL($columnNameSQL, $oldColumn->toArray()); + $declarationSQLChanged = $newDeclarationSQL !== $oldDeclarationSQL; - $declarationSQLChanged = $newDeclarationSQL !== $oldDeclarationSQL; - $defaultChanged = $columnDiff->hasDefaultChanged(); + $defaultChanged = $columnDiff->hasDefaultChanged(); - if (! $declarationSQLChanged && ! $defaultChanged) { + if (! $declarationSQLChanged && ! $defaultChanged && ! $nameChanged) { continue; } - $requireDropDefaultConstraint = $this->alterColumnRequiresDropDefaultConstraint($columnDiff); + $requireDropDefaultConstraint = $this->alterColumnRequiresDropDefaultConstraint($columnDiff); if ($requireDropDefaultConstraint) { $queryParts[] = $this->getAlterTableDropDefaultConstraintClause($oldColumn); @@ -454,20 +468,7 @@ public function getAlterTableSQL(TableDiff $diff): array continue; } - $queryParts[] = $this->getAlterTableAddDefaultConstraintClause($tableName, $newColumn); - } - - $tableNameSQL = $table->getQuotedName($this); - - foreach ($diff->getRenamedColumns() as $oldColumnName => $newColumn) { - $oldColumnName = new Identifier($oldColumnName); - - $sql[] = sprintf( - "sp_rename '%s.%s', '%s', 'COLUMN'", - $tableNameSQL, - $oldColumnName->getQuotedName($this), - $newColumn->getQuotedName($this), - ); + $queryParts[] = $this->getAlterTableAddDefaultConstraintClause($tableName, $newColumn); } foreach ($queryParts as $query) { @@ -479,7 +480,6 @@ public function getAlterTableSQL(TableDiff $diff): array $sql, $commentsSql, $this->getPostAlterTableIndexForeignKeySQL($diff), - $columnSql, ); } @@ -638,6 +638,25 @@ protected function getRenameIndexSQL(string $oldIndexName, Index $index, string ]; } + /** + * 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 list The sequence of SQL statements for renaming the given column. + */ + protected function getRenameColumnSQL(string $tableName, string $oldColumnName, string $newColumnName): array + { + return [sprintf( + "EXEC sp_rename %s, %s, 'COLUMN'", + $this->quoteStringLiteral($tableName . '.' . $oldColumnName), + $this->quoteStringLiteral($newColumnName), + ), + ]; + } + /** * Returns the SQL statement for adding an extended property to a database object. * diff --git a/src/Platforms/SQLitePlatform.php b/src/Platforms/SQLitePlatform.php index 18a84480793..19e833f9d4f 100644 --- a/src/Platforms/SQLitePlatform.php +++ b/src/Platforms/SQLitePlatform.php @@ -614,7 +614,6 @@ public function getAlterTableSQL(TableDiff $diff): array $columns = []; $oldColumnNames = []; $newColumnNames = []; - $columnSql = []; foreach ($table->getColumns() as $column) { $columnName = strtolower($column->getName()); @@ -635,24 +634,7 @@ public function getAlterTableSQL(TableDiff $diff): array ); } - foreach ($diff->getRenamedColumns() as $oldColumnName => $column) { - $oldColumnName = strtolower($oldColumnName); - - $columns = $this->replaceColumn( - $table->getName(), - $columns, - $oldColumnName, - $column, - ); - - if (! isset($newColumnNames[$oldColumnName])) { - continue; - } - - $newColumnNames[$oldColumnName] = $column->getQuotedName($this); - } - - foreach ($diff->getModifiedColumns() as $columnDiff) { + foreach ($diff->getChangedColumns() as $columnDiff) { $oldColumnName = strtolower($columnDiff->getOldColumn()->getName()); $newColumn = $columnDiff->getNewColumn(); @@ -707,7 +689,7 @@ public function getAlterTableSQL(TableDiff $diff): array ); $sql[] = $this->getDropTableSQL($dataTable->getQuotedName($this)); - return array_merge($sql, $this->getPostAlterTableIndexForeignKeySQL($diff), $columnSql); + return array_merge($sql, $this->getPostAlterTableIndexForeignKeySQL($diff)); } /** @@ -744,9 +726,8 @@ private function replaceColumn(string $tableName, array $columns, string $column private function getSimpleAlterTableSQL(TableDiff $diff): array|false { if ( - count($diff->getModifiedColumns()) > 0 + count($diff->getChangedColumns()) > 0 || count($diff->getDroppedColumns()) > 0 - || count($diff->getRenamedColumns()) > 0 || count($diff->getAddedIndexes()) > 0 || count($diff->getModifiedIndexes()) > 0 || count($diff->getDroppedIndexes()) > 0 @@ -760,8 +741,7 @@ private function getSimpleAlterTableSQL(TableDiff $diff): array|false $table = $diff->getOldTable(); - $sql = []; - $columnSql = []; + $sql = []; foreach ($diff->getAddedColumns() as $column) { $definition = array_merge([ @@ -786,7 +766,7 @@ private function getSimpleAlterTableSQL(TableDiff $diff): array|false . $this->getColumnDeclarationSQL($definition['name'], $definition); } - return array_merge($sql, $columnSql); + return $sql; } /** @return string[] */ @@ -808,13 +788,7 @@ private function getColumnNamesInAlteredTable(TableDiff $diff, Table $oldTable): unset($columns[$columnName]); } - foreach ($diff->getRenamedColumns() as $oldColumnName => $column) { - $columnName = $column->getName(); - $columns[strtolower($oldColumnName)] = $columnName; - $columns[strtolower($columnName)] = $columnName; - } - - foreach ($diff->getModifiedColumns() as $columnDiff) { + foreach ($diff->getChangedColumns() as $columnDiff) { $oldColumnName = $columnDiff->getOldColumn()->getName(); $newColumnName = $columnDiff->getNewColumn()->getName(); $columns[strtolower($oldColumnName)] = $newColumnName; diff --git a/src/Schema/ColumnDiff.php b/src/Schema/ColumnDiff.php index 3e4950a6a96..57c550e13d4 100644 --- a/src/Schema/ColumnDiff.php +++ b/src/Schema/ColumnDiff.php @@ -4,6 +4,8 @@ namespace Doctrine\DBAL\Schema; +use function strcasecmp; + /** * Represents the change of a column. */ @@ -14,6 +16,21 @@ public function __construct(private readonly Column $oldColumn, private readonly { } + public function countChangedProperties(): int + { + return (int) $this->hasUnsignedChanged() + + (int) $this->hasAutoIncrementChanged() + + (int) $this->hasDefaultChanged() + + (int) $this->hasFixedChanged() + + (int) $this->hasPrecisionChanged() + + (int) $this->hasScaleChanged() + + (int) $this->hasLengthChanged() + + (int) $this->hasNotNullChanged() + + (int) $this->hasNameChanged() + + (int) $this->hasTypeChanged() + + (int) $this->hasCommentChanged(); + } + public function getOldColumn(): Column { return $this->oldColumn; @@ -24,6 +41,14 @@ public function getNewColumn(): Column return $this->newColumn; } + public function hasNameChanged(): bool + { + $oldColumn = $this->getOldColumn(); + + // Column names are case insensitive + return strcasecmp($oldColumn->getName(), $this->getNewColumn()->getName()) !== 0; + } + public function hasTypeChanged(): bool { return $this->newColumn->getType()::class !== $this->oldColumn->getType()::class; diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index 4c60c0770a3..240df8965dd 100644 --- a/src/Schema/Comparator.php +++ b/src/Schema/Comparator.php @@ -184,10 +184,30 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff continue; } - $modifiedColumns[] = new ColumnDiff($oldColumn, $newColumn); + $modifiedColumns[$oldColumnName] = new ColumnDiff($oldColumn, $newColumn); } - $renamedColumns = $this->detectRenamedColumns($addedColumns, $droppedColumns); + $renamedColumnNames = $newTable->getRenamedColumns(); + + foreach ($addedColumns as $addedColumnName => $addedColumn) { + if (! isset($renamedColumnNames[$addedColumn->getName()])) { + continue; + } + + $removedColumnName = strtolower($renamedColumnNames[$addedColumn->getName()]); + // Explicitly renamed columns need to be diffed, because their types can also have changed + $modifiedColumns[$removedColumnName] = new ColumnDiff( + $droppedColumns[$removedColumnName], + $addedColumn, + ); + + unset( + $addedColumns[$addedColumnName], + $droppedColumns[$removedColumnName], + ); + } + + $this->detectRenamedColumns($modifiedColumns, $addedColumns, $droppedColumns); $oldIndexes = $oldTable->getIndexes(); $newIndexes = $newTable->getIndexes(); @@ -253,17 +273,16 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff return new TableDiff( $oldTable, - $addedColumns, - $modifiedColumns, - $droppedColumns, - $renamedColumns, - $addedIndexes, - $modifiedIndexes, - $droppedIndexes, - $renamedIndexes, - $addedForeignKeys, - $modifiedForeignKeys, - $droppedForeignKeys, + addedColumns: $addedColumns, + changedColumns: $modifiedColumns, + droppedColumns: $droppedColumns, + addedIndexes: $addedIndexes, + modifiedIndexes: $modifiedIndexes, + droppedIndexes: $droppedIndexes, + renamedIndexes: $renamedIndexes, + addedForeignKeys: $addedForeignKeys, + modifiedForeignKeys: $modifiedForeignKeys, + droppedForeignKeys: $droppedForeignKeys, ); } @@ -271,13 +290,13 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff * Try to find columns that only changed their name, rename operations maybe cheaper than add/drop * however ambiguities between different possibilities should not lead to renaming at all. * - * @param array $addedColumns - * @param array $removedColumns - * - * @return array + * @param array $modifiedColumns + * @param array $addedColumns + * @param array $removedColumns */ - private function detectRenamedColumns(array &$addedColumns, array &$removedColumns): array + private function detectRenamedColumns(array &$modifiedColumns, array &$addedColumns, array &$removedColumns): void { + /** @var array>> $candidatesByName */ $candidatesByName = []; foreach ($addedColumns as $addedColumnName => $addedColumn) { @@ -286,33 +305,32 @@ private function detectRenamedColumns(array &$addedColumns, array &$removedColum continue; } - $candidatesByName[$addedColumn->getName()][] = [$removedColumn, $addedColumn, $addedColumnName]; + $candidatesByName[$addedColumnName][] = [$removedColumn, $addedColumn]; } } - $renamedColumns = []; - - foreach ($candidatesByName as $candidates) { + foreach ($candidatesByName as $addedColumnName => $candidates) { if (count($candidates) !== 1) { continue; } - [$removedColumn, $addedColumn] = $candidates[0]; - $removedColumnName = $removedColumn->getName(); - $addedColumnName = strtolower($addedColumn->getName()); + [$oldColumn, $newColumn] = $candidates[0]; + $oldColumnName = strtolower($oldColumn->getName()); - if (isset($renamedColumns[$removedColumnName])) { + if (isset($modifiedColumns[$oldColumnName])) { continue; } - $renamedColumns[$removedColumnName] = $addedColumn; + $modifiedColumns[$oldColumnName] = new ColumnDiff( + $oldColumn, + $newColumn, + ); + unset( $addedColumns[$addedColumnName], - $removedColumns[strtolower($removedColumnName)], + $removedColumns[$oldColumnName], ); } - - return $renamedColumns; } /** diff --git a/src/Schema/SQLiteSchemaManager.php b/src/Schema/SQLiteSchemaManager.php index 20dc4fd257d..ab2d12b5198 100644 --- a/src/Schema/SQLiteSchemaManager.php +++ b/src/Schema/SQLiteSchemaManager.php @@ -60,7 +60,7 @@ public function createForeignKey(ForeignKeyConstraint $foreignKey, string $table { $table = $this->introspectTable($table); - $this->alterTable(new TableDiff($table, [], [], [], [], [], [], [], [], [$foreignKey], [], [])); + $this->alterTable(new TableDiff($table, modifiedForeignKeys: [$foreignKey])); } public function dropForeignKey(string $name, string $table): void @@ -69,7 +69,7 @@ public function dropForeignKey(string $name, string $table): void $foreignKey = $table->getForeignKey($name); - $this->alterTable(new TableDiff($table, [], [], [], [], [], [], [], [], [], [], [$foreignKey])); + $this->alterTable(new TableDiff($table, modifiedForeignKeys: [$foreignKey])); } /** diff --git a/src/Schema/Table.php b/src/Schema/Table.php index cc7f04d2c49..2ee553e17a3 100644 --- a/src/Schema/Table.php +++ b/src/Schema/Table.php @@ -13,12 +13,14 @@ use Doctrine\DBAL\Schema\Exception\InvalidTableName; use Doctrine\DBAL\Schema\Exception\UniqueConstraintDoesNotExist; use Doctrine\DBAL\Types\Type; +use LogicException; use function array_merge; use function array_values; use function in_array; use function is_string; use function preg_match; +use function sprintf; use function strtolower; /** @@ -32,6 +34,9 @@ class Table extends AbstractAsset /** @var Index[] */ private array $implicitIndexes = []; + /** @var array keys are new names, values are old names */ + protected array $renamedColumns = []; + /** @var Index[] */ protected array $_indexes = []; @@ -265,6 +270,48 @@ public function addColumn(string $name, string $typeName, array $options = []): return $column; } + /** @return array */ + final public function getRenamedColumns(): array + { + return $this->renamedColumns; + } + + /** + * @throws LogicException + * @throws SchemaException + */ + final public function renameColumn(string $oldName, string $newName): Column + { + $oldName = $this->normalizeIdentifier($oldName); + $newName = $this->normalizeIdentifier($newName); + + if ($oldName === $newName) { + throw new LogicException(sprintf( + 'Attempt to rename column "%s.%s" to the same name.', + $this->getName(), + $oldName, + )); + } + + $column = $this->getColumn($oldName); + $column->_setName($newName); + unset($this->_columns[$oldName]); + $this->_addColumn($column); + + // If a column is renamed multiple times, we only want to know the original and last new name + if (isset($this->renamedColumns[$oldName])) { + $toRemove = $oldName; + $oldName = $this->renamedColumns[$oldName]; + unset($this->renamedColumns[$toRemove]); + } + + if ($newName !== $oldName) { + $this->renamedColumns[$newName] = $oldName; + } + + return $column; + } + /** @param array $options */ public function modifyColumn(string $name, array $options): self { diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php index 1cd59e84134..4d588dd4b30 100644 --- a/src/Schema/TableDiff.php +++ b/src/Schema/TableDiff.php @@ -4,6 +4,8 @@ namespace Doctrine\DBAL\Schema; +use Doctrine\Deprecations\Deprecation; + use function array_filter; use function count; @@ -19,9 +21,8 @@ class TableDiff * * @param array $droppedForeignKeys * @param array $addedColumns - * @param array $modifiedColumns + * @param array $changedColumns * @param array $droppedColumns - * @param array $renamedColumns * @param array $addedIndexes * @param array $modifiedIndexes * @param array $droppedIndexes @@ -31,17 +32,16 @@ class TableDiff */ public function __construct( private readonly Table $oldTable, - private readonly array $addedColumns, - private readonly array $modifiedColumns, - private readonly array $droppedColumns, - private readonly array $renamedColumns, - private array $addedIndexes, - private readonly array $modifiedIndexes, - private array $droppedIndexes, - private readonly array $renamedIndexes, - private readonly array $addedForeignKeys, - private readonly array $modifiedForeignKeys, - private readonly array $droppedForeignKeys, + private readonly array $addedColumns = [], + private readonly array $changedColumns = [], + private readonly array $droppedColumns = [], + private array $addedIndexes = [], + private readonly array $modifiedIndexes = [], + private array $droppedIndexes = [], + private readonly array $renamedIndexes = [], + private readonly array $addedForeignKeys = [], + private readonly array $modifiedForeignKeys = [], + private readonly array $droppedForeignKeys = [], ) { } @@ -56,10 +56,10 @@ public function getAddedColumns(): array return $this->addedColumns; } - /** @return array */ - public function getModifiedColumns(): array + /** @return array */ + public function getChangedColumns(): array { - return $this->modifiedColumns; + return $this->changedColumns; } /** @return array */ @@ -68,10 +68,30 @@ public function getDroppedColumns(): array return $this->droppedColumns; } - /** @return array */ + /** + * @deprecated Use {@see getModifiedColumns()} instead. + * + * @return array + */ public function getRenamedColumns(): array { - return $this->renamedColumns; + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6080', + '%s is deprecated, you should use `getModifiedColumns()` instead.', + __METHOD__, + ); + $renamed = []; + foreach ($this->getChangedColumns() as $diff) { + if (! $diff->hasNameChanged()) { + continue; + } + + $oldColumnName = $diff->getOldColumn()->getName(); + $renamed[$oldColumnName] = $diff->getNewColumn(); + } + + return $renamed; } /** @return array */ @@ -150,9 +170,8 @@ public function getDroppedForeignKeys(): array public function isEmpty(): bool { return count($this->addedColumns) === 0 - && count($this->modifiedColumns) === 0 + && count($this->changedColumns) === 0 && count($this->droppedColumns) === 0 - && count($this->renamedColumns) === 0 && count($this->addedIndexes) === 0 && count($this->modifiedIndexes) === 0 && count($this->droppedIndexes) === 0 diff --git a/tests/Functional/Platform/RenameColumnTest.php b/tests/Functional/Platform/RenameColumnTest.php index 1b321b19b3e..5e69982c431 100644 --- a/tests/Functional/Platform/RenameColumnTest.php +++ b/tests/Functional/Platform/RenameColumnTest.php @@ -6,12 +6,13 @@ use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Tests\FunctionalTestCase; +use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; class RenameColumnTest extends FunctionalTestCase { /** @dataProvider columnNameProvider */ - public function testColumnPositionRetainedAfterRenaming(string $columnName, string $newColumnName): void + public function testColumnPositionRetainedAfterImplicitRenaming(string $columnName, string $newColumnName): void { $table = new Table('test_rename'); $table->addColumn($columnName, Types::STRING, ['length' => 16]); @@ -34,6 +35,34 @@ public function testColumnPositionRetainedAfterRenaming(string $columnName, stri self::assertCount(2, $columns); self::assertEqualsIgnoringCase($newColumnName, $columns[0]->getName()); self::assertEqualsIgnoringCase('c2', $columns[1]->getName()); + self::assertCount(1, $diff->getRenamedColumns()); + } + + /** @dataProvider columnNameProvider */ + public function testColumnPositionRetainedAfterExplicitRenaming(string $columnName, string $newColumnName): void + { + $table = new Table('test_rename'); + $table->addColumn($columnName, Types::INTEGER, ['length' => 16]); + $table->addColumn('c2', Types::INTEGER); + + $this->dropAndCreateTable($table); + + // Force a different type to make sure it's not being caught implicitly + $table->renameColumn($columnName, $newColumnName)->setType(Type::getType(Types::BIGINT))->setLength(32); + + $sm = $this->connection->createSchemaManager(); + $diff = $sm->createComparator() + ->compareTables($sm->introspectTable('test_rename'), $table); + + $sm->alterTable($diff); + + $table = $sm->introspectTable('test_rename'); + $columns = $table->getColumns(); + + self::assertCount(1, $diff->getChangedColumns()); + self::assertCount(2, $columns); + self::assertEqualsIgnoringCase($newColumnName, $columns[0]->getName()); + self::assertEqualsIgnoringCase('c2', $columns[1]->getName()); } /** @return iterable */ diff --git a/tests/Functional/Schema/ComparatorTest.php b/tests/Functional/Schema/ComparatorTest.php index 16e32be70b6..5d7e81a411a 100644 --- a/tests/Functional/Schema/ComparatorTest.php +++ b/tests/Functional/Schema/ComparatorTest.php @@ -7,8 +7,10 @@ use Doctrine\DBAL\Platforms\AbstractMySQLPlatform; use Doctrine\DBAL\Platforms\MariaDBPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; +use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Tests\FunctionalTestCase; +use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; class ComparatorTest extends FunctionalTestCase @@ -46,6 +48,57 @@ public function testDefaultValueComparison(string $type, mixed $value): void ); } + public function testRenameColumnComparison(): void + { + $platform = $this->connection->getDatabasePlatform(); + $comparator = new Comparator($platform); + + $table = new Table('rename_table'); + $table->addColumn('test', Types::STRING, ['default' => 'baz', 'length' => 20]); + $table->addColumn('test2', Types::STRING, ['default' => 'baz', 'length' => 20]); + $table->addColumn('test3', Types::STRING, ['default' => 'foo', 'length' => 10]); + + $onlineTable = clone $table; + $table->renameColumn('test', 'baz') + ->setLength(40) + ->setComment('Comment'); + + $table->renameColumn('test2', 'foo'); + + $table->getColumn('test3') + ->setAutoincrement(true) + ->setNotnull(false) + ->setType(Type::getType(Types::BIGINT)); + + $compareResult = $comparator->compareTables($onlineTable, $table); + self::assertCount(3, $compareResult->getChangedColumns()); + self::assertCount(2, $compareResult->getRenamedColumns()); + self::assertArrayHasKey('test2', $compareResult->getRenamedColumns()); + + $renamedOnly = $compareResult->getChangedColumns()['test2']; + $renamedAndModified = $compareResult->getChangedColumns()['test']; + $modifiedOnly = $compareResult->getChangedColumns()['test3']; + + self::assertEquals('foo', $renamedOnly->getNewColumn()->getName()); + self::assertTrue($renamedOnly->hasNameChanged()); + self::assertEquals(1, $renamedOnly->countChangedProperties()); + + self::assertEquals('baz', $renamedAndModified->getNewColumn()->getName()); + self::assertTrue($renamedAndModified->hasNameChanged()); + self::assertTrue($renamedAndModified->hasLengthChanged()); + self::assertTrue($renamedAndModified->hasCommentChanged()); + self::assertFalse($renamedAndModified->hasTypeChanged()); + self::assertEquals(3, $renamedAndModified->countChangedProperties()); + + self::assertTrue($modifiedOnly->hasAutoIncrementChanged()); + self::assertTrue($modifiedOnly->hasNotNullChanged()); + self::assertTrue($modifiedOnly->hasTypeChanged()); + self::assertFalse($modifiedOnly->hasLengthChanged()); + self::assertFalse($modifiedOnly->hasCommentChanged()); + self::assertFalse($modifiedOnly->hasNameChanged()); + self::assertEquals(3, $modifiedOnly->countChangedProperties()); + } + /** @return iterable */ public static function defaultValueProvider(): iterable { diff --git a/tests/Functional/Schema/CustomIntrospectionTest.php b/tests/Functional/Schema/CustomIntrospectionTest.php index 8344eb5617d..a65cbb4cecd 100644 --- a/tests/Functional/Schema/CustomIntrospectionTest.php +++ b/tests/Functional/Schema/CustomIntrospectionTest.php @@ -57,7 +57,7 @@ public function testCustomColumnIntrospection(): void $diff = $schemaManager->createComparator()->compareTables($onlineTable, $table); $changedCols = array_map( static fn (ColumnDiff $columnDiff): string => $columnDiff->getOldColumn()->getName(), - $diff->getModifiedColumns(), + $diff->getChangedColumns(), ); self::assertTrue($diff->isEmpty(), sprintf( diff --git a/tests/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Platforms/AbstractMySQLPlatformTestCase.php index 4a48e89f0cf..20fe96ab4f7 100644 --- a/tests/Platforms/AbstractMySQLPlatformTestCase.php +++ b/tests/Platforms/AbstractMySQLPlatformTestCase.php @@ -536,11 +536,10 @@ public function testIgnoresDifferenceInDefaultValuesForUnsupportedColumnTypes(): $diffTable->modifyColumn('def_blob', ['default' => null]); $diffTable->modifyColumn('def_blob_null', ['default' => null]); - self::assertTrue( - $this->createComparator() - ->compareTables($table, $diffTable) - ->isEmpty(), - ); + $comparator = $this->createComparator(); + + self::assertTrue($comparator->compareTables($table, $diffTable)->isEmpty()); + self::assertTrue($comparator->compareTables($table, $diffTable)->isEmpty()); } public function testReturnsGuidTypeDeclarationSQL(): void diff --git a/tests/Platforms/AbstractPlatformTestCase.php b/tests/Platforms/AbstractPlatformTestCase.php index 9669d277b0f..ac0cc8422a1 100644 --- a/tests/Platforms/AbstractPlatformTestCase.php +++ b/tests/Platforms/AbstractPlatformTestCase.php @@ -476,8 +476,8 @@ public function testAlterTableChangeQuotedColumn(): void $table = new Table('mytable'); $table->addColumn('select', Types::INTEGER); - $tableDiff = new TableDiff($table, [], [ - new ColumnDiff( + $tableDiff = new TableDiff($table, changedColumns: [ + 'select' => new ColumnDiff( $table->getColumn('select'), new Column( 'select', @@ -485,7 +485,7 @@ public function testAlterTableChangeQuotedColumn(): void ['length' => 255], ), ), - ], [], [], [], [], [], [], [], [], []); + ]); self::assertStringContainsString( $this->platform->quoteIdentifier('select'), @@ -635,9 +635,9 @@ public function testAlterTableRenameIndex(): void $table->addColumn('id', Types::INTEGER); $table->setPrimaryKey(['id']); - $tableDiff = new TableDiff($table, [], [], [], [], [], [], [], [ + $tableDiff = new TableDiff($table, renamedIndexes: [ 'idx_foo' => new Index('idx_bar', ['id']), - ], [], [], []); + ]); self::assertSame( $this->getAlterTableRenameIndexSQL(), @@ -660,10 +660,10 @@ public function testQuotesAlterTableRenameIndex(): void $table->addColumn('id', Types::INTEGER); $table->setPrimaryKey(['id']); - $tableDiff = new TableDiff($table, [], [], [], [], [], [], [], [ + $tableDiff = new TableDiff($table, renamedIndexes: [ 'create' => new Index('select', ['id']), '`foo`' => new Index('`bar`', ['id']), - ], [], [], []); + ]); self::assertSame( $this->getQuotedAlterTableRenameIndexSQL(), @@ -688,9 +688,7 @@ public function testAlterTableRenameIndexInSchema(): void $table->addColumn('id', Types::INTEGER); $table->setPrimaryKey(['id']); - $tableDiff = new TableDiff($table, [], [], [], [], [], [], [], [ - 'idx_foo' => new Index('idx_bar', ['id']), - ], [], [], []); + $tableDiff = new TableDiff($table, renamedIndexes: ['idx_foo' => new Index('idx_bar', ['id'])]); self::assertSame( $this->getAlterTableRenameIndexInSchemaSQL(), @@ -713,10 +711,10 @@ public function testQuotesAlterTableRenameIndexInSchema(): void $table->addColumn('id', Types::INTEGER); $table->setPrimaryKey(['id']); - $tableDiff = new TableDiff($table, [], [], [], [], [], [], [], [ + $tableDiff = new TableDiff($table, renamedIndexes: [ 'create' => new Index('select', ['id']), '`foo`' => new Index('`bar`', ['id']), - ], [], [], []); + ]); self::assertSame( $this->getQuotedAlterTableRenameIndexInSchemaSQL(), @@ -860,8 +858,8 @@ public function testAlterStringToFixedString(): void $table = new Table('mytable'); $table->addColumn('name', Types::STRING, ['length' => 2]); - $tableDiff = new TableDiff($table, [], [ - new ColumnDiff( + $tableDiff = new TableDiff($table, changedColumns: [ + 'name' => new ColumnDiff( $table->getColumn('name'), new Column( 'name', @@ -869,7 +867,7 @@ public function testAlterStringToFixedString(): void ['fixed' => true, 'length' => 2], ), ), - ], [], [], [], [], [], [], [], [], []); + ]); $sql = $this->platform->getAlterTableSQL($tableDiff); @@ -896,9 +894,9 @@ public function testGeneratesAlterTableRenameIndexUsedByForeignKeySQL(): void $primaryTable->addForeignKeyConstraint($foreignTable->getName(), ['foo'], ['id'], [], 'fk_foo'); $primaryTable->addForeignKeyConstraint($foreignTable->getName(), ['bar'], ['id'], [], 'fk_bar'); - $tableDiff = new TableDiff($primaryTable, [], [], [], [], [], [], [], [ + $tableDiff = new TableDiff($primaryTable, renamedIndexes: [ 'idx_foo' => new Index('idx_foo_renamed', ['foo']), - ], [], [], []); + ]); self::assertSame( $this->getGeneratesAlterTableRenameIndexUsedByForeignKeySQL(), @@ -987,28 +985,3 @@ public static function asciiStringSqlDeclarationDataProvider(): array ]; } } - -interface GetCreateTableSqlDispatchEventListener -{ - public function onSchemaCreateTable(): void; - - public function onSchemaCreateTableColumn(): void; -} - -interface GetAlterTableSqlDispatchEventListener -{ - public function onSchemaAlterTable(): void; - - public function onSchemaAlterTableAddColumn(): void; - - public function onSchemaAlterTableRemoveColumn(): void; - - public function onSchemaAlterTableChangeColumn(): void; - - public function onSchemaAlterTableRenameColumn(): void; -} - -interface GetDropTableSqlDispatchEventListener -{ - public function onSchemaDropTable(): void; -} diff --git a/tests/Platforms/DB2PlatformTest.php b/tests/Platforms/DB2PlatformTest.php index 57320ba85d5..44a1df0dff3 100644 --- a/tests/Platforms/DB2PlatformTest.php +++ b/tests/Platforms/DB2PlatformTest.php @@ -445,10 +445,11 @@ public function testGeneratesAlterColumnSQL( Column $oldColumn, Column $newColumn, ?string $expectedSQLClause, + bool $shouldReorg = true, ): void { - $tableDiff = new TableDiff(new Table('foo'), [], [ - new ColumnDiff($oldColumn, $newColumn), - ], [], [], [], [], [], [], [], [], []); + $tableDiff = new TableDiff(new Table('foo'), changedColumns: [ + $oldColumn->getName() => new ColumnDiff($oldColumn, $newColumn), + ]); $expectedSQL = []; @@ -456,7 +457,9 @@ public function testGeneratesAlterColumnSQL( $expectedSQL[] = 'ALTER TABLE foo ALTER COLUMN bar ' . $expectedSQLClause; } - $expectedSQL[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE foo')"; + if ($shouldReorg) { + $expectedSQL[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE foo')"; + } self::assertSame($expectedSQL, $this->platform->getAlterTableSQL($tableDiff)); } @@ -514,6 +517,7 @@ public static function getGeneratesAlterColumnSQL(): iterable new Column('bar', Type::getType(Types::INTEGER)), new Column('bar', Type::getType(Types::INTEGER), ['autoincrement' => true, 'default' => 666]), null, + false, ], [ new Column('bar', Type::getType(Types::STRING), ['default' => 'foo']), diff --git a/tests/Platforms/SQLServerPlatformTest.php b/tests/Platforms/SQLServerPlatformTest.php index 7f2aedf3921..ae8b612a094 100644 --- a/tests/Platforms/SQLServerPlatformTest.php +++ b/tests/Platforms/SQLServerPlatformTest.php @@ -646,13 +646,13 @@ public function testAlterTableWithSchemaColumnComments(): void { $table = new Table('testschema.mytable'); - $tableDiff = new TableDiff($table, [ + $tableDiff = new TableDiff($table, addedColumns: [ new Column( 'quota', Type::getType(Types::INTEGER), ['comment' => 'A comment'], ), - ], [], [], [], [], [], [], [], [], [], []); + ]); $expectedSql = [ 'ALTER TABLE testschema.mytable ADD quota INT NOT NULL', @@ -667,11 +667,12 @@ public function testAlterTableWithSchemaDropColumnComments(): void { $table = new Table('testschema.mytable'); - $tableDiff = new TableDiff($table, [], [new ColumnDiff( - new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment']), - new Column('quota', Type::getType(Types::INTEGER), []), - ), - ], [], [], [], [], [], [], [], [], []); + $tableDiff = new TableDiff($table, changedColumns: [ + 'quota' => new ColumnDiff( + new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment']), + new Column('quota', Type::getType(Types::INTEGER), []), + ), + ]); $expectedSql = [ "EXEC sp_dropextendedproperty N'MS_Description'" @@ -685,11 +686,12 @@ public function testAlterTableWithSchemaUpdateColumnComments(): void { $table = new Table('testschema.mytable'); - $tableDiff = new TableDiff($table, [], [new ColumnDiff( - new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment']), - new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'B comment']), - ), - ], [], [], [], [], [], [], [], [], []); + $tableDiff = new TableDiff($table, changedColumns: [ + 'quota' => new ColumnDiff( + new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment']), + new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'B comment']), + ), + ]); $expectedSql = ["EXEC sp_updateextendedproperty N'MS_Description', N'B comment', " . "N'SCHEMA', 'testschema', N'TABLE', 'mytable', N'COLUMN', quota", @@ -1059,12 +1061,12 @@ public function testAlterTableWithSchemaSameColumnComments(): void { $table = new Table('testschema.mytable'); - $tableDiff = new TableDiff($table, [], [ - new ColumnDiff( + $tableDiff = new TableDiff($table, changedColumns: [ + 'quota' => new ColumnDiff( new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment', 'notnull' => false]), new Column('quota', Type::getType(Types::INTEGER), ['comment' => 'A comment', 'notnull' => true]), ), - ], [], [], [], [], [], [], [], [], []); + ]); $expectedSql = ['ALTER TABLE testschema.mytable ALTER COLUMN quota INT NOT NULL']; diff --git a/tests/Platforms/SQLitePlatformTest.php b/tests/Platforms/SQLitePlatformTest.php index c15b85dcb32..4a446e0a8d5 100644 --- a/tests/Platforms/SQLitePlatformTest.php +++ b/tests/Platforms/SQLitePlatformTest.php @@ -9,6 +9,7 @@ use Doctrine\DBAL\Platforms\SQLite; use Doctrine\DBAL\Platforms\SQLitePlatform; use Doctrine\DBAL\Schema\Column; +use Doctrine\DBAL\Schema\ColumnDiff; use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; @@ -223,13 +224,13 @@ public function testAlterTableAddColumns(): void { $table = new Table('user'); - $diff = new TableDiff($table, [ + $diff = new TableDiff($table, addedColumns: [ new Column('foo', Type::getType(Types::STRING)), new Column('count', Type::getType(Types::INTEGER), [ 'notnull' => false, 'default' => 1, ]), - ], [], [], [], [], [], [], [], [], [], []); + ]); $expected = [ 'ALTER TABLE user ADD COLUMN foo VARCHAR NOT NULL', @@ -244,9 +245,12 @@ public function testRenameNonExistingColumn(): void $table = new Table('test'); $table->addColumn('id', Types::INTEGER); - $tableDiff = new TableDiff($table, [], [], [], [ - 'value' => new Column('data', Type::getType(Types::STRING)), - ], [], [], [], [], [], [], []); + $tableDiff = new TableDiff($table, changedColumns: [ + 'value' => new ColumnDiff( + new Column('data', Type::getType(Types::STRING)), + new Column('value', Type::getType(Types::STRING)), + ), + ]); $this->expectException(Exception::class); $this->platform->getAlterTableSQL($tableDiff); @@ -296,14 +300,25 @@ public function testAlterTable(): void $table->addForeignKeyConstraint('user', ['parent'], ['id'], ['deferrable' => true, 'deferred' => true]); $table->addIndex(['article', 'post'], 'index1'); - $diff = new TableDiff($table, [], [], [ - new Column('parent', Type::getType(Types::INTEGER), []), - ], [ - 'id' => new Column('key', Type::getType(Types::INTEGER), []), - 'post' => new Column('comment', Type::getType(Types::INTEGER), []), - ], [], [], [ - $table->getIndex('index1'), - ], [], [], [], []); + $diff = new TableDiff( + $table, + changedColumns: [ + 'id' => new ColumnDiff( + $table->getColumn('id'), + new Column('key', Type::getType(Types::INTEGER)), + ), + 'post' => new ColumnDiff( + $table->getColumn('post'), + new Column('comment', Type::getType(Types::INTEGER)), + ), + ], + droppedColumns: [ + new Column('parent', Type::getType(Types::INTEGER), []), + ], + droppedIndexes: [ + $table->getIndex('index1'), + ], + ); $sql = [ 'CREATE TEMPORARY TABLE __temp__user AS SELECT id, article, post FROM user', diff --git a/tests/Schema/AbstractComparatorTestCase.php b/tests/Schema/AbstractComparatorTestCase.php index ed9faafb6f0..5b9ec5a18be 100644 --- a/tests/Schema/AbstractComparatorTestCase.php +++ b/tests/Schema/AbstractComparatorTestCase.php @@ -21,6 +21,7 @@ use PHPUnit\Framework\TestCase; use function array_keys; +use function current; abstract class AbstractComparatorTestCase extends TestCase { @@ -179,7 +180,7 @@ public function testCompareChangeColumnsMultipleNewColumnsRename(): void self::assertEquals(['new_datecolumn2'], $this->getAssetNames($tableDiff->getAddedColumns())); self::assertCount(0, $tableDiff->getDroppedColumns()); - self::assertCount(0, $tableDiff->getModifiedColumns()); + self::assertCount(1, $tableDiff->getChangedColumns()); } public function testCompareSequences(): void @@ -355,9 +356,9 @@ public function testCompareIndexBasedOnPropertiesNotName(): void $tableB->addIndex(['id'], 'bar_foo_idx'); self::assertEquals( - new TableDiff($tableA, [], [], [], [], [], [], [], [ + new TableDiff($tableA, renamedIndexes: [ 'foo_bar_idx' => new Index('bar_foo_idx', ['id']), - ], [], [], []), + ]), $this->comparator->compareTables($tableA, $tableB), ); } @@ -471,9 +472,11 @@ public function testDetectChangeIdentifierType(): void $tableDiff = $this->comparator->compareTables($tableA, $tableB); - $modifiedColumns = $tableDiff->getModifiedColumns(); + $modifiedColumns = $tableDiff->getChangedColumns(); self::assertCount(1, $modifiedColumns); - self::assertEquals('id', $modifiedColumns[0]->getOldColumn()->getName()); + /** @var ColumnDiff $modifiedColumn */ + $modifiedColumn = current($modifiedColumns); + self::assertEquals('id', $modifiedColumn->getOldColumn()->getName()); } public function testDiff(): void diff --git a/tests/Schema/TableTest.php b/tests/Schema/TableTest.php index 75382dd4dae..d825dcc0e33 100644 --- a/tests/Schema/TableTest.php +++ b/tests/Schema/TableTest.php @@ -15,6 +15,7 @@ use Doctrine\DBAL\Schema\UniqueConstraint; use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; +use LogicException; use PHPUnit\Framework\TestCase; use function array_keys; @@ -53,6 +54,52 @@ public function testColumns(): void self::assertCount(2, $table->getColumns()); } + public function testRenameColumn(): void + { + $typeStr = Type::getType(Types::STRING); + $typeTxt = Type::getType(Types::TEXT); + $columns = []; + $columns[] = new Column('foo', $typeStr); + $table = new Table('foo', $columns, [], []); + + self::assertFalse($table->hasColumn('bar')); + self::assertTrue($table->hasColumn('foo')); + + $column = $table->renameColumn('foo', 'bar'); + $column->setType($typeTxt); + self::assertTrue($table->hasColumn('bar'), 'Should now have bar column'); + self::assertFalse($table->hasColumn('foo'), 'Should not have foo column anymore'); + self::assertCount(1, $table->getColumns()); + + self::assertEquals(['bar' => 'foo'], $table->getRenamedColumns()); + $table->renameColumn('bar', 'baz'); + + self::assertTrue($table->hasColumn('baz'), 'Should now have baz column'); + self::assertFalse($table->hasColumn('bar'), 'Should not have bar column anymore'); + self::assertEquals(['baz' => 'foo'], $table->getRenamedColumns()); + self::assertCount(1, $table->getColumns()); + } + + public function testRenameColumnException(): void + { + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Attempt to rename column "foo.baz" to the same name.'); + + $table = new Table('foo'); + $table->renameColumn('baz', '`BaZ`'); + } + + public function testRenameColumnLoop(): void + { + $table = new Table('foo'); + $table->addColumn('baz', Types::INTEGER); + $table->renameColumn('baz', '`foo`'); + self::assertCount(1, $table->getRenamedColumns()); + $table->renameColumn('foo', 'Baz'); + self::assertCount(1, $table->getColumns()); + self::assertCount(0, $table->getRenamedColumns()); + } + public function testColumnsCaseInsensitive(): void { $table = new Table('foo');