Browse files

Merge pull request #456 from deeky666/DBAL-44

[DBAL-44] Fix column default value lifecycle
  • Loading branch information...
2 parents e8efcd7 + 17edd0d commit 8fe741053849afadef12b8bef1cc3203966ef78f @beberlei beberlei committed Dec 22, 2013
View
5 lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php
@@ -470,7 +470,10 @@ public function getAlterTableSQL(TableDiff $diff)
}
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;
}
View
10 lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php
@@ -22,7 +22,6 @@
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\LockMode;
-use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\ColumnDiff;
use Doctrine\DBAL\Schema\Constraint;
@@ -287,7 +286,14 @@ protected function getAlterTableChangeColumnClause(ColumnDiff $columnDiff)
// Do not return alter clause if only comment has changed.
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;
}
}
View
14 lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
@@ -246,7 +246,7 @@ protected function _getCreateTableSQL($tableName, array $columns, array $options
}
// Build default constraints SQL statements.
- if ( ! empty($column['default']) || is_numeric($column['default'])) {
+ if (isset($column['default'])) {
$defaultConstraintsSql[] = 'ALTER TABLE ' . $tableName .
' ADD' . $this->getDefaultConstraintDeclarationSQL($tableName, $column);
}
@@ -352,7 +352,7 @@ protected function getCreateColumnCommentSQL($tableName, $columnName, $comment)
*/
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.");
}
@@ -446,7 +446,7 @@ public function getAlterTableSQL(TableDiff $diff)
$columnDef = $column->toArray();
$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);
$queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
}
@@ -517,15 +517,15 @@ public function getAlterTableSQL(TableDiff $diff)
* if default value has changed and another
* default constraint already exists for the column.
*/
- if ($columnDefaultHasChanged && ( ! empty($fromColumnDefault) || is_numeric($fromColumnDefault))) {
+ if ($columnDefaultHasChanged && null !== $fromColumnDefault) {
$queryParts[] = 'DROP CONSTRAINT ' .
$this->generateDefaultConstraintName($diff->name, $columnDiff->oldColumnName);
}
$queryParts[] = 'ALTER COLUMN ' .
$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);
$queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
}
@@ -546,7 +546,7 @@ public function getAlterTableSQL(TableDiff $diff)
* Drop existing default constraint for the old column name
* if column has default value.
*/
- if ( ! empty($columnDef['default']) || is_numeric($columnDef['default'])) {
+ if (isset($columnDef['default'])) {
$queryParts[] = 'DROP CONSTRAINT ' .
$this->generateDefaultConstraintName($diff->name, $oldColumnName);
}
@@ -557,7 +557,7 @@ public function getAlterTableSQL(TableDiff $diff)
/**
* 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);
$queryParts[] = 'ADD' . $this->getDefaultConstraintDeclarationSQL($diff->name, $columnDef);
}
View
10 lib/Doctrine/DBAL/Schema/Comparator.php
@@ -369,7 +369,15 @@ public function diffColumn(Column $column1, Column $column2)
$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';
}
View
2 lib/Doctrine/DBAL/Schema/DrizzleSchemaManager.php
@@ -41,7 +41,7 @@ protected function _getPortableTableColumnDefinition($tableColumn)
$options = array(
'notnull' => !(bool)$tableColumn['IS_NULLABLE'],
'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'],
'scale' => (int)$tableColumn['NUMERIC_SCALE'],
'precision' => (int)$tableColumn['NUMERIC_PRECISION'],
View
4 lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php
@@ -112,6 +112,10 @@ protected function _getPortableTableColumnDefinition($tableColumn)
if ($tableColumn['default']) {
// Strip quotes from default value.
$default = preg_replace(array("/^'(.*)'$/", "/''/"), array("$1", "'"), $tableColumn['default']);
+
+ if ('autoincrement' == $default) {
+ $default = null;
+ }
}
switch ($tableColumn['type']) {
View
52 tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php
@@ -2,6 +2,8 @@
namespace Doctrine\Tests\DBAL\Functional\Schema;
+use Doctrine\DBAL\Schema\Comparator;
+use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\Type,
Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Platforms\AbstractPlatform;
@@ -670,4 +672,54 @@ public function testListForeignKeysComposite()
$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()));
}
+
+ /**
+ * @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());
+ }
}
View
4 tests/Doctrine/Tests/DBAL/Platforms/PostgreSqlPlatformTest.php
@@ -33,10 +33,10 @@ public function getGenerateAlterTableSql()
'ALTER TABLE mytable ADD quota INT DEFAULT NULL',
'ALTER TABLE mytable DROP foo',
'ALTER TABLE mytable ALTER bar TYPE VARCHAR(255)',
- "ALTER TABLE mytable ALTER bar SET DEFAULT 'def'",
+ "ALTER TABLE mytable ALTER bar SET DEFAULT 'def'",
'ALTER TABLE mytable ALTER bar SET NOT NULL',
'ALTER TABLE mytable ALTER bloo TYPE BOOLEAN',
- "ALTER TABLE mytable ALTER bloo SET DEFAULT 'false'",
+ "ALTER TABLE mytable ALTER bloo SET DEFAULT 'false'",
'ALTER TABLE mytable ALTER bloo SET NOT NULL',
'ALTER TABLE mytable RENAME TO userlist',
);
View
1 tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php
@@ -37,6 +37,7 @@ public function getGenerateAlterTableSql()
'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 ALTER COLUMN bloo BIT NOT NULL',
+ "ALTER TABLE mytable ADD CONSTRAINT DF_6B2BD609_CECED971 DEFAULT '0' FOR bloo",
"sp_RENAME 'mytable', 'userlist'",
"DECLARE @sql NVARCHAR(MAX) = N''; " .
"SELECT @sql += N'EXEC sp_rename N''' + dc.name + ''', N''' " .

0 comments on commit 8fe7410

Please sign in to comment.