Merge pull request #5396 from morozov/pgsql-identity

Autoincrement via identity columns on PostgreSQL
doctrine · May 15, 2022 · a4338b7 · a4338b7
2 parents d40bde4 + a7f9dcf
commit a4338b7
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 57 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -8,6 +8,14 @@ awareness about deprecated code.
 
 # Upgrade to 4.0
 
+## BC BREAK: Auto-increment columns on PostgreSQL are implemented as `IDENTITY`, not `SERIAL`.
+
+Instead of using `SERIAL*` column types for `autoincrement` columns, the DBAL will now use
+the `GENERATED BY DEFAULT AS IDENTITY` clause.
+
+The upgrade to DBAL 4 will require manual migration of the database schema.
+See the [documentation](docs/en/how-to/postgresql-identity-migration.rst) for more details.
+
 ## Removed the `doctrine-dbal` binary.
 
 The documentation explains how the console tools can be bootstrapped for standalone usage.

diff --git a/docs/en/how-to/postgresql-identity-migration.rst b/docs/en/how-to/postgresql-identity-migration.rst
@@ -0,0 +1,67 @@
+Migration to identity columns on PostgreSQL
+===========================================
+
+As of version 4, the DBAL uses identity columns to implement the `autoincrement`
+behavior on PostgreSQL instead of `SERIAL*` column types.
+
+If you have a database with `autoincrement` columns created using DBAL 3 or earlier,
+you will need to perform the following schema migration before being able to continue managing
+the schema with the DBAL:
+
+1. Identify all `autoincrement` columns and their tables.
+2. Create the `upgrade_serial_to_identity()` function in the database as described in
+   `PostgreSQL 10 identity columns explained <https://www.2ndquadrant.com/en/blog/postgresql-10-identity-columns/>`_:
+
+    .. code-block:: sql
+
+      CREATE OR REPLACE FUNCTION upgrade_serial_to_identity(tbl regclass, col name)
+      RETURNS void
+      LANGUAGE plpgsql
+      AS $$
+      DECLARE
+        colnum smallint;
+        seqid oid;
+        count int;
+      BEGIN
+        -- find column number
+        SELECT attnum INTO colnum FROM pg_attribute WHERE attrelid = tbl AND attname = col;
+        IF NOT FOUND THEN
+          RAISE EXCEPTION 'column does not exist';
+        END IF;
+
+        -- find sequence
+           SELECT INTO seqid objid
+          FROM pg_depend
+          WHERE (refclassid, refobjid, refobjsubid) = ('pg_class'::regclass, tbl, colnum)
+            AND classid = 'pg_class'::regclass AND objsubid = 0
+            AND deptype = 'a';
+
+        GET DIAGNOSTICS count = ROW_COUNT;
+        IF count < 1 THEN
+          RAISE EXCEPTION 'no linked sequence found';
+        ELSIF count > 1 THEN
+          RAISE EXCEPTION 'more than one linked sequence found';
+        END IF;
+
+        -- drop the default
+        EXECUTE 'ALTER TABLE ' || tbl || ' ALTER COLUMN ' || quote_ident(col) || ' DROP DEFAULT';
+
+        -- change the dependency between column and sequence to internal
+        UPDATE pg_depend
+          SET deptype = 'i'
+          WHERE (classid, objid, objsubid) = ('pg_class'::regclass, seqid, 0)
+            AND deptype = 'a';
+
+        -- mark the column as identity column
+        UPDATE pg_attribute
+          SET attidentity = 'd'
+          WHERE attrelid = tbl
+            AND attname = col;
+      END;
+      $$;
+
+3. For each column and their table, run `upgrade_serial_to_identity(<table>, <column>)`.
+
+Without this migration, next time when DBAL 4 is used to manage the schema, it will perform a similar migration
+but instead of reusing the existing sequence, it will drop it and create a new one. As a result,
+all new sequence numbers will be generated from `1`, which is most likely undesired.
diff --git a/docs/en/sidebar.rst b/docs/en/sidebar.rst
@@ -21,3 +21,5 @@
     reference/testing
 
     explanation/implicit-indexes
