From a1126d6a973055acfecddf20974e408377e6afc9 Mon Sep 17 00:00:00 2001 From: Fabrizio Balliano Date: Fri, 24 Jul 2026 22:21:22 +0100 Subject: [PATCH 1/4] fix(MySQL): combine the drop and the creation of an index over the same columns When an index is dropped and another one over the same columns is added, AbstractMySQLPlatform combines both into a single ALTER TABLE statement, so that the columns are never left unindexed. It compares the two column lists through the legacy Index::getColumns() accessor, which returns the raw map keys: the introspection API marks them as quoted ('"parent_id"'), while an index built in memory keeps them bare ('parent_id'). The two lists therefore never match when the dropped index comes from an introspected table, the statements are not combined, and the drop is emitted on its own. That makes the alteration fail whenever the dropped index is the one backing a foreign key: InnoDB rejects the drop with "Cannot drop index 'X': needed in a foreign key constraint" (which disabling foreign_key_checks does not lift), even though the added index would provide the same cover. Read the column names through the non-deprecated Index::getIndexedColumns() accessor and compare their unquoted identifier values, so the comparison is agnostic of whether the index was introspected or built in memory, as done in #7392 for the same accessor in the SQLite table-recreation path. Signed-off-by: Fabrizio Balliano --- src/Platforms/AbstractMySQLPlatform.php | 24 +++++++++- .../Schema/MySQLSchemaManagerTest.php | 48 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 04e903f397..7dbe52f5f7 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\Platforms\MySQL\MySQLMetadataProvider; use Doctrine\DBAL\Schema\ForeignKeyConstraint; use Doctrine\DBAL\Schema\Index; +use Doctrine\DBAL\Schema\Index\IndexedColumn; use Doctrine\DBAL\Schema\MySQLSchemaManager; use Doctrine\DBAL\Schema\Name\UnquotedIdentifierFolding; use Doctrine\DBAL\Schema\TableDiff; @@ -466,8 +467,10 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array foreach ($diff->getDroppedIndexes() as $droppedIndex) { $sql = array_merge($sql, $this->getPreAlterTableAlterPrimaryKeySQL($diff, $droppedIndex)); + $droppedIndexColumnNames = self::getUnquotedIndexedColumnNames($droppedIndex); + foreach ($diff->getAddedIndexes() as $addedIndex) { - if ($droppedIndex->getColumns() !== $addedIndex->getColumns()) { + if ($droppedIndexColumnNames !== self::getUnquotedIndexedColumnNames($addedIndex)) { continue; } @@ -500,6 +503,25 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array ); } + /** + * Returns the names of the columns the index spans, unquoted. + * + * The names are read via the non-deprecated API so that the result does not + * depend on whether the index was introspected, in which case the + * introspection marks its column names as quoted, or built in memory. + * + * @return list + */ + private static function getUnquotedIndexedColumnNames(Index $index): array + { + return array_map( + static function (IndexedColumn $indexedColumn): string { + return $indexedColumn->getColumnName()->getIdentifier()->getValue(); + }, + $index->getIndexedColumns(), + ); + } + /** @return list */ private function getPreAlterTableAlterPrimaryKeySQL(TableDiff $diff, Index $index): array { diff --git a/tests/Functional/Schema/MySQLSchemaManagerTest.php b/tests/Functional/Schema/MySQLSchemaManagerTest.php index b8a91e64ce..5b7138e25e 100644 --- a/tests/Functional/Schema/MySQLSchemaManagerTest.php +++ b/tests/Functional/Schema/MySQLSchemaManagerTest.php @@ -894,6 +894,54 @@ public function testMigrateOldBoolean(): void self::assertTrue($diff->isEmpty(), 'Tables should be identical.'); } + public function testAlterIntrospectedTableReplacesIndexBackingForeignKey(): void + { + $this->connection->executeStatement('DROP TABLE IF EXISTS index_replacement_child'); + $this->connection->executeStatement('DROP TABLE IF EXISTS index_replacement_parent'); + $this->connection->executeStatement( + 'CREATE TABLE index_replacement_parent (id INT NOT NULL, PRIMARY KEY (id)) ENGINE = InnoDB', + ); + + // The unique key is the only index covering the foreign key column, so + // InnoDB refuses to drop it unless another index takes over the cover + // in the same statement. + $this->connection->executeStatement(<<<'SQL' + CREATE TABLE index_replacement_child ( + id INT NOT NULL, + parent_id INT NOT NULL, + val INT NOT NULL, + PRIMARY KEY (id), + UNIQUE KEY uniq_parent_val (parent_id, val), + CONSTRAINT fk_index_replacement FOREIGN KEY (parent_id) + REFERENCES index_replacement_parent (id) + ) ENGINE = InnoDB + SQL); + + $oldTable = $this->schemaManager->introspectTableByUnquotedName('index_replacement_child'); + $newTable = $oldTable->edit() + ->setIndexes( + Index::editor() + ->setUnquotedName('idx_parent_val') + ->setUnquotedColumnNames('parent_id', 'val') + ->create(), + ) + ->create(); + + $diff = $this->schemaManager->createComparator()->compareTables($oldTable, $newTable); + + // Dropping the unique key and adding its replacement have to be combined + // into a single ALTER TABLE statement, which they are not when the + // dropped index comes from introspection and its column names are + // therefore quoted. + $this->schemaManager->alterTable($diff); + + $table = $this->schemaManager->introspectTableByUnquotedName('index_replacement_child'); + + self::assertFalse($table->hasIndex('uniq_parent_val')); + self::assertTrue($table->hasIndex('idx_parent_val')); + self::assertSame(IndexType::REGULAR, $table->getIndex('idx_parent_val')->getType()); + } + public function getExpectedDefaultSchemaName(): ?string { return null; From 8f49534f73d830956ff2f44e1a9ec687cbb56cbd Mon Sep 17 00:00:00 2001 From: Fabrizio Balliano Date: Sat, 1 Aug 2026 21:39:05 +0100 Subject: [PATCH 2/4] address review: compare index columns via UnqualifiedName::equals(), make the test platform-agnostic The dropped and added index columns are now compared pairwise through UnqualifiedName::equals() with the platform's unquoted-identifier folding, instead of comparing extracted values. The test moves to SchemaManagerTest and builds the tables through the editor API, with no platform-specific SQL. The unique index spans exactly the referencing column of the foreign key, so that no implicit index is created and the unique index remains the only cover of the constraint: with an extra covering index, InnoDB would allow the uncombined drop and the test would pass even without the fix. Verified failing with error 1553 on MySQL without the fix. Signed-off-by: Fabrizio Balliano --- src/Platforms/AbstractMySQLPlatform.php | 37 ++++----- .../Schema/MySQLSchemaManagerTest.php | 48 ----------- tests/Functional/Schema/SchemaManagerTest.php | 81 +++++++++++++++++++ 3 files changed, 100 insertions(+), 66 deletions(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 7dbe52f5f7..e1b4e77766 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -11,7 +11,6 @@ use Doctrine\DBAL\Platforms\MySQL\MySQLMetadataProvider; use Doctrine\DBAL\Schema\ForeignKeyConstraint; use Doctrine\DBAL\Schema\Index; -use Doctrine\DBAL\Schema\Index\IndexedColumn; use Doctrine\DBAL\Schema\MySQLSchemaManager; use Doctrine\DBAL\Schema\Name\UnquotedIdentifierFolding; use Doctrine\DBAL\Schema\TableDiff; @@ -467,10 +466,8 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array foreach ($diff->getDroppedIndexes() as $droppedIndex) { $sql = array_merge($sql, $this->getPreAlterTableAlterPrimaryKeySQL($diff, $droppedIndex)); - $droppedIndexColumnNames = self::getUnquotedIndexedColumnNames($droppedIndex); - foreach ($diff->getAddedIndexes() as $addedIndex) { - if ($droppedIndexColumnNames !== self::getUnquotedIndexedColumnNames($addedIndex)) { + if (! $this->indexesSpanSameColumns($droppedIndex, $addedIndex)) { continue; } @@ -504,22 +501,26 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array } /** - * Returns the names of the columns the index spans, unquoted. - * - * The names are read via the non-deprecated API so that the result does not - * depend on whether the index was introspected, in which case the - * introspection marks its column names as quoted, or built in memory. - * - * @return list + * Returns whether the two indexes span the same columns in the same order. */ - private static function getUnquotedIndexedColumnNames(Index $index): array + private function indexesSpanSameColumns(Index $index1, Index $index2): bool { - return array_map( - static function (IndexedColumn $indexedColumn): string { - return $indexedColumn->getColumnName()->getIdentifier()->getValue(); - }, - $index->getIndexedColumns(), - ); + $columns1 = $index1->getIndexedColumns(); + $columns2 = $index2->getIndexedColumns(); + + if (count($columns1) !== count($columns2)) { + return false; + } + + $folding = $this->getUnquotedIdentifierFolding(); + + foreach ($columns1 as $i => $column1) { + if (! $column1->getColumnName()->equals($columns2[$i]->getColumnName(), $folding)) { + return false; + } + } + + return true; } /** @return list */ diff --git a/tests/Functional/Schema/MySQLSchemaManagerTest.php b/tests/Functional/Schema/MySQLSchemaManagerTest.php index 5b7138e25e..b8a91e64ce 100644 --- a/tests/Functional/Schema/MySQLSchemaManagerTest.php +++ b/tests/Functional/Schema/MySQLSchemaManagerTest.php @@ -894,54 +894,6 @@ public function testMigrateOldBoolean(): void self::assertTrue($diff->isEmpty(), 'Tables should be identical.'); } - public function testAlterIntrospectedTableReplacesIndexBackingForeignKey(): void - { - $this->connection->executeStatement('DROP TABLE IF EXISTS index_replacement_child'); - $this->connection->executeStatement('DROP TABLE IF EXISTS index_replacement_parent'); - $this->connection->executeStatement( - 'CREATE TABLE index_replacement_parent (id INT NOT NULL, PRIMARY KEY (id)) ENGINE = InnoDB', - ); - - // The unique key is the only index covering the foreign key column, so - // InnoDB refuses to drop it unless another index takes over the cover - // in the same statement. - $this->connection->executeStatement(<<<'SQL' - CREATE TABLE index_replacement_child ( - id INT NOT NULL, - parent_id INT NOT NULL, - val INT NOT NULL, - PRIMARY KEY (id), - UNIQUE KEY uniq_parent_val (parent_id, val), - CONSTRAINT fk_index_replacement FOREIGN KEY (parent_id) - REFERENCES index_replacement_parent (id) - ) ENGINE = InnoDB - SQL); - - $oldTable = $this->schemaManager->introspectTableByUnquotedName('index_replacement_child'); - $newTable = $oldTable->edit() - ->setIndexes( - Index::editor() - ->setUnquotedName('idx_parent_val') - ->setUnquotedColumnNames('parent_id', 'val') - ->create(), - ) - ->create(); - - $diff = $this->schemaManager->createComparator()->compareTables($oldTable, $newTable); - - // Dropping the unique key and adding its replacement have to be combined - // into a single ALTER TABLE statement, which they are not when the - // dropped index comes from introspection and its column names are - // therefore quoted. - $this->schemaManager->alterTable($diff); - - $table = $this->schemaManager->introspectTableByUnquotedName('index_replacement_child'); - - self::assertFalse($table->hasIndex('uniq_parent_val')); - self::assertTrue($table->hasIndex('idx_parent_val')); - self::assertSame(IndexType::REGULAR, $table->getIndex('idx_parent_val')->getType()); - } - public function getExpectedDefaultSchemaName(): ?string { return null; diff --git a/tests/Functional/Schema/SchemaManagerTest.php b/tests/Functional/Schema/SchemaManagerTest.php index 4981ae00ac..8711a71198 100644 --- a/tests/Functional/Schema/SchemaManagerTest.php +++ b/tests/Functional/Schema/SchemaManagerTest.php @@ -505,6 +505,87 @@ public function testDropForeignKey(): void ); } + public function testReplaceIndexBackingForeignKey(): void + { + $this->dropTableIfExists('index_replacement_child'); + $this->dropTableIfExists('index_replacement_parent'); + + $parentTable = Table::editor() + ->setUnquotedName('index_replacement_parent') + ->setColumns( + Column::editor() + ->setUnquotedName('id') + ->setTypeName(Types::INTEGER) + ->create(), + ) + ->setPrimaryKeyConstraint( + PrimaryKeyConstraint::editor() + ->setUnquotedColumnNames('id') + ->create(), + ) + ->create(); + + // The unique index spans exactly the referencing column of the foreign key, so no implicit + // index is created, and it remains the only index covering the constraint. On MySQL, InnoDB + // refuses to drop such an index unless another index takes over the cover in the same + // ALTER TABLE statement. + $childTable = Table::editor() + ->setUnquotedName('index_replacement_child') + ->setColumns( + Column::editor() + ->setUnquotedName('id') + ->setTypeName(Types::INTEGER) + ->create(), + Column::editor() + ->setUnquotedName('parent_id') + ->setTypeName(Types::INTEGER) + ->create(), + ) + ->setPrimaryKeyConstraint( + PrimaryKeyConstraint::editor() + ->setUnquotedColumnNames('id') + ->create(), + ) + ->setIndexes( + Index::editor() + ->setUnquotedName('uniq_parent_id') + ->setUnquotedColumnNames('parent_id') + ->setType(IndexType::UNIQUE) + ->create(), + ) + ->setForeignKeyConstraints( + ForeignKeyConstraint::editor() + ->setUnquotedName('fk_index_replacement') + ->setUnquotedReferencingColumnNames('parent_id') + ->setUnquotedReferencedTableName('index_replacement_parent') + ->setUnquotedReferencedColumnNames('id') + ->create(), + ) + ->create(); + + $this->schemaManager->createTable($parentTable); + $this->schemaManager->createTable($childTable); + + $oldTable = $this->schemaManager->introspectTableByUnquotedName('index_replacement_child'); + + $newIndex = Index::editor() + ->setUnquotedName('idx_parent_id') + ->setUnquotedColumnNames('parent_id') + ->create(); + + $newTable = $oldTable->edit() + ->setIndexes($newIndex) + ->create(); + + $diff = $this->schemaManager->createComparator()->compareTables($oldTable, $newTable); + $this->schemaManager->alterTable($diff); + + $table = $this->schemaManager->introspectTableByUnquotedName('index_replacement_child'); + + self::assertFalse($table->hasIndex('uniq_parent_id')); + $this->assertIndexEquals($newIndex, $table->getIndex('idx_parent_id')); + } + /** @param callable(AbstractSchemaManager): list $introspect */ #[DataProvider('quotedAndUnquotedIndexIntrospection')] public function testIntrospectTableIndexes( From 28a57f6942343dcb0fba7defd5f6a12674cbe71c Mon Sep 17 00:00:00 2001 From: Fabrizio Balliano Date: Sun, 2 Aug 2026 09:21:02 +0100 Subject: [PATCH 3/4] address review: compare index columns via Index::getUnquotedColumns() Index::getIndexedColumns() throws InvalidState on an index without valid columns, which would be a behavior break in 4.x. Compare the lists returned by Index::getUnquotedColumns() instead, as done in the SQLite table-recreation path. Signed-off-by: Fabrizio Balliano --- src/Platforms/AbstractMySQLPlatform.php | 27 +++---------------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index e1b4e77766..ee969238ca 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -466,8 +466,10 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array foreach ($diff->getDroppedIndexes() as $droppedIndex) { $sql = array_merge($sql, $this->getPreAlterTableAlterPrimaryKeySQL($diff, $droppedIndex)); + $droppedIndexColumns = $droppedIndex->getUnquotedColumns(); + foreach ($diff->getAddedIndexes() as $addedIndex) { - if (! $this->indexesSpanSameColumns($droppedIndex, $addedIndex)) { + if ($droppedIndexColumns !== $addedIndex->getUnquotedColumns()) { continue; } @@ -500,29 +502,6 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array ); } - /** - * Returns whether the two indexes span the same columns in the same order. - */ - private function indexesSpanSameColumns(Index $index1, Index $index2): bool - { - $columns1 = $index1->getIndexedColumns(); - $columns2 = $index2->getIndexedColumns(); - - if (count($columns1) !== count($columns2)) { - return false; - } - - $folding = $this->getUnquotedIdentifierFolding(); - - foreach ($columns1 as $i => $column1) { - if (! $column1->getColumnName()->equals($columns2[$i]->getColumnName(), $folding)) { - return false; - } - } - - return true; - } - /** @return list */ private function getPreAlterTableAlterPrimaryKeySQL(TableDiff $diff, Index $index): array { From bd7d4d314b348897e0b78f60e5d57f2a9887e6fa Mon Sep 17 00:00:00 2001 From: Fabrizio Balliano Date: Sun, 2 Aug 2026 09:22:39 +0100 Subject: [PATCH 4/4] inline the index column comparison Signed-off-by: Fabrizio Balliano --- src/Platforms/AbstractMySQLPlatform.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index ee969238ca..529d9441a8 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -466,10 +466,8 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array foreach ($diff->getDroppedIndexes() as $droppedIndex) { $sql = array_merge($sql, $this->getPreAlterTableAlterPrimaryKeySQL($diff, $droppedIndex)); - $droppedIndexColumns = $droppedIndex->getUnquotedColumns(); - foreach ($diff->getAddedIndexes() as $addedIndex) { - if ($droppedIndexColumns !== $addedIndex->getUnquotedColumns()) { + if ($droppedIndex->getUnquotedColumns() !== $addedIndex->getUnquotedColumns()) { continue; }