Skip to content

Commit

Permalink
Update MySQL table generation for non-conventional schemas.
Browse files Browse the repository at this point in the history
When generating schema from a Table instance, we were incorrectly
assigning autoIncrement to integer columns just because they were
involved in a primary key. In the new implementation, an autoIncrement
is only assigned when:

* The primary key has only one column
* That column is integer/bigint.
* There are no other fields flagged as autoIncrement.

This allows MySQL schema generation to handle autoincrements in unique
keys correctly.

Refs #6668
  • Loading branch information
markstory committed Jun 10, 2015
1 parent e6dae1d commit b059505
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 4 deletions.
4 changes: 1 addition & 3 deletions src/Database/Schema/MysqlSchema.php
Expand Up @@ -332,9 +332,7 @@ public function columnSql(Table $table, $name)
if (isset($data['null']) && $data['null'] === false) {
$out .= ' NOT NULL';
}
if (in_array($data['type'], ['integer', 'biginteger']) &&
([$name] == (array)$table->primaryKey() || $data['autoIncrement'] === true)
) {
if (in_array($data['type'], ['integer', 'biginteger']) && $data['autoIncrement'] === true) {
$out .= ' AUTO_INCREMENT';
}
if (isset($data['null']) && $data['null'] === true) {
Expand Down
24 changes: 24 additions & 0 deletions src/Database/Schema/Table.php
Expand Up @@ -508,6 +508,15 @@ public function addConstraint($name, $attrs)
throw new Exception($msg);
}
}

if ($attrs['type'] === static::CONSTRAINT_PRIMARY && count($attrs['columns']) === 1) {
$column = $attrs['columns'][0];
if (!$this->_hasAutoincrement() &&
in_array($this->columnType($column), ['integer', 'biginteger'])
) {
$this->_columns[$attrs['columns'][0]]['autoIncrement'] = true;
}
}
if ($attrs['type'] === static::CONSTRAINT_FOREIGN) {
$attrs = $this->_checkForeignKey($attrs);
} else {
Expand All @@ -517,6 +526,21 @@ public function addConstraint($name, $attrs)
return $this;
}

/**
* Check whether or not a table has an autoIncrement column defined.
*
* @return bool
*/
protected function _hasAutoincrement()
{
foreach ($this->_columns as $column) {
if (isset($column['autoIncrement']) && $column['autoIncrement']) {
return true;
}
}
return false;
}

/**
* Helper method to check/validate foreign keys.
*
Expand Down
32 changes: 31 additions & 1 deletion tests/TestCase/Database/Schema/MysqlSchemaTest.php
Expand Up @@ -390,6 +390,36 @@ public function testDescribeTableOptions()
$this->assertArrayHasKey('collation', $result->options());
}

public function testDescribeNonPrimaryAutoIncrement()
{
$this->_needsConnection();
$connection = ConnectionManager::get('test');

$sql = <<<SQL
CREATE TEMPORARY TABLE `odd_primary_key` (
`id` BIGINT UNSIGNED NOT NULL,
`other_field` INTEGER(11) NOT NULL AUTO_INCREMENT,
PRIMARY KEY (`id`),
UNIQUE KEY `other_field` (`other_field`)
)
SQL;
$connection->execute($sql);
$schema = new SchemaCollection($connection);
$table = $schema->describe('odd_primary_key');

$column = $table->column('id');
$this->assertNull($column['autoIncrement'], 'should not autoincrement');
$this->assertTrue($column['unsigned'], 'should be unsigned');

$column = $table->column('other_field');
$this->assertTrue($column['autoIncrement'], 'should not autoincrement');
$this->assertFalse($column['unsigned'], 'should not be unsigned');

$output = $table->createSql($connection);
$this->assertContains('`id` BIGINT UNSIGNED NOT NULL,', $output[0]);
$this->assertContains('`other_field` INTEGER(11) NOT NULL AUTO_INCREMENT,', $output[0]);
}

/**
* Column provider for creating column sql
*
Expand Down Expand Up @@ -689,7 +719,7 @@ public function testColumnSqlPrimaryKey()
$table = new Table('articles');
$table->addColumn('id', [
'type' => 'integer',
'null' => false
'null' => false,
])
->addConstraint('primary', [
'type' => 'primary',
Expand Down

0 comments on commit b059505

Please sign in to comment.