+
+    how-to/postgresql-identity-migration
diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php
@@ -330,19 +330,13 @@ public function getAlterTableSQL(TableDiff $diff): array
 
             if ($columnDiff->hasChanged('autoincrement')) {
                 if ($column->getAutoincrement()) {
-                    // add autoincrement
-                    $seqName = $this->getIdentitySequenceName($diff->name, $oldColumnName);
-
-                    $sql[] = 'CREATE SEQUENCE ' . $seqName;
-                    $sql[] = "SELECT setval('" . $seqName . "', (SELECT MAX(" . $oldColumnName . ') FROM '
-                        . $diff->getName($this)->getQuotedName($this) . '))';
-                    $query = 'ALTER ' . $oldColumnName . " SET DEFAULT nextval('" . $seqName . "')";
-                    $sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
+                    $query = 'ADD GENERATED BY DEFAULT AS IDENTITY';
                 } else {
-                    // Drop autoincrement, but do NOT drop the sequence. It might be re-used by other tables or have
-                    $query = 'ALTER ' . $oldColumnName . ' DROP DEFAULT';
-                    $sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this) . ' ' . $query;
+                    $query = 'DROP IDENTITY';
                 }
+
+                $sql[] = 'ALTER TABLE ' . $diff->getName($this)->getQuotedName($this)
+                    . ' ALTER ' . $oldColumnName . ' ' . $query;
             }
 
             $newComment = $column->getComment();
@@ -657,35 +651,23 @@ public function getBooleanTypeDeclarationSQL(array $column): string
      */
     public function getIntegerTypeDeclarationSQL(array $column): string
     {
-        if (! empty($column['autoincrement'])) {
-            return 'SERIAL';
-        }
-
-        return 'INT';
+        return 'INT' . $this->_getCommonIntegerTypeDeclarationSQL($column);
     }
 
     /**
      * {@inheritDoc}
      */
     public function getBigIntTypeDeclarationSQL(array $column): string
     {
-        if (! empty($column['autoincrement'])) {
-            return 'BIGSERIAL';
-        }
-
-        return 'BIGINT';
+        return 'BIGINT' . $this->_getCommonIntegerTypeDeclarationSQL($column);
     }
 
     /**
      * {@inheritDoc}
      */
     public function getSmallIntTypeDeclarationSQL(array $column): string
     {
-        if (! empty($column['autoincrement'])) {
-            return 'SMALLSERIAL';
-        }
-
-        return 'SMALLINT';
+        return 'SMALLINT' . $this->_getCommonIntegerTypeDeclarationSQL($column);
     }
 
     /**
@@ -733,6 +715,10 @@ public function getTimeTypeDeclarationSQL(array $column): string
      */
     protected function _getCommonIntegerTypeDeclarationSQL(array $column): string
     {
+        if (! empty($column['autoincrement'])) {
+            return ' GENERATED BY DEFAULT AS IDENTITY';
+        }
+
         return '';
     }
 

diff --git a/src/Schema/PostgreSQLSchemaManager.php b/src/Schema/PostgreSQLSchemaManager.php
@@ -254,18 +254,9 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column
             $length = (int) $matches[1];
         }
 
-        $matches = [];
-
-        $autoincrement = false;
+        $autoincrement = $tableColumn['attidentity'] === 'd';
 
