Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DBAL-1062] Fix renaming indexes used by foreign key constraints #756

Merged
merged 1 commit into from
Dec 26, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions lib/Doctrine/DBAL/Platforms/MySQL57Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}
*/
Expand Down
93 changes: 92 additions & 1 deletion lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
23 changes: 23 additions & 0 deletions lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
);
}
}
33 changes: 33 additions & 0 deletions tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
);
}
}
10 changes: 10 additions & 0 deletions tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
}
}
10 changes: 10 additions & 0 deletions tests/Doctrine/Tests/DBAL/Platforms/MySQL57PlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
}
}
10 changes: 10 additions & 0 deletions tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
}
}
10 changes: 10 additions & 0 deletions tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
}
}
18 changes: 18 additions & 0 deletions tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
);
}
}
Loading