Skip to content

Commit

Permalink
Fixed key/index operations for MySQL
Browse files Browse the repository at this point in the history 
The methods hasIndex, dropIndex and hasForeignKey fail to identify
index/keys correctly when using column names.
  • Loading branch information
Bernat Arlandis committed Nov 4, 2015
1 parent 9ec3510 commit d87b111
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 18 deletions.
8 changes: 3 additions & 5 deletions src/Phinx/Db/Adapter/MysqlAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,7 @@ public function hasIndex($tableName, $columns)
$indexes = $this->getIndexes($tableName);

foreach ($indexes as $index) {
$a = array_diff($columns, $index['columns']);
if (empty($a)) {
if ($columns == $index['columns']) {
return true;
}
}
Expand Down Expand Up @@ -553,8 +552,7 @@ public function dropIndex($tableName, $columns)
$columns = array_map('strtolower', $columns);

foreach ($indexes as $indexName => $index) {
$a = array_diff($columns, $index['columns']);
if (empty($a)) {
if ($columns == $index['columns']) {
$this->execute(
sprintf(
'ALTER TABLE %s DROP INDEX %s',
Expand Down Expand Up @@ -611,7 +609,7 @@ public function hasForeignKey($tableName, $columns, $constraint = null)
} else {
foreach ($foreignKeys as $key) {
$a = array_diff($columns, $key['columns']);
if (empty($a)) {
if ($columns == $key['columns']) {
return true;
}
}
Expand Down
24 changes: 23 additions & 1 deletion tests/Phinx/Db/Adapter/MysqlAdapterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ public function testCreateTableWithMultiplePrimaryKeys()
->addColumn('tag_id', 'integer')
->save();
$this->assertTrue($this->adapter->hasIndex('table1', array('user_id', 'tag_id')));
$this->assertTrue($this->adapter->hasIndex('table1', array('tag_id', 'USER_ID')));
$this->assertTrue($this->adapter->hasIndex('table1', array('USER_ID', 'tag_id')));
$this->assertFalse($this->adapter->hasIndex('table1', array('tag_id', 'user_id')));
$this->assertFalse($this->adapter->hasIndex('table1', array('tag_id', 'user_email')));
}

Expand Down Expand Up @@ -697,6 +698,27 @@ public function testDropIndex()
$this->assertTrue($table4->hasIndex(array('fname', 'lname')));
$this->adapter->dropIndex($table4->getName(), array('fname', 'lname'));
$this->assertFalse($table4->hasIndex(array('fname', 'lname')));

// don't drop multiple column index when dropping single column
$table2 = new \Phinx\Db\Table('table5', array(), $this->adapter);
$table2->addColumn('fname', 'string')
->addColumn('lname', 'string')
->addIndex(array('fname', 'lname'))
->save();
$this->assertTrue($table2->hasIndex(array('fname', 'lname')));
$this->adapter->dropIndex($table2->getName(), array('fname'));
$this->assertTrue($table2->hasIndex(array('fname', 'lname')));

// don't drop multiple column index with name specified when dropping
// single column
$table4 = new \Phinx\Db\Table('table6', array(), $this->adapter);
$table4->addColumn('fname', 'string')
->addColumn('lname', 'string')
->addIndex(array('fname', 'lname'), array('name' => 'multiname'))
->save();
$this->assertTrue($table4->hasIndex(array('fname', 'lname')));
$this->adapter->dropIndex($table4->getName(), array('fname'));
$this->assertTrue($table4->hasIndex(array('fname', 'lname')));
}

public function testDropIndexByName()
Expand Down
82 changes: 70 additions & 12 deletions tests/Phinx/Db/Adapter/MysqlAdapterUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,34 @@ private function prepareCaseIndexes()
'Comment' => '',
'Index_comment' => '');

$index3 = array('Table' => 'table_name',
'Non_unique' => '0',
'Key_name' => 'multiple_index_name',
'Seq_in_index' => '1',
'Column_name' => 'column_name',
'Collation' => 'A',
'Cardinality' => '0',
'Sub_part' => 'NULL',
'Packed' => 'NULL',
'Null' => '',
'Index_type' => 'BTREE',
'Comment' => '',
'Index_comment' => '');

$index4 = array('Table' => 'table_name',
'Non_unique' => '0',
'Key_name' => 'multiple_index_name',
'Seq_in_index' => '2',
'Column_name' => 'another_column_name',
'Collation' => 'A',
'Cardinality' => '0',
'Sub_part' => 'NULL',
'Packed' => 'NULL',
'Null' => '',
'Index_type' => 'BTREE',
'Comment' => '',
'Index_comment' => '');

$this->result->expects($this->at(0))
->method('fetch')
->will($this->returnValue($index1));
Expand All @@ -1090,22 +1118,31 @@ private function prepareCaseIndexes()
->will($this->returnValue($index2));

$this->result->expects($this->at(2))
->method('fetch')
->will($this->returnValue($index3));

$this->result->expects($this->at(3))
->method('fetch')
->will($this->returnValue($index4));

$this->result->expects($this->at(4))
->method('fetch')
->will($this->returnValue(null));

$this->assertQuerySql("SHOW INDEXES FROM `table_name`", $this->result);
return array($index1, $index2);
return array($index1, $index2, $index3, $index4);
}

public function testGetIndexes()
{
list($index1, $index2) = $this->prepareCaseIndexes();
list($index1, $index2, $index3, $index4) = $this->prepareCaseIndexes();
$indexes = $this->adapter->getIndexes("table_name");

$this->assertTrue(is_array($indexes));
$this->assertEquals(2, count($indexes));
$this->assertEquals(3, count($indexes));
$this->assertEquals(array('columns' => array($index1['Column_name'])), $indexes[$index1['Key_name']]);
$this->assertEquals(array('columns' => array($index2['Column_name'])), $indexes[$index2['Key_name']]);
$this->assertEquals(array('columns' => array($index3['Column_name'], $index4['Column_name'])), $indexes[$index3['Key_name']]);
}


Expand All @@ -1118,7 +1155,7 @@ public function testHasIndexExistsAsString()
public function testHasIndexNotExistsAsString()
{
$this->prepareCaseIndexes();
$this->assertFalse($this->adapter->hasIndex("table_name", "column_name_not_exists"));
$this->assertFalse($this->adapter->hasIndex("table_name", "another_column_name"));
}

public function testHasIndexExistsAsArray()
Expand All @@ -1130,7 +1167,7 @@ public function testHasIndexExistsAsArray()
public function testHasIndexNotExistsAsArray()
{
$this->prepareCaseIndexes();
$this->assertFalse($this->adapter->hasIndex("table_name", array("column_name_not_exists")));
$this->assertFalse($this->adapter->hasIndex("table_name", array("another_column_name")));
}

public function testAddIndex()
Expand Down Expand Up @@ -1184,11 +1221,32 @@ private function prepareCaseForeignKeys()
'REFERENCED_TABLE_NAME' => 'other_table',
'REFERENCED_COLUMN_NAME' => 'id');


$fk1 = array('CONSTRAINT_NAME' => 'fk2',
'TABLE_NAME' => 'table_name',
'COLUMN_NAME' => 'other_table_id',
'REFERENCED_TABLE_NAME' => 'other_table',
'REFERENCED_COLUMN_NAME' => 'id');

$fk2 = array('CONSTRAINT_NAME' => 'fk2',
'TABLE_NAME' => 'table_name',
'COLUMN_NAME' => 'another_table_id',
'REFERENCED_TABLE_NAME' => 'other_table',
'REFERENCED_COLUMN_NAME' => 'id');

$this->result->expects($this->at(0))
->method('fetch')
->will($this->returnValue($fk));

$this->result->expects($this->at(1))
->method('fetch')
->will($this->returnValue($fk1));

$this->result->expects($this->at(2))
->method('fetch')
->will($this->returnValue($fk2));

$this->result->expects($this->at(3))
->method('fetch')
->will($this->returnValue(null));

Expand All @@ -1204,16 +1262,16 @@ private function prepareCaseForeignKeys()
AND TABLE_NAME = \'table_name\'
ORDER BY POSITION_IN_UNIQUE_CONSTRAINT';
$this->assertQuerySql($expectedSql, $this->result);
return array($fk);
return array($fk, $fk1, $fk2);
}

public function testGetForeignKeys()
{
list($fk) = $this->prepareCaseForeignKeys();
list($fk, $fk1, $fk2) = $this->prepareCaseForeignKeys();
$foreignkeys = $this->adapter->getForeignKeys("table_name");

$this->assertTrue(is_array($foreignkeys));
$this->assertEquals(1, count($foreignkeys));
$this->assertEquals(2, count($foreignkeys));
$this->assertEquals('table_name', $foreignkeys['fk1']['table']);
$this->assertEquals(array('other_table_id'), $foreignkeys['fk1']['columns']);
$this->assertEquals('other_table', $foreignkeys['fk1']['referenced_table']);
Expand All @@ -1235,13 +1293,13 @@ public function testHasForeignKeyExistsAsStringAndConstraint()
public function testHasForeignKeyNotExistsAsString()
{
$this->prepareCaseForeignKeys();
$this->assertFalse($this->adapter->hasForeignKey("table_name", "not_table_id"));
$this->assertFalse($this->adapter->hasForeignKey("table_name", "another_table_id"));
}

public function testHasForeignKeyNotExistsAsStringAndConstraint()
{
$this->prepareCaseForeignKeys();
$this->assertFalse($this->adapter->hasForeignKey("table_name", "not_table_id"), 'fk2');
$this->assertFalse($this->adapter->hasForeignKey("table_name", "other_table_id", 'fk3'));

This comment has been minimized.

Sign in to view

Copy link
@berarma

berarma Nov 4, 2015 •
edited
Loading

Contributor

Notice the change in the parentheses. This is a bug in the test code that should be addressed anyway.

This comment has been minimized.

Sign in to view

Copy link
@robmorgan

robmorgan Nov 4, 2015

Member

thx for spotting
}

public function testHasForeignKeyExistsAsArray()
Expand All @@ -1259,13 +1317,13 @@ public function testHasForeignKeyExistsAsArrayAndConstraint()
public function testHasForeignKeyNotExistsAsArray()
{
$this->prepareCaseForeignKeys();
$this->assertFalse($this->adapter->hasForeignKey("table_name", array("not_table_id")));
$this->assertFalse($this->adapter->hasForeignKey("table_name", array("another_table_id")));
}

public function testHasForeignKeyNotExistsAsArrayAndConstraint()
{
$this->prepareCaseForeignKeys();
$this->assertFalse($this->adapter->hasForeignKey("table_name", array("not_table_id"), 'fk2'));
$this->assertFalse($this->adapter->hasForeignKey("table_name", array("other_table_id"), 'fk3'));
}

public function testAddForeignKeyBasic()
Expand Down

0 comments on commit d87b111

Please sign in to comment.