Unverified Commit ae25512f authored by Robin Appelman's avatar Robin Appelman Committed by Luís Cobucci

Fix changing column from int to bigint with autoincrement

- SERIAL and BIGSERIAL are not true types and can't be used in ALTER TABLE expressions
- When changing between int and bigint auto increment fields the default should no be dropped
  to preserve the link between the column and the sequence.
parent 2202adf8
......@@ -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;
}
}
......@@ -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
......
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