Skip to content

Commit

Permalink
allow defining duplicate indexes based on columns and properties on a…
Browse files Browse the repository at this point in the history 
… table
  • Loading branch information
@deeky666
deeky666 committed Jan 3, 2015
1 parent 89d4f06 commit 62054e2
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 59 deletions.
84 changes: 60 additions & 24 deletions lib/Doctrine/DBAL/Schema/Comparator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,37 +238,37 @@ public function diffTable(Table $table1, Table $table2)
$table1Indexes = $table1->getIndexes();
$table2Indexes = $table2->getIndexes();

foreach ($table2Indexes as $index2Name => $index2Definition) {
foreach ($table1Indexes as $index1Name => $index1Definition) {
if ($this->diffIndex($index1Definition, $index2Definition) === false) {
if ( ! $index1Definition->isPrimary() && $index1Name != $index2Name) {
$tableDifferences->renamedIndexes[$index1Name] = $index2Definition;
$changes++;
}

unset($table1Indexes[$index1Name]);
unset($table2Indexes[$index2Name]);
} else {
if ($index1Name == $index2Name) {
$tableDifferences->changedIndexes[$index2Name] = $table2Indexes[$index2Name];
unset($table1Indexes[$index1Name]);
unset($table2Indexes[$index2Name]);
$changes++;
}
}
/* See if all the indexes in table 1 exist in table 2 */
foreach ($table2Indexes as $indexName => $index) {
if (($index->isPrimary() && $table1->hasPrimaryKey()) || $table1->hasIndex($indexName)) {
continue;
}
}

foreach ($table1Indexes as $index1Name => $index1Definition) {
$tableDifferences->removedIndexes[$index1Name] = $index1Definition;
$tableDifferences->addedIndexes[$indexName] = $index;
$changes++;
}
/* See if there are any removed indexes in table 2 */
foreach ($table1Indexes as $indexName => $index) {
// See if index is removed in table 2.
if (($index->isPrimary() && ! $table2->hasPrimaryKey()) ||
! $index->isPrimary() && ! $table2->hasIndex($indexName)
) {
$tableDifferences->removedIndexes[$indexName] = $index;
$changes++;
continue;
}

foreach ($table2Indexes as $index2Name => $index2Definition) {
$tableDifferences->addedIndexes[$index2Name] = $index2Definition;
$changes++;
// See if index has changed in table 2.
$table2Index = $index->isPrimary() ? $table2->getPrimaryKey() : $table2->getIndex($indexName);

if ($this->diffIndex($index, $table2Index)) {
$tableDifferences->changedIndexes[$indexName] = $table2Index;
$changes++;
}
}

$this->detectIndexRenamings($tableDifferences);

$fromFkeys = $table1->getForeignKeys();
$toFkeys = $table2->getForeignKeys();

Expand Down Expand Up @@ -335,6 +335,42 @@ private function detectColumnRenamings(TableDiff $tableDifferences)
}
}

/**
* Try to find indexes 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 \Doctrine\DBAL\Schema\TableDiff $tableDifferences
*
* @return void
*/
private function detectIndexRenamings(TableDiff $tableDifferences)
{
$renameCandidates = array();

foreach ($tableDifferences->addedIndexes as $addedIndexName => $addedIndex) {
foreach ($tableDifferences->removedIndexes as $removedIndex) {
if (! $this->diffIndex($addedIndex, $removedIndex)) {
$renameCandidates[$addedIndex->getName()][] = array($removedIndex, $addedIndex, $addedIndexName);
}
}
}

foreach ($renameCandidates as $candidateIndexes) {
if (count($candidateIndexes) === 1) {
list($removedIndex, $addedIndex) = $candidateIndexes[0];

$removedIndexName = strtolower($removedIndex->getName());
$addedIndexName = strtolower($addedIndex->getName());

if (! isset($tableDifferences->renamedIndexes[$removedIndexName])) {
$tableDifferences->renamedIndexes[$removedIndexName] = $addedIndex;
unset($tableDifferences->addedIndexes[$addedIndexName]);
unset($tableDifferences->removedIndexes[$removedIndexName]);
}
}
}
}

/**
* @param \Doctrine\DBAL\Schema\ForeignKeyConstraint $key1
* @param \Doctrine\DBAL\Schema\ForeignKeyConstraint $key2
Expand Down
28 changes: 14 additions & 14 deletions lib/Doctrine/DBAL/Schema/Table.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,27 +489,13 @@ protected function _addColumn(Column $column)
*/
protected function _addIndex(Index $indexCandidate)
{
// check for duplicates
foreach ($this->_indexes as $existingIndex) {
if ($indexCandidate->isFullfilledBy($existingIndex)) {
return $this;
}
}

$indexName = $indexCandidate->getName();
$indexName = $this->normalizeIdentifier($indexName);

if (isset($this->_indexes[$indexName]) || ($this->_primaryKeyName != false && $indexCandidate->isPrimary())) {
throw SchemaException::indexAlreadyExists($indexName, $this->_name);
}

// remove overruled indexes
foreach ($this->_indexes as $idxKey => $existingIndex) {
if ($indexCandidate->overrules($existingIndex)) {
unset($this->_indexes[$idxKey]);
}
}

if ($indexCandidate->isPrimary()) {
$this->_primaryKeyName = $indexName;
}
Expand Down Expand Up @@ -538,9 +524,23 @@ protected function _addForeignKeyConstraint(ForeignKeyConstraint $constraint)
$name = $this->normalizeIdentifier($name);

$this->_fkConstraints[$name] = $constraint;

// add an explicit index on the foreign key columns. If there is already an index that fulfils this requirements drop the request.
// In the case of __construct calling this method during hydration from schema-details all the explicitly added indexes
// lead to duplicates. This creates computation overhead in this case, however no duplicate indexes are ever added (based on columns).
$indexName = $this->_generateIdentifierName(
array_merge(array($this->getName()), $constraint->getColumns()),
"idx",
$this->_getMaxIdentifierLength()
);
$indexCandidate = $this->_createIndex($constraint->getColumns(), $indexName, false, false);

foreach ($this->_indexes as $existingIndex) {
if ($indexCandidate->isFullfilledBy($existingIndex)) {
return;
}
}

$this->addIndex($constraint->getColumns());
}

Expand Down
53 changes: 53 additions & 0 deletions tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,59 @@ public function testDetectRenameColumnAmbiguous()
$this->assertEquals(0, count($tableDiff->renamedColumns), "no renamings should take place.");
}

