fix table column alteration

lib/Doctrine/DBAL/Platforms/DB2Platform.php

@@ -19,8 +19,11 @@
 
 namespace Doctrine\DBAL\Platforms;
 
+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\ColumnDiff;
 use Doctrine\DBAL\Schema\Identifier;
 use Doctrine\DBAL\Schema\Index;
+use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\TableDiff;
 
 class DB2Platform extends AbstractPlatform
@@ -464,6 +467,7 @@ class DB2Platform extends AbstractPlatform
     {
         $sql = array();
         $columnSql = array();
+        $commentsSQL = array();
 
         $queryParts = array();
         foreach ($diff->addedColumns as $column) {
@@ -483,6 +487,16 @@ class DB2Platform extends AbstractPlatform
             }
 
             $queryParts[] = $queryPart;
+
+            $comment = $this->getColumnComment($column);
+
+            if (null !== $comment && '' !== $comment) {
+                $commentsSQL[] = $this->getCommentOnColumnSQL(
+                    $diff->getName($this)->getQuotedName($this),
+                    $column->getQuotedName($this),
+                    $comment
+                );
+            }
         }
 
         foreach ($diff->removedColumns as $column) {
@@ -498,10 +512,19 @@ class DB2Platform extends AbstractPlatform
                 continue;
             }
 
-            /* @var $columnDiff \Doctrine\DBAL\Schema\ColumnDiff */
-            $column = $columnDiff->column;
-            $queryParts[] =  'ALTER ' . ($columnDiff->getOldColumnName()->getQuotedName($this)) . ' '
-                    . $this->getColumnDeclarationSQL($column->getQuotedName($this), $column->toArray());
+            if ($columnDiff->hasChanged('comment')) {
+                $commentsSQL[] = $this->getCommentOnColumnSQL(
+                    $diff->getName($this)->getQuotedName($this),
+                    $columnDiff->column->getQuotedName($this),
+                    $this->getColumnComment($columnDiff->column)
+                );
+
+                if (count($columnDiff->changedProperties) === 1) {
+                    continue;
+                }
+            }
+
+            $this->gatherAlterColumnSQL($diff->fromTable, $columnDiff, $sql, $queryParts);
         }
 
         foreach ($diff->renamedColumns as $oldColumnName => $column) {
@@ -527,6 +550,8 @@ class DB2Platform extends AbstractPlatform
                 $sql[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE " . $diff->getName($this)->getQuotedName($this) . "')";
             }
 
+            $sql = array_merge($sql, $commentsSQL);
+
             if ($diff->newName !== false) {
                 $sql[] =  'RENAME TABLE ' . $diff->getName($this)->getQuotedName($this) . ' TO ' . $diff->getNewName()->getQuotedName($this);
             }
@@ -541,6 +566,84 @@ class DB2Platform extends AbstractPlatform
         return array_merge($sql, $tableSql, $columnSql);
     }
 
