From 8dadeb7ebcfdb1e37e126078260eebee2e401d75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steve=20M=C3=BCller?= Date: Wed, 24 Dec 2014 12:10:09 +0100 Subject: [PATCH] fix renaming indexes used by foreign key constraints --- .../DBAL/Platforms/MySQL57Platform.php | 17 ++++ lib/Doctrine/DBAL/Platforms/MySqlPlatform.php | 93 ++++++++++++++++++- .../DBAL/Platforms/SqlitePlatform.php | 6 ++ .../DBAL/Schema/ForeignKeyConstraint.php | 23 +++++ .../SchemaManagerFunctionalTestCase.php | 41 ++++++++ .../AbstractMySQLPlatformTestCase.php | 13 +++ .../Platforms/AbstractPlatformTestCase.php | 33 +++++++ .../AbstractPostgreSqlPlatformTestCase.php | 10 ++ .../AbstractSQLServerPlatformTestCase.php | 10 ++ .../Tests/DBAL/Platforms/DB2PlatformTest.php | 10 ++ .../DBAL/Platforms/MySQL57PlatformTest.php | 10 ++ .../DBAL/Platforms/OraclePlatformTest.php | 10 ++ .../Platforms/SQLAnywherePlatformTest.php | 10 ++ .../DBAL/Platforms/SqlitePlatformTest.php | 18 ++++ .../DBAL/Schema/ForeignKeyConstraintTest.php | 56 +++++++++++ 15 files changed, 359 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/Tests/DBAL/Schema/ForeignKeyConstraintTest.php diff --git a/lib/Doctrine/DBAL/Platforms/MySQL57Platform.php b/lib/Doctrine/DBAL/Platforms/MySQL57Platform.php index 6f7102eb39b..36b525e2063 100644 --- a/lib/Doctrine/DBAL/Platforms/MySQL57Platform.php +++ b/lib/Doctrine/DBAL/Platforms/MySQL57Platform.php @@ -20,6 +20,7 @@ namespace Doctrine\DBAL\Platforms; use Doctrine\DBAL\Schema\Index; +use Doctrine\DBAL\Schema\TableDiff; /** * Provides the behavior, features and SQL dialect of the MySQL 5.7 database platform. @@ -30,6 +31,22 @@ */ class MySQL57Platform extends MySqlPlatform { + /** + * {@inheritdoc} + */ + protected function getPreAlterTableRenameIndexForeignKeySQL(TableDiff $diff) + { + return array(); + } + + /** + * {@inheritdoc} + */ + protected function getPostAlterTableRenameIndexForeignKeySQL(TableDiff $diff) + { + return array(); + } + /** * {@inheritdoc} */ diff --git a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php b/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php index 5ab8193f044..a7ee12c53ee 100644 --- a/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php @@ -667,7 +667,98 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff) $diff->removedForeignKeys = array(); } - $sql = array_merge($sql, parent::getPreAlterTableIndexForeignKeySQL($diff)); + $sql = array_merge( + $sql, + parent::getPreAlterTableIndexForeignKeySQL($diff), + $this->getPreAlterTableRenameIndexForeignKeySQL($diff) + ); + + return $sql; + } + + /** + * @param TableDiff $diff The table diff to gather the SQL for. + * + * @return array + */ + protected function getPreAlterTableRenameIndexForeignKeySQL(TableDiff $diff) + { + $sql = array(); + $tableName = $diff->getName($this)->getQuotedName($this); + + foreach ($this->getRemainingForeignKeyConstraintsRequiringRenamedIndexes($diff) as $foreignKey) { + if (! in_array($foreignKey, $diff->changedForeignKeys, true)) { + $sql[] = $this->getDropForeignKeySQL($foreignKey, $tableName); + } + } + + return $sql; + } + + /** + * Returns the remaining foreign key constraints that require one of the renamed indexes. + * + * "Remaining" here refers to the diff between the foreign keys currently defined in the associated + * table and the foreign keys to be removed. + * + * @param TableDiff $diff The table diff to evaluate. + * + * @return array + */ + private function getRemainingForeignKeyConstraintsRequiringRenamedIndexes(TableDiff $diff) + { + if (empty($diff->renamedIndexes) || ! $diff->fromTable instanceof Table) { + return array(); + } + + $foreignKeys = array(); + /** @var \Doctrine\DBAL\Schema\ForeignKeyConstraint[] $remainingForeignKeys */ + $remainingForeignKeys = array_diff_key( + $diff->fromTable->getForeignKeys(), + $diff->removedForeignKeys + ); + + foreach ($remainingForeignKeys as $foreignKey) { + foreach ($diff->renamedIndexes as $index) { + if ($foreignKey->intersectsIndexColumns($index)) { + $foreignKeys[] = $foreignKey; + + break; + } + } + } + + return $foreignKeys; + } + + /** + * {@inheritdoc} + */ + protected function getPostAlterTableIndexForeignKeySQL(TableDiff $diff) + { + return array_merge( + parent::getPostAlterTableIndexForeignKeySQL($diff), + $this->getPostAlterTableRenameIndexForeignKeySQL($diff) + ); + } + + /** + * @param TableDiff $diff The table diff to gather the SQL for. + * + * @return array + */ + protected function getPostAlterTableRenameIndexForeignKeySQL(TableDiff $diff) + { + $sql = array(); + $tableName = (false !== $diff->newName) + ? $diff->getNewName()->getQuotedName($this) + : $diff->getName($this)->getQuotedName($this); + + foreach ($this->getRemainingForeignKeyConstraintsRequiringRenamedIndexes($diff) as $foreignKey) { + if (! in_array($foreignKey, $diff->changedForeignKeys, true)) { + $sql[] = $this->getCreateForeignKeySQL($foreignKey, $tableName); + } + } return $sql; } diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php index 6a00a927552..cccb582305c 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -986,6 +986,12 @@ private function getIndexesInAlteredTable(TableDiff $diff) $columnNames = $this->getColumnNamesInAlteredTable($diff); foreach ($indexes as $key => $index) { + foreach ($diff->renamedIndexes as $oldIndexName => $renamedIndex) { + if (strtolower($key) === strtolower($oldIndexName)) { + unset($indexes[$key]); + } + } + $changed = false; $indexColumns = array(); foreach ($index->getColumns() as $columnName) { diff --git a/lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php b/lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php index 66f35f90705..6070eb2e5ba 100644 --- a/lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php +++ b/lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php @@ -363,4 +363,27 @@ private function onEvent($event) return false; } + + /** + * Checks whether this foreign key constraint intersects the given index columns. + * + * Returns `true` if at least one of this foreign key's local columns + * matches one of the given index's columns, `false` otherwise. + * + * @param Index $index The index to be checked against. + * + * @return boolean + */ + public function intersectsIndexColumns(Index $index) + { + foreach ($index->getColumns() as $indexColumn) { + foreach ($this->_localColumnNames as $localColumn) { + if (strtolower($indexColumn) === strtolower($localColumn->getName())) { + return true; + } + } + } + + return false; + } } diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php index a27fc356cfb..0f7e4028eb2 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php @@ -557,6 +557,47 @@ public function testUpdateSchemaWithForeignKeyRenaming() $this->_sm->alterTable($tableDiff); } + /** + * @group DBAL-1062 + */ + public function testRenameIndexUsedInForeignKeyConstraint() + { + if (! $this->_sm->getDatabasePlatform()->supportsForeignKeyConstraints()) { + $this->markTestSkipped('This test is only supported on platforms that have foreign keys.'); + } + + $primaryTable = new Table('test_rename_index_primary'); + $primaryTable->addColumn('id', 'integer'); + $primaryTable->setPrimaryKey(array('id')); + + $foreignTable = new Table('test_rename_index_foreign'); + $foreignTable->addColumn('fk', 'integer'); + $foreignTable->addIndex(array('fk'), 'rename_index_fk_idx'); + $foreignTable->addForeignKeyConstraint( + 'test_rename_index_primary', + array('fk'), + array('id'), + array(), + 'fk_constraint' + ); + + $this->_sm->dropAndCreateTable($primaryTable); + $this->_sm->dropAndCreateTable($foreignTable); + + $foreignTable2 = clone $foreignTable; + $foreignTable2->renameIndex('rename_index_fk_idx', 'renamed_index_fk_idx'); + + $comparator = new Comparator(); + + $this->_sm->alterTable($comparator->diffTable($foreignTable, $foreignTable2)); + + $foreignTable = $this->_sm->listTableDetails('test_rename_index_foreign'); + + $this->assertFalse($foreignTable->hasIndex('rename_index_fk_idx')); + $this->assertTrue($foreignTable->hasIndex('renamed_index_fk_idx')); + $this->assertTrue($foreignTable->hasForeignKey('fk_constraint')); + } + /** * @group DBAL-42 */ diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php index add3572afc5..b0ca92e622e 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php @@ -667,4 +667,17 @@ protected function getAlterStringToFixedStringSQL() 'ALTER TABLE mytable CHANGE name name CHAR(2) NOT NULL', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'ALTER TABLE mytable DROP FOREIGN KEY fk_foo', + 'DROP INDEX idx_foo ON mytable', + 'CREATE INDEX idx_foo_renamed ON mytable (foo)', + 'ALTER TABLE mytable ADD CONSTRAINT fk_foo FOREIGN KEY (foo) REFERENCES foreign_table (id)', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php index 0f0b3f46341..bd59d3cd247 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php @@ -1159,4 +1159,37 @@ public function testAlterStringToFixedString() * @return array */ abstract protected function getAlterStringToFixedStringSQL(); + + /** + * @group DBAL-1062 + */ + public function testGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + $foreignTable = new Table('foreign_table'); + $foreignTable->addColumn('id', 'integer'); + $foreignTable->setPrimaryKey(array('id')); + + $primaryTable = new Table('mytable'); + $primaryTable->addColumn('foo', 'integer'); + $primaryTable->addColumn('bar', 'integer'); + $primaryTable->addColumn('baz', 'integer'); + $primaryTable->addIndex(array('foo'), 'idx_foo'); + $primaryTable->addIndex(array('bar'), 'idx_bar'); + $primaryTable->addForeignKeyConstraint($foreignTable, array('foo'), array('id'), array(), 'fk_foo'); + $primaryTable->addForeignKeyConstraint($foreignTable, array('bar'), array('id'), array(), 'fk_bar'); + + $tableDiff = new TableDiff('mytable'); + $tableDiff->fromTable = $primaryTable; + $tableDiff->renamedIndexes['idx_foo'] = new Index('idx_foo_renamed', array('foo')); + + $this->assertSame( + $this->getGeneratesAlterTableRenameIndexUsedByForeignKeySQL(), + $this->_platform->getAlterTableSQL($tableDiff) + ); + } + + /** + * @return array + */ + abstract protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL(); } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPostgreSqlPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPostgreSqlPlatformTestCase.php index 210bd94ab19..47fe9347d60 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPostgreSqlPlatformTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPostgreSqlPlatformTestCase.php @@ -765,4 +765,14 @@ protected function getAlterStringToFixedStringSQL() 'ALTER TABLE mytable ALTER name TYPE CHAR(2)', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'ALTER INDEX idx_foo RENAME TO idx_foo_renamed', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php index 36e4bfa9eb1..7714a6c438b 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php @@ -1212,4 +1212,14 @@ protected function getAlterStringToFixedStringSQL() 'ALTER TABLE mytable ALTER COLUMN name NCHAR(2) NOT NULL', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + "EXEC sp_RENAME N'mytable.idx_foo', N'idx_foo_renamed', N'INDEX'", + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php index f21005a1180..257cb954d69 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php @@ -650,4 +650,14 @@ protected function getAlterStringToFixedStringSQL() 'CALL SYSPROC.ADMIN_CMD (\'REORG TABLE mytable\')', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'RENAME INDEX idx_foo TO idx_foo_renamed', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/MySQL57PlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/MySQL57PlatformTest.php index c75cf86b8b1..2fb87070d9a 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/MySQL57PlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/MySQL57PlatformTest.php @@ -55,4 +55,14 @@ protected function getQuotedAlterTableRenameIndexInSchemaSQL() 'ALTER TABLE `schema`.`table` RENAME INDEX `foo` TO `bar`', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'ALTER TABLE mytable RENAME INDEX idx_foo TO idx_foo_renamed', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php index 680fdc9c4a0..53493b97fe8 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php @@ -668,4 +668,14 @@ protected function getAlterStringToFixedStringSQL() 'ALTER TABLE mytable MODIFY (name CHAR(2) DEFAULT NULL)', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'ALTER INDEX idx_foo RENAME TO idx_foo_renamed', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php index ee538244ed3..0137ef7f0ae 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php @@ -961,4 +961,14 @@ protected function getAlterStringToFixedStringSQL() 'ALTER TABLE mytable ALTER name CHAR(2) NOT NULL', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'ALTER INDEX idx_foo ON mytable RENAME TO idx_foo_renamed', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php index 1d73c581a45..50e86d07a48 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php @@ -638,4 +638,22 @@ protected function getAlterStringToFixedStringSQL() 'DROP TABLE __temp__mytable', ); } + + /** + * {@inheritdoc} + */ + protected function getGeneratesAlterTableRenameIndexUsedByForeignKeySQL() + { + return array( + 'DROP INDEX idx_foo', + 'DROP INDEX idx_bar', + 'CREATE TEMPORARY TABLE __temp__mytable AS SELECT foo, bar, baz FROM mytable', + 'DROP TABLE mytable', + 'CREATE TABLE mytable (foo INTEGER NOT NULL, bar INTEGER NOT NULL, baz INTEGER NOT NULL, CONSTRAINT fk_foo FOREIGN KEY (foo) REFERENCES foreign_table (id) NOT DEFERRABLE INITIALLY IMMEDIATE, CONSTRAINT fk_bar FOREIGN KEY (bar) REFERENCES foreign_table (id) NOT DEFERRABLE INITIALLY IMMEDIATE)', + 'INSERT INTO mytable (foo, bar, baz) SELECT foo, bar, baz FROM __temp__mytable', + 'DROP TABLE __temp__mytable', + 'CREATE INDEX idx_bar ON mytable (bar)', + 'CREATE INDEX idx_foo_renamed ON mytable (foo)', + ); + } } diff --git a/tests/Doctrine/Tests/DBAL/Schema/ForeignKeyConstraintTest.php b/tests/Doctrine/Tests/DBAL/Schema/ForeignKeyConstraintTest.php new file mode 100644 index 00000000000..6bae18c378c --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Schema/ForeignKeyConstraintTest.php @@ -0,0 +1,56 @@ +getMockBuilder('Doctrine\DBAL\Schema\Index') + ->disableOriginalConstructor() + ->getMock(); + $index->expects($this->once()) + ->method('getColumns') + ->will($this->returnValue($indexColumns)); + + $this->assertSame($expectedResult, $foreignKey->intersectsIndexColumns($index)); + } + + /** + * @return array + */ + public function getIntersectsIndexColumnsData() + { + return array( + array(array('baz'), false), + array(array('baz', 'bloo'), false), + + array(array('foo'), true), + array(array('bar'), true), + + array(array('foo', 'bar'), true), + array(array('bar', 'foo'), true), + + array(array('foo', 'baz'), true), + array(array('baz', 'foo'), true), + + array(array('bar', 'baz'), true), + array(array('baz', 'bar'), true), + + array(array('foo', 'bloo', 'baz'), true), + array(array('bloo', 'foo', 'baz'), true), + array(array('bloo', 'baz', 'foo'), true), + + array(array('FOO'), true), + ); + } +}