Unverified Commit 754880b6 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 8c1e5136
...@@ -29,6 +29,7 @@ use Doctrine\DBAL\Types\BinaryType; ...@@ -29,6 +29,7 @@ use Doctrine\DBAL\Types\BinaryType;
use Doctrine\DBAL\Types\BigIntType; use Doctrine\DBAL\Types\BigIntType;
use Doctrine\DBAL\Types\BlobType; use Doctrine\DBAL\Types\BlobType;
use Doctrine\DBAL\Types\IntegerType; use Doctrine\DBAL\Types\IntegerType;
use Doctrine\DBAL\Types\Type;
/** /**
* PostgreSqlPlatform. * PostgreSqlPlatform.
...@@ -536,12 +537,16 @@ class PostgreSqlPlatform extends AbstractPlatform ...@@ -536,12 +537,16 @@ class PostgreSqlPlatform extends AbstractPlatform
if ($columnDiff->hasChanged('type') || $columnDiff->hasChanged('precision') || $columnDiff->hasChanged('scale') || $columnDiff->hasChanged('fixed')) { if ($columnDiff->hasChanged('type') || $columnDiff->hasChanged('precision') || $columnDiff->hasChanged('scale') || $columnDiff->hasChanged('fixed')) {
$type = $column->getType(); $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. // 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; $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() $defaultClause = null === $column->getDefault()
? ' DROP DEFAULT' ? ' DROP DEFAULT'
: ' SET' . $this->getDefaultValueDeclarationSQL($column->toArray()); : ' SET' . $this->getDefaultValueDeclarationSQL($column->toArray());
...@@ -1204,6 +1209,31 @@ class PostgreSqlPlatform extends AbstractPlatform ...@@ -1204,6 +1209,31 @@ class PostgreSqlPlatform extends AbstractPlatform
private function isSerialField(array $field) : bool private function isSerialField(array $field) : bool
{ {
return $field['autoincrement'] ?? false === true && isset($field['type']) 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; ...@@ -5,6 +5,9 @@ namespace Doctrine\Tests\DBAL\Functional\Schema;
use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\PostgreSQL94Platform; use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
use Doctrine\DBAL\Schema; use Doctrine\DBAL\Schema;
use Doctrine\DBAL\Schema\Comparator;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Type;
class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
...@@ -463,6 +466,42 @@ class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase ...@@ -463,6 +466,42 @@ class PostgreSqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
self::assertNull($columns['id']->getDefault()); 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 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