+    /**
+     * Gathers the table alteration SQL for a given column diff.
+     *
+     * @param Table      $table      The table to gather the SQL for.
+     * @param ColumnDiff $columnDiff The column diff to evaluate.
+     * @param array      $sql        The sequence of table alteration statements to fill.
+     * @param array      $queryParts The sequence of column alteration clauses to fill.
+     */
+    private function gatherAlterColumnSQL(Table $table, ColumnDiff $columnDiff, array &$sql, array &$queryParts)
+    {
+        $alterColumnClauses = $this->getAlterColumnClausesSQL($columnDiff);
+
+        if (empty($alterColumnClauses)) {
+            return;
+        }
+
+        // If we have a single column alteration, we can append the clause to the main query.
+        if (count($alterColumnClauses) === 1) {
+            $queryParts[] = current($alterColumnClauses);
+
+            return;
+        }
+
+        // We have multiple alterations for the same column,
+        // so we need to trigger a complete ALTER TABLE statement
+        // for each ALTER COLUMN clause.
+        foreach ($alterColumnClauses as $alterColumnClause) {
+            $sql[] = 'ALTER TABLE ' . $table->getQuotedName($this) . ' ' . $alterColumnClause;
+        }
+    }
+
+    /**
+     * Returns the ALTER COLUMN SQL clauses for altering a column described by the given column diff.
+     *
+     * @param ColumnDiff $columnDiff The column diff to evaluate.
+     *
+     * @return array
+     */
+    private function getAlterColumnClausesSQL(ColumnDiff $columnDiff)
+    {
+        $column = $columnDiff->column->toArray();
+
+        $alterClause = 'ALTER COLUMN ' . $columnDiff->column->getQuotedName($this);
+
+        if ($column['columnDefinition']) {
+            return array($alterClause . ' ' . $column['columnDefinition']);
+        }
+
+        $clauses = array();
+
+        if ($columnDiff->hasChanged('type') ||
+            $columnDiff->hasChanged('length') ||
+            $columnDiff->hasChanged('precision') ||
+            $columnDiff->hasChanged('scale') ||
+            $columnDiff->hasChanged('fixed')
+        ) {
+            $clauses[] = $alterClause . ' SET DATA TYPE ' . $column['type']->getSQLDeclaration($column, $this);
+        }
+
+        if ($columnDiff->hasChanged('notnull')) {
+            $clauses[] = $column['notnull'] ? $alterClause . ' SET NOT NULL' : $alterClause . ' DROP NOT NULL';
+        }
+
+        if ($columnDiff->hasChanged('default')) {
+            if (isset($column['default'])) {
+                $defaultClause = $this->getDefaultValueDeclarationSQL($column);
+
+                if ($defaultClause) {
+                    $clauses[] = $alterClause . ' SET' . $defaultClause;
+                }
+            } else {
+                $clauses[] = $alterClause . ' DROP DEFAULT';
+            }
+        }
+
+        return $clauses;
+    }
+
     /**
      * {@inheritDoc}
      */

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

