From e34045a68ea7dfa718aef899b6953c6d41a8014e Mon Sep 17 00:00:00 2001 From: Tofandel Date: Fri, 26 Jan 2024 14:35:14 +0100 Subject: [PATCH] Merge branch '3.8.x' into 4.0.x # Conflicts: # psalm.xml.dist # src/Platforms/AbstractMySQLPlatform.php # src/Platforms/DB2Platform.php # src/Platforms/OraclePlatform.php # src/Platforms/PostgreSQLPlatform.php # src/Platforms/SQLServerPlatform.php # src/Platforms/SqlitePlatform.php # src/Schema/ColumnDiff.php # src/Schema/Comparator.php # src/Schema/Table.php # src/Schema/TableDiff.php # tests/Functional/Platform/RenameColumnTest.php # tests/Platforms/AbstractMySQLPlatformTestCase.php # tests/Platforms/AbstractPlatformTestCase.php # tests/Platforms/DB2PlatformTest.php # tests/Platforms/OraclePlatformTest.php # tests/Platforms/PostgreSQLPlatformTest.php # tests/Platforms/SQLServerPlatformTest.php # tests/Platforms/SQLitePlatformTest.php # tests/Schema/AbstractComparatorTestCase.php # tests/Schema/TableDiffTest.php --- .doctrine-project.json | 4 +- .github/workflows/continuous-integration.yml | 2 +- README.md | 28 +++---- psalm.xml.dist | 1 + src/Platforms/AbstractMySQLPlatform.php | 35 +++------ src/Platforms/AbstractPlatform.php | 14 ++++ src/Platforms/DB2Platform.php | 63 +++++++++------ src/Platforms/OraclePlatform.php | 61 ++++++++++----- src/Platforms/PostgreSQLPlatform.php | 32 ++++---- src/Platforms/SQLServerPlatform.php | 73 ++++++++++------- src/Platforms/SQLitePlatform.php | 30 +------ 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 | 33 +++++++- tests/Functional/Schema/ComparatorTest.php | 53 +++++++++++++ .../Schema/CustomIntrospectionTest.php | 2 +- .../AbstractMySQLPlatformTestCase.php | 9 +-- tests/Platforms/AbstractPlatformTestCase.php | 38 +++++---- tests/Platforms/DB2PlatformTest.php | 10 ++- tests/Platforms/SQLServerPlatformTest.php | 30 +++---- tests/Platforms/SQLitePlatformTest.php | 41 ++++++---- tests/Schema/AbstractComparatorTestCase.php | 10 +-- tests/Schema/TableTest.php | 47 +++++++++++ 26 files changed, 560 insertions(+), 269 deletions(-) diff --git a/.doctrine-project.json b/.doctrine-project.json index 7b4d623cbab..b0a66e4b73f 100644 --- a/.doctrine-project.json +++ b/.doctrine-project.json @@ -15,13 +15,13 @@ "name": "3.8", "branchName": "3.8.x", "slug": "3.8", - "upcoming": true + "current": true }, { "name": "3.7", "branchName": "3.7.x", "slug": "3.7", - "current": true + "maintained": false }, { "name": "3.6", diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 96d254e90ae..286c1385e22 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -351,7 +351,7 @@ jobs: mysql-version: - "5.7" - "8.0" - - "8.2" + - "8.3" extension: - "mysqli" - "pdo_mysql" diff --git a/README.md b/README.md index b76a305df1f..99fe1058785 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,11 @@ # Doctrine DBAL -| [4.0-dev][4.0] | [3.6][3.6] | +| [4.0-dev][4.0] | [3.8][3.8] | |:-----------------------------------------------:|:---------------------------------------------------:| -| [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.6 image]][GA 3.6] | -| [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.6 image]][AppVeyor 3.6] | -| [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.6 image]][CodeCov 3.6] | -| N/A | [![Type Coverage][TypeCov 3.6 image]][TypeCov 3.6] | +| [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.8 image]][GA 3.8] | +| [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.8 image]][AppVeyor 3.8] | +| [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.8 image]][CodeCov 3.8] | +| N/A | [![Type Coverage][TypeCov 3.8 image]][TypeCov 3.8] | Powerful ***D***ata***B***ase ***A***bstraction ***L***ayer with many features for database schema introspection and schema management. @@ -23,12 +23,12 @@ Powerful ***D***ata***B***ase ***A***bstraction ***L***ayer with many features f [GA 4.0]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A4.0.x [GA 4.0 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg - [Coverage 3.6 image]: https://codecov.io/gh/doctrine/dbal/branch/3.6.x/graph/badge.svg - [3.6]: https://github.com/doctrine/dbal/tree/3.6.x - [CodeCov 3.6]: https://codecov.io/gh/doctrine/dbal/branch/3.6.x - [AppVeyor 3.6]: https://ci.appveyor.com/project/doctrine/dbal/branch/3.6.x - [AppVeyor 3.6 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/3.6.x?svg=true - [GA 3.6]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A3.6.x - [GA 3.6 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=3.6.x - [TypeCov 3.6]: https://shepherd.dev/github/doctrine/dbal - [TypeCov 3.6 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg + [Coverage 3.8 image]: https://codecov.io/gh/doctrine/dbal/branch/3.8.x/graph/badge.svg + [3.8]: https://github.com/doctrine/dbal/tree/3.8.x + [CodeCov 3.8]: https://codecov.io/gh/doctrine/dbal/branch/3.8.x + [AppVeyor 3.8]: https://ci.appveyor.com/project/doctrine/dbal/branch/3.8.x + [AppVeyor 3.8 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/3.8.x?svg=true + [GA 3.8]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A3.8.x + [GA 3.8 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=3.8.x + [TypeCov 3.8]: https://shepherd.dev/github/doctrine/dbal + [TypeCov 3.8 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg 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..2d44adefdcf 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; @@ -353,7 +352,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 +365,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,17 +393,16 @@ 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(), ); } diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 3959e2fa295..44542e87fc2 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -1290,6 +1290,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 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. * diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php index a00b24bb702..03dc5dc2f8f 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; @@ -296,11 +297,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,22 +318,16 @@ 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 . "')"; } @@ -362,10 +359,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 +387,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 ' . $newName; - $alterClause = 'ALTER COLUMN ' . $columnDiff->getNewColumn()->getQuotedName($this); + if ($newColumn->getColumnDefinition() !== null) { + $needsReorg = true; - if ($newColumn['columnDefinition'] !== null) { - return [$alterClause . ' ' . $newColumn['columnDefinition']]; + return [$alterClause . ' ' . $newColumn->getColumnDefinition()]; } $clauses = []; + if ($columnDiff->hasNameChanged()) { + $clauses[] = 'RENAME COLUMN ' . $oldName . ' TO ' . $newName; + } + if ( $columnDiff->hasTypeChanged() || $columnDiff->hasLengthChanged() || @@ -408,22 +416,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 +498,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..755f3ade99a 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; } @@ -551,23 +552,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 +615,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); diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php index 166ee7547fb..925c844871c 100644 --- a/src/Platforms/PostgreSQLPlatform.php +++ b/src/Platforms/PostgreSQLPlatform.php @@ -234,11 +234,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 +261,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 +270,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,17 +286,14 @@ public function getAlterTableSQL(TableDiff $diff): array $query = 'DROP IDENTITY'; } - $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ALTER ' . $oldColumnName . ' ' . $query; + $sql[] = 'ALTER TABLE ' . $tableNameSQL . ' ALTER ' . $newColumnName . ' ' . $query; } - $newComment = $newColumn->getComment(); - $oldComment = $columnDiff->getOldColumn()->getComment(); - - if ($columnDiff->hasCommentChanged() || $oldComment !== $newComment) { + if ($columnDiff->hasCommentChanged()) { $commentsSQL[] = $this->getCommentOnColumnSQL( $tableNameSQL, $newColumn->getQuotedName($this), - $newComment, + $newColumn->getComment(), ); } @@ -301,13 +306,6 @@ public function getAlterTableSQL(TableDiff $diff): array $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); - } - return array_merge( $this->getPreAlterTableIndexForeignKeySQL($diff), $sql, diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php index 506fe52bbea..2655db4c94a 100644 --- a/src/Platforms/SQLServerPlatform.php +++ b/src/Platforms/SQLServerPlatform.php @@ -397,16 +397,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 +440,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 +469,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) { @@ -638,6 +640,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 string[] 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..dbe1c575eea 100644 --- a/src/Platforms/SQLitePlatform.php +++ b/src/Platforms/SQLitePlatform.php @@ -635,24 +635,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(); @@ -744,9 +727,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 @@ -808,13 +790,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..be224b8f1a1 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 = $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..8a03cf17552 100644 --- a/tests/Functional/Platform/RenameColumnTest.php +++ b/tests/Functional/Platform/RenameColumnTest.php @@ -6,12 +6,15 @@ use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Tests\FunctionalTestCase; +use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; +use function array_values; + 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 +37,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 = array_values($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..e58a68ec0ba 100644 --- a/tests/Platforms/AbstractPlatformTestCase.php +++ b/tests/Platforms/AbstractPlatformTestCase.php @@ -476,7 +476,7 @@ public function testAlterTableChangeQuotedColumn(): void $table = new Table('mytable'); $table->addColumn('select', Types::INTEGER); - $tableDiff = new TableDiff($table, [], [ + $tableDiff = new TableDiff($table, changedColumns: [ new ColumnDiff( $table->getColumn('select'), new Column( @@ -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,7 +858,7 @@ public function testAlterStringToFixedString(): void $table = new Table('mytable'); $table->addColumn('name', Types::STRING, ['length' => 2]); - $tableDiff = new TableDiff($table, [], [ + $tableDiff = new TableDiff($table, changedColumns: [ new ColumnDiff( $table->getColumn('name'), new Column( @@ -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(), @@ -997,15 +995,15 @@ public function onSchemaCreateTableColumn(): void; interface GetAlterTableSqlDispatchEventListener { - public function onSchemaAlterTable(): void; + public function onSchemaAlterTable(SchemaAlterTableEventArgs $args): void; - public function onSchemaAlterTableAddColumn(): void; + public function onSchemaAlterTableAddColumn(SchemaAlterTableAddColumnEventArgs $args): void; - public function onSchemaAlterTableRemoveColumn(): void; + public function onSchemaAlterTableRemoveColumn(SchemaAlterTableRemoveColumnEventArgs $args): void; - public function onSchemaAlterTableChangeColumn(): void; + public function onSchemaAlterTableChangeColumn(SchemaAlterTableChangeColumnEventArgs $args): void; - public function onSchemaAlterTableRenameColumn(): void; + public function onSchemaAlterTableRenameColumn(SchemaAlterTableRenameColumnEventArgs $args): void; } interface GetDropTableSqlDispatchEventListener diff --git a/tests/Platforms/DB2PlatformTest.php b/tests/Platforms/DB2PlatformTest.php index 57320ba85d5..c3e064bb599 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'), [], [ + $tableDiff = new TableDiff(new Table('foo'), changedColumns: [ 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..7aedcc4759a 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: [ + 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: [ + 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, [], [ + $tableDiff = new TableDiff($table, changedColumns: [ 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..f8b19c1395b 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: [ + new ColumnDiff( + $table->getColumn('id'), + new Column('key', Type::getType(Types::INTEGER)), + ), + 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..d3a45c154f8 100644 --- a/tests/Schema/AbstractComparatorTestCase.php +++ b/tests/Schema/AbstractComparatorTestCase.php @@ -179,7 +179,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 +355,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 +471,9 @@ 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()); + self::assertEquals('id', current($modifiedColumns)->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');