/**
* @group DBAL-1063
*/
public function testDetectRenameIndex()
{
$table1 = new Table('foo');
$table1->addColumn('foo', 'integer');

$table2 = clone $table1;

$table1->addIndex(array('foo'), 'idx_foo');

$table2->addIndex(array('foo'), 'idx_bar');

$comparator = new Comparator();
$tableDiff = $comparator->diffTable($table1, $table2);

$this->assertCount(0, $tableDiff->addedIndexes);
$this->assertCount(0, $tableDiff->removedIndexes);
$this->assertArrayHasKey('idx_foo', $tableDiff->renamedIndexes);
$this->assertEquals('idx_bar', $tableDiff->renamedIndexes['idx_foo']->getName());
}

/**
* You can easily have ambiguities in the index renaming. If these
* are detected no renaming should take place, instead adding and dropping
* should be used exclusively.
*
* @group DBAL-1063
*/
public function testDetectRenameIndexAmbiguous()
{
$table1 = new Table('foo');
$table1->addColumn('foo', 'integer');

$table2 = clone $table1;

$table1->addIndex(array('foo'), 'idx_foo');
$table1->addIndex(array('foo'), 'idx_bar');

$table2->addIndex(array('foo'), 'idx_baz');

$comparator = new Comparator();
$tableDiff = $comparator->diffTable($table1, $table2);

$this->assertCount(1, $tableDiff->addedIndexes);
$this->assertArrayHasKey('idx_baz', $tableDiff->addedIndexes);
$this->assertCount(2, $tableDiff->removedIndexes);
$this->assertArrayHasKey('idx_foo', $tableDiff->removedIndexes);
$this->assertArrayHasKey('idx_bar', $tableDiff->removedIndexes);
$this->assertCount(0, $tableDiff->renamedIndexes);
}

public function testDetectChangeIdentifierType()
{
$this->markTestSkipped('DBAL-2 was reopened, this test cannot work anymore.');
Expand Down
115 changes: 94 additions & 21 deletions tests/Doctrine/Tests/DBAL/Schema/TableTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,19 +351,6 @@ public function testAllowImplicitSchemaTableInAutogeneratedIndexNames()
$this->assertEquals(1, count($table->getIndexes()));
}

/**
* @group DBAL-50
*/
public function testAddIndexTwice_IgnoreSecond()
{
$table = new Table("foo.bar");
$table->addColumn('baz', 'integer', array());
$table->addIndex(array('baz'));
$table->addIndex(array('baz'));

$this->assertEquals(1, count($table->getIndexes()));
}

/**
* @group DBAL-50
*/
Expand All @@ -385,10 +372,57 @@ public function testAddForeignKeyIndexImplicitly()
$this->assertEquals(array('id'), $index->getColumns());
}