-        if (
-            $tableColumn['default'] !== null
-            && preg_match("/^nextval\('(.*)'(::.*)?\)$/", $tableColumn['default'], $matches) === 1
-        ) {
-            $tableColumn['sequence'] = $matches[1];
-            $tableColumn['default']  = null;
-            $autoincrement           = true;
-        }
+        $matches = [];
 
         if ($tableColumn['default'] !== null) {
             if (preg_match("/^['(](.*)[')]::/", $tableColumn['default'], $matches) === 1) {
@@ -441,6 +432,7 @@ protected function selectDatabaseColumns(string $databaseName, ?string $tableNam
             (SELECT format_type(t2.typbasetype, t2.typtypmod) FROM
               pg_catalog.pg_type t2 WHERE t2.typtype = 'd' AND t2.oid = a.atttypid) AS domain_complete_type,
             a.attnotnull AS isnotnull,
+            a.attidentity,
             (SELECT 't'
              FROM pg_index
              WHERE c.oid = pg_index.indrelid

diff --git a/tests/Functional/Schema/PostgreSQLSchemaManagerTest.php b/tests/Functional/Schema/PostgreSQLSchemaManagerTest.php
@@ -4,7 +4,6 @@
 
 namespace Doctrine\DBAL\Tests\Functional\Schema;
 
-use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
@@ -67,12 +66,6 @@ public function testDetectsAutoIncrement(): void
 
     public function testAlterTableAutoIncrementAdd(): void
     {
-        // see https://github.com/doctrine/dbal/issues/4745
-        try {
-            $this->schemaManager->dropSequence('autoinc_table_add_id_seq');
-        } catch (DatabaseObjectNotFoundException $e) {
-        }
-
         $tableFrom = new Table('autoinc_table_add');
         $tableFrom->addColumn('id', 'integer');
         $this->dropAndCreateTable($tableFrom);
@@ -89,11 +82,7 @@ public function testAlterTableAutoIncrementAdd(): void
         self::assertNotNull($diff);
 
         $sql = $platform->getAlterTableSQL($diff);
-        self::assertEquals([
-            'CREATE SEQUENCE autoinc_table_add_id_seq',
-            "SELECT setval('autoinc_table_add_id_seq', (SELECT MAX(id) FROM autoinc_table_add))",
-            "ALTER TABLE autoinc_table_add ALTER id SET DEFAULT nextval('autoinc_table_add_id_seq')",
-        ], $sql);
+        self::assertEquals(['ALTER TABLE autoinc_table_add ALTER id ADD GENERATED BY DEFAULT AS IDENTITY'], $sql);
 
         $this->schemaManager->alterTable($diff);
         $tableFinal = $this->schemaManager->listTableDetails('autoinc_table_add');
@@ -118,7 +107,7 @@ public function testAlterTableAutoIncrementDrop(): void
         self::assertNotNull($diff);
 
         self::assertEquals(
-            ['ALTER TABLE autoinc_table_drop ALTER id DROP DEFAULT'],
+            ['ALTER TABLE autoinc_table_drop ALTER id DROP IDENTITY'],
             $platform->getAlterTableSQL($diff)
         );
 

diff --git a/tests/Platforms/PostgreSQLPlatformTest.php b/tests/Platforms/PostgreSQLPlatformTest.php
@@ -31,7 +31,8 @@ public function createPlatform(): AbstractPlatform
 
     public function getGenerateTableSql(): string
     {
-        return 'CREATE TABLE test (id SERIAL NOT NULL, test VARCHAR(255) DEFAULT NULL, PRIMARY KEY(id))';
+        return 'CREATE TABLE test (id INT GENERATED BY DEFAULT AS IDENTITY NOT NULL, test VARCHAR(255) DEFAULT NULL'
+            . ', PRIMARY KEY(id))';
     }
 
     /**
@@ -207,7 +208,7 @@ public function testGenerateTableWithAutoincrement(): void
         $column->setAutoincrement(true);
 
         self::assertEquals(
-            ['CREATE TABLE autoinc_table (id SERIAL NOT NULL)'],
+            ['CREATE TABLE autoinc_table (id INT GENERATED BY DEFAULT AS IDENTITY NOT NULL)'],
             $this->platform->getCreateTableSQL($table)
         );
     }
@@ -218,8 +219,8 @@ public function testGenerateTableWithAutoincrement(): void
     public static function serialTypes(): iterable
     {
         return [
-            ['integer', 'SERIAL'],
-            ['bigint', 'BIGSERIAL'],
+            ['integer', 'INT GENERATED BY DEFAULT AS IDENTITY'],
+            ['bigint', 'BIGINT GENERATED BY DEFAULT AS IDENTITY'],
         ];
     }
 
@@ -276,11 +277,11 @@ public function testGeneratesTypeDeclarationForIntegers(): void
             $this->platform->getIntegerTypeDeclarationSQL([])
         );
         self::assertEquals(
-            'SERIAL',
+            'INT GENERATED BY DEFAULT AS IDENTITY',
             $this->platform->getIntegerTypeDeclarationSQL(['autoincrement' => true])
         );
         self::assertEquals(
-            'SERIAL',
+            'INT GENERATED BY DEFAULT AS IDENTITY',
             $this->platform->getIntegerTypeDeclarationSQL(
                 ['autoincrement' => true, 'primary' => true]
             )
@@ -911,7 +912,7 @@ public function testReturnsJsonTypeDeclarationSQL(): void
     public function testReturnsSmallIntTypeDeclarationSQL(): void
     {
         self::assertSame(
-            'SMALLSERIAL',
+            'SMALLINT GENERATED BY DEFAULT AS IDENTITY',
             $this->platform->getSmallIntTypeDeclarationSQL(['autoincrement' => true])
         );