@@ -4,8 +4,12 @@ namespace Doctrine\Tests\DBAL\Platforms;
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\DB2Platform;
+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\ColumnDiff;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Schema\TableDiff;
+use Doctrine\DBAL\Types\Type;
 
 class DB2PlatformTest extends AbstractPlatformTestCase
 {
@@ -22,7 +26,15 @@ class DB2PlatformTest extends AbstractPlatformTestCase
     public function getGenerateAlterTableSql()
     {
         return array(
-            "ALTER TABLE mytable ADD COLUMN quota INTEGER DEFAULT NULL DROP COLUMN foo ALTER bar baz VARCHAR(255) DEFAULT 'def' NOT NULL ALTER bloo bloo SMALLINT DEFAULT '0' NOT NULL",
+            "ALTER TABLE mytable ALTER COLUMN baz SET DATA TYPE VARCHAR(255)",
+            "ALTER TABLE mytable ALTER COLUMN baz SET NOT NULL",
+            "ALTER TABLE mytable ALTER COLUMN baz SET DEFAULT 'def'",
+            "ALTER TABLE mytable ALTER COLUMN bloo SET DATA TYPE SMALLINT",
+            "ALTER TABLE mytable ALTER COLUMN bloo SET NOT NULL",
+            "ALTER TABLE mytable ALTER COLUMN bloo SET DEFAULT '0'",
+            "ALTER TABLE mytable " .
+            "ADD COLUMN quota INTEGER DEFAULT NULL " .
+            "DROP COLUMN foo",
             "CALL SYSPROC.ADMIN_CMD ('REORG TABLE mytable')",
             'RENAME TABLE mytable TO userlist',
         );
@@ -101,8 +113,12 @@ class DB2PlatformTest extends AbstractPlatformTestCase
     public function getAlterTableColumnCommentsSQL()
     {
         return array(
-            "ALTER TABLE mytable ADD COLUMN quota INTEGER NOT NULL WITH DEFAULT ALTER foo foo VARCHAR(255) NOT NULL ALTER bar baz VARCHAR(255) NOT NULL",
-            "CALL SYSPROC.ADMIN_CMD ('REORG TABLE mytable')"
+            "ALTER TABLE mytable " .
+            "ADD COLUMN quota INTEGER NOT NULL WITH DEFAULT",
+            "CALL SYSPROC.ADMIN_CMD ('REORG TABLE mytable')",
+            "COMMENT ON COLUMN mytable.quota IS 'A comment'",
+            "COMMENT ON COLUMN mytable.foo IS ''",
+            "COMMENT ON COLUMN mytable.baz IS 'B comment'",
         );
     }
 
@@ -490,8 +506,11 @@ class DB2PlatformTest extends AbstractPlatformTestCase
         return array(
             'ALTER TABLE "foo" DROP FOREIGN KEY fk1',
             'ALTER TABLE "foo" DROP FOREIGN KEY fk2',
-            'ALTER TABLE "foo" ADD COLUMN bloo INTEGER NOT NULL WITH DEFAULT DROP COLUMN baz ' .
-            'ALTER bar bar INTEGER DEFAULT NULL RENAME COLUMN id TO war',
+            'ALTER TABLE "foo" ' .
+            'ADD COLUMN bloo INTEGER NOT NULL WITH DEFAULT ' .
+            'DROP COLUMN baz ' .
+            'ALTER COLUMN bar DROP NOT NULL ' .
+            'RENAME COLUMN id TO war',
             'CALL SYSPROC.ADMIN_CMD (\'REORG TABLE "foo"\')',
             'RENAME TABLE "foo" TO "table"',
             'ALTER TABLE "table" ADD CONSTRAINT fk_add FOREIGN KEY (fk3) REFERENCES fk_table (id)',
@@ -510,4 +529,90 @@ class DB2PlatformTest extends AbstractPlatformTestCase
             'COMMENT ON COLUMN "select"."from" IS \'comment\'',
         );
     }
+
+    /**
+     * @group DBAL-944
+     *
+     * @dataProvider getGeneratesAlterColumnSQL
+     */
+    public function testGeneratesAlterColumnSQL($changedProperty, Column $column, $expectedSQLClause = null)
+    {
+        $tableDiff = new TableDiff('foo');
+        $tableDiff->fromTable = new Table('foo');
+        $tableDiff->changedColumns['bar'] = new ColumnDiff('bar', $column, array($changedProperty));
+
+        $expectedSQL = array();
+
+        if (null !== $expectedSQLClause) {
+            $expectedSQL[] = 'ALTER TABLE foo ALTER COLUMN bar ' . $expectedSQLClause;
+        }
+
+        $expectedSQL[] = "CALL SYSPROC.ADMIN_CMD ('REORG TABLE foo')";
+
+        $this->assertSame($expectedSQL, $this->_platform->getAlterTableSQL($tableDiff));
+    }
+
+    /**
+     * @return array
+     */
+    public function getGeneratesAlterColumnSQL()
+    {
+        return array(
+            array(
+                'columnDefinition',
+                new Column('bar', Type::getType('decimal'), array('columnDefinition' => 'MONEY NOT NULL')),
+                'MONEY NOT NULL'
+            ),
+            array(
+                'type',
+                new Column('bar', Type::getType('integer')),
+                'SET DATA TYPE INTEGER'
+            ),
+            array(
+                'length',
+                new Column('bar', Type::getType('string'), array('length' => 100)),
+                'SET DATA TYPE VARCHAR(100)'
+            ),
+            array(
+                'precision',
+                new Column('bar', Type::getType('decimal'), array('precision' => 10, 'scale' => 2)),
+                'SET DATA TYPE NUMERIC(10, 2)'
+            ),
+            array(
+                'scale',
+                new Column('bar', Type::getType('decimal'), array('precision' => 5, 'scale' => 4)),
+                'SET DATA TYPE NUMERIC(5, 4)'
+            ),
+            array(
+                'fixed',
+                new Column('bar', Type::getType('string'), array('length' => 20, 'fixed' => true)),
+                'SET DATA TYPE CHAR(20)'
+            ),
+            array(
+                'notnull',
+                new Column('bar', Type::getType('string'), array('notnull' => true)),
+                'SET NOT NULL'
+            ),
+            array(
+                'notnull',
+                new Column('bar', Type::getType('string'), array('notnull' => false)),
+                'DROP NOT NULL'
+            ),
+            array(
+                'default',
+                new Column('bar', Type::getType('string'), array('default' => 'foo')),
+                "SET DEFAULT 'foo'"
+            ),
+            array(
+                'default',
+                new Column('bar', Type::getType('integer'), array('autoincrement' => true, 'default' => 666)),
+                null
+            ),
+            array(
+                'default',
+                new Column('bar', Type::getType('string')),
+                "DROP DEFAULT"
+            ),
+        );
+    }
 }