fix doctrine#6198: stop considering generated column definition as being its default value (doctrine#6199)

|      Q       |   A
|------------- | -----------
| Type         | bug/feature/improvement
| Fixed issues | doctrine#6198 




#### Summary

The `pg_get_expr(adbin, adrelid)` expression in Postgresql's internal
table `pg_attrdef` is used to know a column definition

for most column, if this returns something, it means this column's
default value. So DBAL has been considering has ALWAYS meaning it
contains its default.

However for generated columns, it contains the value definition and not
its default value, so we change the selectTableColumns' query to
correctly set the 'default' column to `null` for generated columns

It will help setting correctly the column's attributes which in turn
will help generate correct schema diff.

<!-- Fill in the relevant information below to help triage your pull
request. -->

Author: allan-simon

src/Driver/AbstractPostgreSQLDriver.php

@@ -8,6 +8,7 @@
 use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
+use Doctrine\DBAL\Platforms\PostgreSQL120Platform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Schema\PostgreSQLSchemaManager;
@@ -40,6 +41,10 @@ public function createDatabasePlatformForVersion($version)
         $patchVersion = $versionParts['patch'] ?? 0;
         $version      = $majorVersion . '.' . $minorVersion . '.' . $patchVersion;
 
+        if (version_compare($version, '12.0', '>=')) {
+            return new PostgreSQL120Platform();
+        }
+
         if (version_compare($version, '10.0', '>=')) {
             return new PostgreSQL100Platform();
         }

src/Platforms/PostgreSQL120Platform.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Platforms;
+
+/**
+ * Provides the behavior, features and SQL dialect of the PostgreSQL 12.0 database platform.
+ */
+class PostgreSQL120Platform extends PostgreSQL100Platform
+{
+    public function getDefaultColumnValueSQLSnippet(): string
+    {
+        // in case of GENERATED ALWAYS AS (foobar) STORED column (added in PostgreSQL 12.0)
+        // PostgreSQL's pg_get_expr(adbin, adrelid) will return the 'foobar' part
+        // which is not the 'default' value of the column but its 'definition'
+        // so in that case we force it to NULL as DBAL will use that column only for the
+        // 'default' value
+        return <<<'SQL'
+            SELECT
+                CASE
+                    WHEN a.attgenerated = 's' THEN NULL
+                    ELSE pg_get_expr(adbin, adrelid)
+                END
+             FROM pg_attrdef
+             WHERE c.oid = pg_attrdef.adrelid
+                AND pg_attrdef.adnum=a.attnum
+        SQL;
+    }
+}

src/Platforms/PostgreSQLPlatform.php

@@ -1193,6 +1193,19 @@ public function getTruncateTableSQL($tableName, $cascade = false)
         return $sql;
     }
 
+    /**
+     * Get the snippet used to retrieve the default value for a given column
+     */
+    public function getDefaultColumnValueSQLSnippet(): string
+    {
+        return <<<'SQL'
+             SELECT pg_get_expr(adbin, adrelid)
+             FROM pg_attrdef
+             WHERE c.oid = pg_attrdef.adrelid
+                AND pg_attrdef.adnum=a.attnum
+        SQL;
+    }
+
     /**
      * {@inheritDoc}
      */

src/Schema/PostgreSQLSchemaManager.php

@@ -617,7 +617,7 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
             $sql .= ' c.relname AS table_name, n.nspname AS schema_name,';
         }
 
-        $sql .= <<<'SQL'
+        $sql .= sprintf(<<<'SQL'
             a.attnum,
             quote_ident(a.attname) AS field,
             t.typname AS type,
@@ -633,11 +633,7 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
                 AND pg_index.indkey[0] = a.attnum
                 AND pg_index.indisprimary = 't'
             ) AS pri,
-            (SELECT pg_get_expr(adbin, adrelid)
-             FROM pg_attrdef
-             WHERE c.oid = pg_attrdef.adrelid
-                AND pg_attrdef.adnum=a.attnum
-            ) AS default,
+            (%s) AS default,
             (SELECT pg_description.description
                 FROM pg_description WHERE pg_description.objoid = c.oid AND a.attnum = pg_description.objsubid
             ) AS comment
@@ -652,7 +648,7 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
                     ON d.objid = c.oid
                         AND d.deptype = 'e'
                         AND d.classid = (SELECT oid FROM pg_class WHERE relname = 'pg_class')
-SQL;
+SQL, $this->_platform->getDefaultColumnValueSQLSnippet());
 
         $conditions = array_merge([
             'a.attnum > 0',

tests/Driver/VersionAwarePlatformDriverTest.php

@@ -17,6 +17,7 @@
 use Doctrine\DBAL\Platforms\MySQL84Platform;
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
+use Doctrine\DBAL\Platforms\PostgreSQL120Platform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\VersionAwarePlatformDriver;
 use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
@@ -90,6 +91,7 @@ public static function postgreSQLVersionProvider(): array
             ['9.4.0', PostgreSQL94Platform::class],
             ['9.4.1', PostgreSQL94Platform::class],
             ['10', PostgreSQL100Platform::class],
+            ['12', PostgreSQL120Platform::class],
         ];
     }
 

tests/Functional/Schema/PostgreSQLSchemaManagerTest.php

@@ -5,6 +5,7 @@
 use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
 use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\PostgreSQL120Platform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Comparator;
@@ -334,6 +335,35 @@ public function testBooleanDefault(callable $comparatorFactory): void
         self::assertFalse($diff);
     }
 
+    /**
+     * @param callable(AbstractSchemaManager):Comparator $comparatorFactory
+     *
+     * @dataProvider \Doctrine\DBAL\Tests\Functional\Schema\ComparatorTestUtils::comparatorProvider
+     */
+    public function testGeneratedColumn(callable $comparatorFactory): void
+    {
+        $wrappedConnection = $this->connection->getWrappedConnection();
+        if (! $this->connection->getDatabasePlatform() instanceof PostgreSQL120Platform) {
+             self::markTestSkipped('Generated columns are not supported in Postgres 11 and earlier');
+        }
+
+        $table = new Table('ddc6198_generated_always_as');
+        $table->addColumn('id', Types::INTEGER);
+        $table->addColumn(
+            'idIsOdd',
+            Types::BOOLEAN,
+            ['columnDefinition' => 'boolean GENERATED ALWAYS AS (id % 2 = 1) STORED', 'notNull' => false],
+        );
+
+        $this->dropAndCreateTable($table);
+
+        $databaseTable = $this->schemaManager->introspectTable($table->getName());
+
+        $diff = $comparatorFactory($this->schemaManager)->diffTable($table, $databaseTable);
+
+        self::assertFalse($diff);
+    }
+
     /**
      * PostgreSQL stores BINARY columns as BLOB
      */