Skip to content

Commit 96cd2b6

Browse files
committed
fix #6198: stop considering generated column definition as being its default value
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.
1 parent 45941c6 commit 96cd2b6

6 files changed

+84
-6
lines changed

src/Driver/AbstractPostgreSQLDriver.php

+5
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Doctrine\DBAL\Exception;
99
use Doctrine\DBAL\Platforms\AbstractPlatform;
1010
use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
11+
use Doctrine\DBAL\Platforms\PostgreSQL120Platform;
1112
use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
1213
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
1314
use Doctrine\DBAL\Schema\PostgreSQLSchemaManager;
@@ -40,6 +41,10 @@ public function createDatabasePlatformForVersion($version)
4041
$patchVersion = $versionParts['patch'] ?? 0;
4142
$version = $majorVersion . '.' . $minorVersion . '.' . $patchVersion;
4243

44+
if (version_compare($version, '12.0', '>=')) {
45+
return new PostgreSQL120Platform();
46+
}
47+
4348
if (version_compare($version, '10.0', '>=')) {
4449
return new PostgreSQL100Platform();
4550
}
+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\DBAL\Platforms;
6+
7+
/**
8+
* Provides the behavior, features and SQL dialect of the PostgreSQL 12.0 database platform.
9+
*/
10+
class PostgreSQL120Platform extends PostgreSQL100Platform
11+
{
12+
public function getDefaultColumnValueSQLSnippet(): string
13+
{
14+
// in case of GENERATED ALWAYS AS ( foobar) STORED column (added in PostgreSQL 12.0)
15+
// postgreSQL's pg_get_expr(adbin, adrelid) will return the 'foobar' part
16+
// which is not the 'default' value of the column but its 'definition'
17+
// so in that case we force it to NULL as DBAL will use that column only for the
18+
// 'default' value
19+
return <<<'SQL'
20+
SELECT
21+
CASE
22+
WHEN a.attgenerated = 's' THEN NULL
23+
ELSE pg_get_expr(adbin, adrelid)
24+
END
25+
FROM pg_attrdef
26+
WHERE c.oid = pg_attrdef.adrelid
27+
AND pg_attrdef.adnum=a.attnum
28+
SQL;
29+
}
30+
}

src/Platforms/PostgreSQLPlatform.php

+13
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,19 @@ public function getTruncateTableSQL($tableName, $cascade = false)
11861186
return $sql;
11871187
}
11881188

1189+
/**
1190+
* Get the snippet used to retrieve the default value for a given column
1191+
*/
1192+
public function getDefaultColumnValueSQLSnippet(): string
1193+
{
1194+
return <<<'SQL'
1195+
SELECT pg_get_expr(adbin, adrelid)
1196+
FROM pg_attrdef
1197+
WHERE c.oid = pg_attrdef.adrelid
1198+
AND pg_attrdef.adnum=a.attnum
1199+
SQL;
1200+
}
1201+
11891202
/**
11901203
* {@inheritDoc}
11911204
*/

src/Schema/PostgreSQLSchemaManager.php

+4-6
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,9 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
616616
$sql .= ' c.relname AS table_name, n.nspname AS schema_name,';
617617
}
618618

619-
$sql .= <<<'SQL'
619+
$fragmentDefaultValueSQL = $this->_platform->getDefaultColumnValueSQLSnippet();
620+
621+
$sql .= <<<SQL
620622
a.attnum,
621623
quote_ident(a.attname) AS field,
622624
t.typname AS type,
@@ -632,11 +634,7 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
632634
AND pg_index.indkey[0] = a.attnum
633635
AND pg_index.indisprimary = 't'
634636
) AS pri,
635-
(SELECT pg_get_expr(adbin, adrelid)
636-
FROM pg_attrdef
637-
WHERE c.oid = pg_attrdef.adrelid
638-
AND pg_attrdef.adnum=a.attnum
639-
) AS default,
637+
($fragmentDefaultValueSQL) AS default,
640638
(SELECT pg_description.description
641639
FROM pg_description WHERE pg_description.objoid = c.oid AND a.attnum = pg_description.objsubid
642640
) AS comment

tests/Driver/VersionAwarePlatformDriverTest.php

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Doctrine\DBAL\Platforms\MySQL80Platform;
1515
use Doctrine\DBAL\Platforms\MySQLPlatform;
1616
use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
17+
use Doctrine\DBAL\Platforms\PostgreSQL120Platform;
1718
use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
1819
use Doctrine\DBAL\VersionAwarePlatformDriver;
1920
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
@@ -109,6 +110,7 @@ public static function postgreSQLVersionProvider(): array
109110
['9.4.0', PostgreSQL94Platform::class],
110111
['9.4.1', PostgreSQL94Platform::class],
111112
['10', PostgreSQL100Platform::class],
113+
['12', PostgreSQL120Platform::class],
112114
];
113115
}
114116

tests/Functional/Schema/PostgreSQLSchemaManagerTest.php

+30
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
66
use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
77
use Doctrine\DBAL\Platforms\AbstractPlatform;
8+
use Doctrine\DBAL\Platforms\PostgreSQL120Platform;
89
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
910
use Doctrine\DBAL\Schema\AbstractSchemaManager;
1011
use Doctrine\DBAL\Schema\Comparator;
@@ -320,6 +321,35 @@ public function testBooleanDefault(callable $comparatorFactory): void
320321
self::assertFalse($diff);
321322
}
322323

324+
/**
325+
* @param callable(AbstractSchemaManager):Comparator $comparatorFactory
326+
*
327+
* @dataProvider \Doctrine\DBAL\Tests\Functional\Schema\ComparatorTestUtils::comparatorProvider
328+
*/
329+
public function testGeneratedColumn(callable $comparatorFactory): void
330+
{
331+
$wrappedConnection = $this->connection->getWrappedConnection();
332+
if (! $this->connection->getDatabasePlatform() instanceof PostgreSQL120Platform) {
333+
self::markTestSkipped('Generated columns are not supported in Postgres 11 and earlier');
334+
}
335+
336+
$table = new Table('ddc6198_generated_always_as');
337+
$table->addColumn('id', Types::INTEGER);
338+
$table->addColumn(
339+
'idIsOdd',
340+
Types::BOOLEAN,
341+
['columnDefinition' => 'boolean GENERATED ALWAYS AS (id % 2 = 1) STORED', 'notNull' => false],
342+
);
343+
344+
$this->dropAndCreateTable($table);
345+
346+
$databaseTable = $this->schemaManager->introspectTable($table->getName());
347+
348+
$diff = $comparatorFactory($this->schemaManager)->diffTable($table, $databaseTable);
349+
350+
self::assertFalse($diff);
351+
}
352+
323353
/**
324354
* PostgreSQL stores BINARY columns as BLOB
325355
*/

0 commit comments

Comments
 (0)