Merge pull request #2916 from icewind1991/postgres-int-to-bigint

Fix changing column from int to bigint with autoincrement

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

@@ -29,6 +29,7 @@ use Doctrine\DBAL\Types\BinaryType;
 use Doctrine\DBAL\Types\BigIntType;
 use Doctrine\DBAL\Types\BlobType;
 use Doctrine\DBAL\Types\IntegerType;
+use Doctrine\DBAL\Types\Type;
 
 /**
  * PostgreSqlPlatform.
@@ -536,12 +537,16 @@ class PostgreSqlPlatform extends AbstractPlatform
             if ($columnDiff->hasChanged('type') || $columnDiff->hasChanged('precision') || $columnDiff->hasChanged('scale') || $columnDiff->hasChanged('fixed')) {
                 $type = $column->getType();
 
+                // SERIAL/BIGSERIAL are not "real" types and we can't alter a column to that type
+                $columnDefinition = $column->toArray();
+                $columnDefinition['autoincrement'] = false;
+
                 // here was a server version check before, but DBAL API does not support this anymore.
-                $query = 'ALTER ' . $oldColumnName . ' TYPE ' . $type->getSQLDeclaration($column->toArray(), $this);
+                $query = 'ALTER ' . $oldColumnName . ' TYPE ' . $type->getSQLDeclaration($columnDefinition, $this);
                 $sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
             }
 
-            if ($columnDiff->hasChanged('default') || $columnDiff->hasChanged('type')) {
+            if ($columnDiff->hasChanged('default') || $this->typeChangeBreaksDefaultValue($columnDiff)) {
                 $defaultClause = null === $column->getDefault()
                     ? ' DROP DEFAULT'
                     : ' SET' . $this->getDefaultValueDeclarationSQL($column->toArray());
@@ -1204,6 +1209,31 @@ class PostgreSqlPlatform extends AbstractPlatform
     private function isSerialField(array $field) : bool
     {
         return $field['autoincrement'] ?? false === true && isset($field['type'])
-            && ($field['type'] instanceof IntegerType || $field['type'] instanceof BigIntType);
+            && $this->isNumericType($field['type']);
+    }
+
+    /**
+     * Check whether the type of a column is changed in a way that invalidates the default value for the column
+     *
+     * @param ColumnDiff $columnDiff
+     * @return bool
+     */
+    private function typeChangeBreaksDefaultValue(ColumnDiff $columnDiff) : bool
+    {
+        if (! $columnDiff->fromColumn) {
+            return $columnDiff->hasChanged('type');
+        }
+
+        $oldTypeIsNumeric = $this->isNumericType($columnDiff->fromColumn->getType());
+        $newTypeIsNumeric = $this->isNumericType($columnDiff->column->getType());
+
+        // default should not be changed when switching between numeric types and the default comes from a sequence
+        return $columnDiff->hasChanged('type')
+            && ! ($oldTypeIsNumeric && $newTypeIsNumeric && $columnDiff->column->getAutoincrement());
+    }
+
+    private function isNumericType(Type $type) : bool
+    {
+        return $type instanceof IntegerType || $type instanceof BigIntType;
     }
 }

tests/Doctrine/Tests/DBAL/Functional/Schema/PostgreSqlSchemaManagerTest.php

@@ -5,6 +5,9 @@ namespace Doctrine\Tests\DBAL\Functional\Schema;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Schema;
+use Doctrine\DBAL\Schema\Comparator;
+use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Schema\TableDiff;
 use Doctrine\DBAL\Types\Type;
 
 class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
@@ -463,6 +466,42 @@ class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
 
         self::assertNull($columns['id']->getDefault());
     }
+
+    /**
+     * @group 2916
+     *
+     * @dataProvider autoIncrementTypeMigrations
+     */
+    public function testAlterTableAutoIncrementIntToBigInt(string $from, string $to, string $expected) : void
+    {
+        $tableFrom = new Table('autoinc_type_modification');
+        $column = $tableFrom->addColumn('id', $from);
+        $column->setAutoincrement(true);
+        $this->_sm->dropAndCreateTable($tableFrom);
+        $tableFrom = $this->_sm->listTableDetails('autoinc_type_modification');
+        self::assertTrue($tableFrom->getColumn('id')->getAutoincrement());
+
+        $tableTo = new Table('autoinc_type_modification');
+        $column = $tableTo->addColumn('id', $to);
+        $column->setAutoincrement(true);
+
+        $c = new Comparator();
+        $diff = $c->diffTable($tableFrom, $tableTo);
+        self::assertInstanceOf(TableDiff::class, $diff, "There should be a difference and not false being returned from the table comparison");
+        self::assertSame(['ALTER TABLE autoinc_type_modification ALTER id TYPE ' . $expected], $this->_conn->getDatabasePlatform()->getAlterTableSQL($diff));
+
+        $this->_sm->alterTable($diff);
+        $tableFinal = $this->_sm->listTableDetails('autoinc_type_modification');
+        self::assertTrue($tableFinal->getColumn('id')->getAutoincrement());
+    }
+
+    public function autoIncrementTypeMigrations() : array
+    {
+        return [
+            'int->bigint' => ['integer', 'bigint', 'BIGINT'],
+            'bigint->int' => ['bigint', 'integer', 'INT'],
+        ];
+    }
 }
 
 class MoneyType extends Type