Commit 17edd0d5 authored by Steve Müller's avatar Steve Müller

fix column default value lifecycle

parent ba4ab893
...@@ -470,7 +470,10 @@ class PostgreSqlPlatform extends AbstractPlatform ...@@ -470,7 +470,10 @@ class PostgreSqlPlatform extends AbstractPlatform
} }
if ($columnDiff->hasChanged('default')) { if ($columnDiff->hasChanged('default')) {
$query = 'ALTER ' . $oldColumnName . ' SET ' . $this->getDefaultValueDeclarationSQL($column->toArray()); $defaultClause = null === $column->getDefault()
? ' DROP DEFAULT'
: ' SET' . $this->getDefaultValueDeclarationSQL($column->toArray());
$query = 'ALTER ' . $oldColumnName . $defaultClause;
$sql[] = 'ALTER TABLE ' . $diff->getName()->getQuotedName($this) . ' ' . $query; $sql[] = 'ALTER TABLE ' . $diff->getName()->getQuotedName($this) . ' ' . $query;
} }
......
...@@ -22,7 +22,6 @@ namespace Doctrine\DBAL\Platforms; ...@@ -22,7 +22,6 @@ namespace Doctrine\DBAL\Platforms;
use Doctrine\DBAL\Connection; use Doctrine\DBAL\Connection;
use Doctrine\DBAL\DBALException; use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\LockMode; use Doctrine\DBAL\LockMode;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\Column; use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\ColumnDiff; use Doctrine\DBAL\Schema\ColumnDiff;
use Doctrine\DBAL\Schema\Constraint; use Doctrine\DBAL\Schema\Constraint;
...@@ -287,7 +286,14 @@ class SQLAnywherePlatform extends AbstractPlatform ...@@ -287,7 +286,14 @@ class SQLAnywherePlatform extends AbstractPlatform
// Do not return alter clause if only comment has changed. // Do not return alter clause if only comment has changed.
if ( ! ($columnDiff->hasChanged('comment') && count($columnDiff->changedProperties) === 1)) { if ( ! ($columnDiff->hasChanged('comment') && count($columnDiff->changedProperties) === 1)) {
return 'ALTER ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray()); $columnAlterationClause = 'ALTER ' .
$this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
if ($columnDiff->hasChanged('default') && null === $column->getDefault()) {
$columnAlterationClause .= ', ALTER ' . $column->getQuotedName($this) . ' DROP DEFAULT';
}
return $columnAlterationClause;
} }
} }
......
...@@ -246,7 +246,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -246,7 +246,7 @@ class SQLServerPlatform extends AbstractPlatform
} }
// Build default constraints SQL statements. // Build default constraints SQL statements.
if ( ! empty($column['default']) || is_numeric($column['default'])) { if (isset($column['default'])) {
$defaultConstraintsSql[] = 'ALTER TABLE ' . $tableName . $defaultConstraintsSql[] = 'ALTER TABLE ' . $tableName .
' ADD' . $this->getDefaultConstraintDeclarationSQL($tableName, $column); ' ADD' . $this->getDefaultConstraintDeclarationSQL($tableName, $column);
} }
...@@ -352,7 +352,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -352,7 +352,7 @@ class SQLServerPlatform extends AbstractPlatform
*/ */
public function getDefaultConstraintDeclarationSQL($table, array $column) public function getDefaultConstraintDeclarationSQL($table, array $column)
{ {
if (empty($column['default']) && ! is_numeric($column['default'])) { if ( ! isset($column['default'])) {
throw new \InvalidArgumentException("Incomplete column definition. 'default' required."); throw new \InvalidArgumentException("Incomplete column definition. 'default' required.");
} }
...@@ -446,7 +446,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -446,7 +446,7 @@ class SQLServerPlatform extends AbstractPlatform
$columnDef = $column->toArray(); $columnDef = $column->toArray();
$queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef); $queryParts[] = 'ADD ' . $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef);
if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) { if (isset($columnDef['default'])) {
$columnDef['name'] = $column->getQuotedName($this); $columnDef['name'] = $column->getQuotedName($this);
$queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef); $queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
} }
...@@ -517,7 +517,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -517,7 +517,7 @@ class SQLServerPlatform extends AbstractPlatform
* if default value has changed and another * if default value has changed and another
* default constraint already exists for the column. * default constraint already exists for the column.
*/ */
if ($columnDefaultHasChanged && ( ! empty($fromColumnDefault) || is_numeric($fromColumnDefault))) { if ($columnDefaultHasChanged && null !== $fromColumnDefault) {
$queryParts[] = 'DROP CONSTRAINT ' . $queryParts[] = 'DROP CONSTRAINT ' .
$this->generateDefaultConstraintName($diff->name, $columnDiff->oldColumnName); $this->generateDefaultConstraintName($diff->name, $columnDiff->oldColumnName);
} }
...@@ -525,7 +525,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -525,7 +525,7 @@ class SQLServerPlatform extends AbstractPlatform
$queryParts[] = 'ALTER COLUMN ' . $queryParts[] = 'ALTER COLUMN ' .
$this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef); $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnDef);
if ($columnDefaultHasChanged && (! empty($columnDef['default']) || is_numeric($columnDef['default']))) { if ($columnDefaultHasChanged && isset($columnDef['default'])) {
$columnDef['name'] = $column->getQuotedName($this); $columnDef['name'] = $column->getQuotedName($this);
$queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef); $queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
} }
...@@ -546,7 +546,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -546,7 +546,7 @@ class SQLServerPlatform extends AbstractPlatform
* Drop existing default constraint for the old column name * Drop existing default constraint for the old column name
* if column has default value. * if column has default value.
*/ */
if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) { if (isset($columnDef['default'])) {
$queryParts[] = 'DROP CONSTRAINT ' . $queryParts[] = 'DROP CONSTRAINT ' .
$this->generateDefaultConstraintName($diff->name, $oldColumnName); $this->generateDefaultConstraintName($diff->name, $oldColumnName);
} }
...@@ -557,7 +557,7 @@ class SQLServerPlatform extends AbstractPlatform ...@@ -557,7 +557,7 @@ class SQLServerPlatform extends AbstractPlatform
/** /**
* Readd default constraint for the new column name. * Readd default constraint for the new column name.
*/ */
if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) { if (isset($columnDef['default'])) {
$columnDef['name'] = $column->getQuotedName($this); $columnDef['name'] = $column->getQuotedName($this);
$queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef); $queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
} }
......
...@@ -369,7 +369,15 @@ class Comparator ...@@ -369,7 +369,15 @@ class Comparator
$changedProperties[] = 'notnull'; $changedProperties[] = 'notnull';
} }
if ($column1->getDefault() != $column2->getDefault()) { $column1Default = $column1->getDefault();
$column2Default = $column2->getDefault();
if ($column1Default != $column2Default ||
// Null values need to be checked additionally as they tell whether to create or drop a default value.
// null != 0, null != false, null != '' etc. This affects platform's table alteration SQL generation.
(null === $column1Default && null !== $column2Default) ||
(null === $column2Default && null !== $column1Default)
) {
$changedProperties[] = 'default'; $changedProperties[] = 'default';
} }
......
...@@ -41,7 +41,7 @@ class DrizzleSchemaManager extends AbstractSchemaManager ...@@ -41,7 +41,7 @@ class DrizzleSchemaManager extends AbstractSchemaManager
$options = array( $options = array(
'notnull' => !(bool)$tableColumn['IS_NULLABLE'], 'notnull' => !(bool)$tableColumn['IS_NULLABLE'],
'length' => (int)$tableColumn['CHARACTER_MAXIMUM_LENGTH'], 'length' => (int)$tableColumn['CHARACTER_MAXIMUM_LENGTH'],
'default' => empty($tableColumn['COLUMN_DEFAULT']) ? null : $tableColumn['COLUMN_DEFAULT'], 'default' => isset($tableColumn['COLUMN_DEFAULT']) ? $tableColumn['COLUMN_DEFAULT'] : null,
'autoincrement' => (bool)$tableColumn['IS_AUTO_INCREMENT'], 'autoincrement' => (bool)$tableColumn['IS_AUTO_INCREMENT'],
'scale' => (int)$tableColumn['NUMERIC_SCALE'], 'scale' => (int)$tableColumn['NUMERIC_SCALE'],
'precision' => (int)$tableColumn['NUMERIC_PRECISION'], 'precision' => (int)$tableColumn['NUMERIC_PRECISION'],
......
...@@ -112,6 +112,10 @@ class SQLAnywhereSchemaManager extends AbstractSchemaManager ...@@ -112,6 +112,10 @@ class SQLAnywhereSchemaManager extends AbstractSchemaManager
if ($tableColumn['default']) { if ($tableColumn['default']) {
// Strip quotes from default value. // Strip quotes from default value.
$default = preg_replace(array("/^'(.*)'$/", "/''/"), array("$1", "'"), $tableColumn['default']); $default = preg_replace(array("/^'(.*)'$/", "/''/"), array("$1", "'"), $tableColumn['default']);
if ('autoincrement' == $default) {
$default = null;
}
} }
switch ($tableColumn['type']) { switch ($tableColumn['type']) {
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
namespace Doctrine\Tests\DBAL\Functional\Schema; namespace Doctrine\Tests\DBAL\Functional\Schema;
use Doctrine\DBAL\Schema\Comparator;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\Type, use Doctrine\DBAL\Types\Type,
Doctrine\DBAL\Schema\AbstractSchemaManager; Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\AbstractPlatform;
...@@ -670,4 +672,54 @@ class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTest ...@@ -670,4 +672,54 @@ class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTest
$this->assertEquals(array('id', 'foreign_key_test'), array_map('strtolower', $fkeys[0]->getLocalColumns())); $this->assertEquals(array('id', 'foreign_key_test'), array_map('strtolower', $fkeys[0]->getLocalColumns()));
$this->assertEquals(array('id', 'other_id'), array_map('strtolower', $fkeys[0]->getForeignColumns())); $this->assertEquals(array('id', 'other_id'), array_map('strtolower', $fkeys[0]->getForeignColumns()));
} }
/**
* @group DBAL-44
*/
public function testColumnDefaultLifecycle()
{
$table = new Table("col_def_lifecycle");
$table->addColumn('id', 'integer', array('primary' => true, 'autoincrement' => true));
$table->addColumn('column1', 'string', array('default' => null));
$table->addColumn('column2', 'string', array('default' => false));
$table->addColumn('column3', 'string', array('default' => true));
$table->addColumn('column4', 'string', array('default' => 0));
$table->addColumn('column5', 'string', array('default' => ''));
$table->addColumn('column6', 'string', array('default' => 'def'));
$table->setPrimaryKey(array('id'));
$this->_sm->dropAndCreateTable($table);
$columns = $this->_sm->listTableColumns('col_def_lifecycle');
$this->assertNull($columns['id']->getDefault());
$this->assertNull($columns['column1']->getDefault());
$this->assertSame('', $columns['column2']->getDefault());
$this->assertSame('1', $columns['column3']->getDefault());
$this->assertSame('0', $columns['column4']->getDefault());
$this->assertSame('', $columns['column5']->getDefault());
$this->assertSame('def', $columns['column6']->getDefault());
$diffTable = clone $table;
$diffTable->changeColumn('column1', array('default' => false));
$diffTable->changeColumn('column2', array('default' => null));
$diffTable->changeColumn('column3', array('default' => false));
$diffTable->changeColumn('column4', array('default' => null));
$diffTable->changeColumn('column5', array('default' => false));
$diffTable->changeColumn('column6', array('default' => 666));
$comparator = new Comparator();
$this->_sm->alterTable($comparator->diffTable($table, $diffTable));
$columns = $this->_sm->listTableColumns('col_def_lifecycle');
$this->assertSame('', $columns['column1']->getDefault());
$this->assertNull($columns['column2']->getDefault());
$this->assertSame('', $columns['column3']->getDefault());
$this->assertNull($columns['column4']->getDefault());
$this->assertSame('', $columns['column5']->getDefault());
$this->assertSame('666', $columns['column6']->getDefault());
}
} }
...@@ -37,6 +37,7 @@ class SQLServerPlatformTest extends AbstractPlatformTestCase ...@@ -37,6 +37,7 @@ class SQLServerPlatformTest extends AbstractPlatformTestCase
'ALTER TABLE mytable ALTER COLUMN baz NVARCHAR(255) NOT NULL', 'ALTER TABLE mytable ALTER COLUMN baz NVARCHAR(255) NOT NULL',
"ALTER TABLE mytable ADD CONSTRAINT DF_6B2BD609_78240498 DEFAULT 'def' FOR baz", "ALTER TABLE mytable ADD CONSTRAINT DF_6B2BD609_78240498 DEFAULT 'def' FOR baz",
'ALTER TABLE mytable ALTER COLUMN bloo BIT NOT NULL', 'ALTER TABLE mytable ALTER COLUMN bloo BIT NOT NULL',
"ALTER TABLE mytable ADD CONSTRAINT DF_6B2BD609_CECED971 DEFAULT '0' FOR bloo",
"sp_RENAME 'mytable', 'userlist'", "sp_RENAME 'mytable', 'userlist'",
"DECLARE @sql NVARCHAR(MAX) = N''; " . "DECLARE @sql NVARCHAR(MAX) = N''; " .
"SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N''' " . "SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N''' " .
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment