From 2588762739714e931f2c005ca10d213eacf7454f Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Tue, 11 Mar 2025 07:05:31 -0700 Subject: [PATCH 1/3] Drop and create primary key in a single statement --- src/Platforms/AbstractMySQLPlatform.php | 10 ++++++++-- tests/Functional/Schema/AlterTableTest.php | 14 -------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 6a1d6326f07..544c953eb42 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -377,11 +377,17 @@ public function getAlterTableSQL(TableDiff $diff): array unset($addedIndexes['primary']); $diffModified = true; } elseif (isset($modifiedIndexes['primary'])) { + $oldTable = $diff->getOldTable(); $addedColumns = $this->indexAssetsByLowerCaseName($diff->getAddedColumns()); - // Necessary in case the new primary key includes a new auto_increment column + // Necessary in case the new primary key includes an auto_increment column foreach ($modifiedIndexes['primary']->getColumns() as $columnName) { - if (isset($addedColumns[$columnName]) && $addedColumns[$columnName]->getAutoincrement()) { + if ( + (isset($addedColumns[$columnName]) && $addedColumns[$columnName]->getAutoincrement()) + || ( + $oldTable->hasColumn($columnName) + && $oldTable->getColumn($columnName)->getAutoincrement()) + ) { $keyColumns = array_values(array_unique($modifiedIndexes['primary']->getColumns())); $queryParts[] = 'DROP PRIMARY KEY'; $queryParts[] = 'ADD PRIMARY KEY (' . implode(', ', $keyColumns) . ')'; diff --git a/tests/Functional/Schema/AlterTableTest.php b/tests/Functional/Schema/AlterTableTest.php index 50f11acaef9..adf59a5ac50 100644 --- a/tests/Functional/Schema/AlterTableTest.php +++ b/tests/Functional/Schema/AlterTableTest.php @@ -111,13 +111,6 @@ public function testDropPrimaryKeyWithAutoincrementColumn(): void public function testDropNonAutoincrementColumnFromCompositePrimaryKeyWithAutoincrementColumn(): void { - if ($this->connection->getDatabasePlatform() instanceof AbstractMySQLPlatform) { - self::markTestIncomplete( - 'DBAL does not restore the auto-increment attribute after dropping and adding the constraint,' - . ' which is a bug.', - ); - } - $this->ensureDroppingPrimaryKeyConstraintIsSupported(); $table = new Table('alter_pk'); @@ -135,13 +128,6 @@ public function testAddNonAutoincrementColumnToPrimaryKeyWithAutoincrementColumn { $platform = $this->connection->getDatabasePlatform(); - if ($platform instanceof AbstractMySQLPlatform) { - self::markTestIncomplete( - 'DBAL does not restore the auto-increment attribute after dropping and adding the constraint,' - . ' which is a bug.', - ); - } - if ($platform instanceof SQLitePlatform) { self::markTestSkipped( 'SQLite does not support auto-increment columns as part of composite primary key constraint', From 87c7b4d51249e4cb57a314a2fed7f3da47827604 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Tue, 11 Mar 2025 07:54:07 -0700 Subject: [PATCH 2/3] Remove special handling for primary keys covering an auto-increment column --- src/Platforms/AbstractMySQLPlatform.php | 41 ++++++++----------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 544c953eb42..f1c4fe1d383 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -9,7 +9,6 @@ use Doctrine\DBAL\Exception\InvalidColumnType\ColumnValuesRequired; use Doctrine\DBAL\Platforms\Keywords\KeywordList; use Doctrine\DBAL\Platforms\Keywords\MySQLKeywords; -use Doctrine\DBAL\Schema\AbstractAsset; use Doctrine\DBAL\Schema\ForeignKeyConstraint; use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\MySQLSchemaManager; @@ -367,8 +366,8 @@ public function getAlterTableSQL(TableDiff $diff): array . $this->getColumnDeclarationSQL($newColumn->getQuotedName($this), $newColumnProperties); } - $addedIndexes = $this->indexAssetsByLowerCaseName($diff->getAddedIndexes()); - $modifiedIndexes = $this->indexAssetsByLowerCaseName($diff->getModifiedIndexes()); + $addedIndexes = $this->indexIndexesByLowerCaseName($diff->getAddedIndexes()); + $modifiedIndexes = $this->indexIndexesByLowerCaseName($diff->getModifiedIndexes()); $diffModified = false; if (isset($addedIndexes['primary'])) { @@ -377,25 +376,11 @@ public function getAlterTableSQL(TableDiff $diff): array unset($addedIndexes['primary']); $diffModified = true; } elseif (isset($modifiedIndexes['primary'])) { - $oldTable = $diff->getOldTable(); - $addedColumns = $this->indexAssetsByLowerCaseName($diff->getAddedColumns()); - - // Necessary in case the new primary key includes an auto_increment column - foreach ($modifiedIndexes['primary']->getColumns() as $columnName) { - if ( - (isset($addedColumns[$columnName]) && $addedColumns[$columnName]->getAutoincrement()) - || ( - $oldTable->hasColumn($columnName) - && $oldTable->getColumn($columnName)->getAutoincrement()) - ) { - $keyColumns = array_values(array_unique($modifiedIndexes['primary']->getColumns())); - $queryParts[] = 'DROP PRIMARY KEY'; - $queryParts[] = 'ADD PRIMARY KEY (' . implode(', ', $keyColumns) . ')'; - unset($modifiedIndexes['primary']); - $diffModified = true; - break; - } - } + $keyColumns = array_values(array_unique($modifiedIndexes['primary']->getColumns())); + $queryParts[] = 'DROP PRIMARY KEY'; + $queryParts[] = 'ADD PRIMARY KEY (' . implode(', ', $keyColumns) . ')'; + unset($modifiedIndexes['primary']); + $diffModified = true; } if ($diffModified) { @@ -839,18 +824,16 @@ public function createSchemaManager(Connection $connection): MySQLSchemaManager } /** - * @param array $assets - * - * @return array + * @param array $indexes * - * @template T of AbstractAsset + * @return array */ - private function indexAssetsByLowerCaseName(array $assets): array + private function indexIndexesByLowerCaseName(array $indexes): array { $result = []; - foreach ($assets as $asset) { - $result[strtolower($asset->getName())] = $asset; + foreach ($indexes as $index) { + $result[strtolower($index->getName())] = $index; } return $result; From bb9615729887cfbb621d447bd1de982dbc31a0b7 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Tue, 11 Mar 2025 07:54:41 -0700 Subject: [PATCH 3/3] Ensure that the migration diff is never empty --- tests/Functional/Schema/AlterTableTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Functional/Schema/AlterTableTest.php b/tests/Functional/Schema/AlterTableTest.php index adf59a5ac50..8ba92b8b624 100644 --- a/tests/Functional/Schema/AlterTableTest.php +++ b/tests/Functional/Schema/AlterTableTest.php @@ -230,6 +230,8 @@ private function testMigration(Table $oldTable, callable $migration): void $diff = $schemaManager->createComparator() ->compareTables($oldTable, $newTable); + self::assertFalse($diff->isEmpty()); + $schemaManager->alterTable($diff); $introspectedTable = $schemaManager->introspectTable($newTable->getName());