Merge pull request #456 from deeky666/DBAL-44

[DBAL-44] Fix column default value lifecycle

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

@@ -470,7 +470,10 @@ class PostgreSqlPlatform extends AbstractPlatform
             }
 
             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;
             }
 

lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php

@@ -22,7 +22,6 @@ namespace Doctrine\DBAL\Platforms;
 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 @@ class SQLAnywherePlatform extends AbstractPlatform
 
         // 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;
         }
     }
 

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

@@ -246,7 +246,7 @@ class SQLServerPlatform extends AbstractPlatform
             }
 
             // 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 @@ class SQLServerPlatform extends AbstractPlatform
      */
     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 @@ class SQLServerPlatform extends AbstractPlatform
             $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,7 +517,7 @@ class SQLServerPlatform extends AbstractPlatform
              * 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);
             }
@@ -525,7 +525,7 @@ class SQLServerPlatform extends AbstractPlatform
             $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 @@ class SQLServerPlatform extends AbstractPlatform
              * 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 @@ class SQLServerPlatform extends AbstractPlatform
             /**
              * 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);
             }

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -369,7 +369,15 @@ class Comparator
             $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';
         }
 

lib/Doctrine/DBAL/Schema/DrizzleSchemaManager.php

@@ -41,7 +41,7 @@ class DrizzleSchemaManager extends AbstractSchemaManager
         $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'],

lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php

@@ -112,6 +112,10 @@ class SQLAnywhereSchemaManager extends AbstractSchemaManager
         if ($tableColumn['default']) {
             // Strip quotes from default value.
             $default = preg_replace(array("/^'(.*)'$/", "/''/"), array("$1", "'"), $tableColumn['default']);
+
+            if ('autoincrement' == $default) {
+                $default = null;
+            }
         }
 
         switch ($tableColumn['type']) {

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 @@ class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTest
         $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());
+    }
 }

tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php

@@ -37,6 +37,7 @@ class SQLServerPlatformTest extends AbstractPlatformTestCase
             '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''' " .