/**
* @group DBAL-1063
*/
public function testAddForeignKeyDoesNotCreateDuplicateIndex()
{
$table = new Table('foo');
$table->addColumn('bar', 'integer');
$table->addIndex(array('bar'), 'bar_idx');

$foreignTable = new Table('bar');
$foreignTable->addColumn('foo', 'integer');

$table->addForeignKeyConstraint($foreignTable, array('bar'), array('foo'));

$this->assertCount(1, $table->getIndexes());
$this->assertTrue($table->hasIndex('bar_idx'));
$this->assertSame(array('bar'), $table->getIndex('bar_idx')->getColumns());
}

/**
* @group DBAL-1063
*/
public function testAddForeignKeyAddsImplicitIndexIfIndexColumnsDoNotSpan()
{
$table = new Table('foo');
$table->addColumn('bar', 'integer');
$table->addColumn('baz', 'string');
$table->addColumn('bloo', 'string');
$table->addIndex(array('baz', 'bar'), 'composite_idx');
$table->addIndex(array('bar', 'baz', 'bloo'), 'full_idx');

$foreignTable = new Table('bar');
$foreignTable->addColumn('foo', 'integer');
$foreignTable->addColumn('baz', 'string');

$table->addForeignKeyConstraint($foreignTable, array('bar', 'baz'), array('foo', 'baz'));

$this->assertCount(3, $table->getIndexes());
$this->assertTrue($table->hasIndex('composite_idx'));
$this->assertTrue($table->hasIndex('full_idx'));
$this->assertTrue($table->hasIndex('idx_8c73652176ff8caa78240498'));
$this->assertSame(array('baz', 'bar'), $table->getIndex('composite_idx')->getColumns());
$this->assertSame(array('bar', 'baz', 'bloo'), $table->getIndex('full_idx')->getColumns());
$this->assertSame(array('bar', 'baz'), $table->getIndex('idx_8c73652176ff8caa78240498')->getColumns());
}

/**
* @group DBAL-50
* @group DBAL-1063
*/
public function testOverruleIndex()
public function testOverrulingIndexDoesNotDropOverruledIndex()
{
$table = new Table("bar");
$table->addColumn('baz', 'integer', array());
Expand All @@ -399,23 +433,62 @@ public function testOverruleIndex()
$index = current($indexes);

$table->addUniqueIndex(array('baz'));
$this->assertEquals(1, count($table->getIndexes()));
$this->assertFalse($table->hasIndex($index->getName()));
$this->assertEquals(2, count($table->getIndexes()));
$this->assertTrue($table->hasIndex($index->getName()));
}

/**
* @group DBAL-1063
*/
public function testAllowsAddingDuplicateIndexesBasedOnColumns()
{
$table = new Table('foo');
$table->addColumn('bar', 'integer');
$table->addIndex(array('bar'), 'bar_idx');
$table->addIndex(array('bar'), 'duplicate_idx');

$this->assertCount(2, $table->getIndexes());
$this->assertTrue($table->hasIndex('bar_idx'));
$this->assertTrue($table->hasIndex('duplicate_idx'));
$this->assertSame(array('bar'), $table->getIndex('bar_idx')->getColumns());
$this->assertSame(array('bar'), $table->getIndex('duplicate_idx')->getColumns());
}

public function testPrimaryKeyOverrulesUniqueIndex()
/**
* @group DBAL-1063
*/
public function testAllowsAddingFulfillingIndexesBasedOnColumns()
{
$table = new Table('foo');
$table->addColumn('bar', 'integer');
$table->addColumn('baz', 'string');
$table->addIndex(array('bar'), 'bar_idx');
$table->addIndex(array('bar', 'baz'), 'fulfilling_idx');

$this->assertCount(2, $table->getIndexes());
$this->assertTrue($table->hasIndex('bar_idx'));
$this->assertTrue($table->hasIndex('fulfilling_idx'));
$this->assertSame(array('bar'), $table->getIndex('bar_idx')->getColumns());
$this->assertSame(array('bar', 'baz'), $table->getIndex('fulfilling_idx')->getColumns());
}

/**
* @group DBAL-50
* @group DBAL-1063
*/
public function testPrimaryKeyOverrulingUniqueIndexDoesNotDropUniqueIndex()
{
$table = new Table("bar");
$table->addColumn('baz', 'integer', array());
$table->addUniqueIndex(array('baz'));
$table->addUniqueIndex(array('baz'), 'idx_unique');

$table->setPrimaryKey(array('baz'));

$indexes = $table->getIndexes();
$this->assertEquals(1, count($indexes), "Table should only contain the primary key table index, not the unique one anymore, because it was overruled.");
$this->assertEquals(2, count($indexes), "Table should only contain both the primary key table index and the unique one, even though it was overruled.");

$index = current($indexes);
$this->assertTrue($index->isPrimary());
$this->assertTrue($table->hasPrimaryKey());
$this->assertTrue($table->hasIndex('idx_unique'));
}

/**
Expand Down

0 comments on commit 62054e2

Please sign in to comment.