Skip to content

Commit

Permalink
Fix 2193 Postgres GENERATED ALWAYS (#2195)
Browse files Browse the repository at this point in the history 
* Update the postgres generated clause to GENERATED BY DEFAULT to allow database seeding.

* Copied insert statements from PdoAdapter and updated to include OVERRIDING SYSTEM VALUE to allow database seeding when a column was created with GENERATED ALWAYS.

* Update PostgresAdapter unit tests with expected output.

* Update PostgresAdapter unit tests with default generated clause.

* Only override system value if Postgres version is greater than or equal to 10.

* Add checks for Postgres version and coding standard fix.

* Another Postgres version check.
  • Loading branch information
@captain-redbeard
captain-redbeard authored Sep 7, 2023
1 parent 9fb8416 commit 7bc24ba
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 6 deletions.
86 changes: 86 additions & 0 deletions src/Phinx/Db/Adapter/PostgresAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ class PostgresAdapter extends PdoAdapter
{
public const GENERATED_ALWAYS = 'ALWAYS';
public const GENERATED_BY_DEFAULT = 'BY DEFAULT';
/**
* Allow insert when a column was created with the GENERATED ALWAYS clause.
* This is required for seeding the database.
*/
public const OVERRIDE_SYSTEM_VALUE = 'OVERRIDING SYSTEM VALUE';

/**
* @var string[]
Expand Down Expand Up @@ -1586,4 +1591,85 @@ public function setSearchPath(): void
)
);
}

/**
* @inheritDoc
*/
public function insert(Table $table, array $row): void
{
$sql = sprintf(
'INSERT INTO %s ',
$this->quoteTableName($table->getName())
);
$columns = array_keys($row);
$sql .= '(' . implode(', ', array_map([$this, 'quoteColumnName'], $columns)) . ')';

foreach ($row as $column => $value) {
if (is_bool($value)) {
$row[$column] = $this->castToBool($value);
}
}

$override = '';
if ($this->useIdentity) {
$override = self::OVERRIDE_SYSTEM_VALUE . ' ';
}

if ($this->isDryRunEnabled()) {
$sql .= ' ' . $override . 'VALUES (' . implode(', ', array_map([$this, 'quoteValue'], $row)) . ');';
$this->output->writeln($sql);
} else {
$sql .= ' ' . $override . 'VALUES (' . implode(', ', array_fill(0, count($columns), '?')) . ')';
$stmt = $this->getConnection()->prepare($sql);
$stmt->execute(array_values($row));
}
}

/**
* @inheritDoc
*/
public function bulkinsert(Table $table, array $rows): void
{
$sql = sprintf(
'INSERT INTO %s ',
$this->quoteTableName($table->getName())
);
$current = current($rows);
$keys = array_keys($current);

$override = '';
if ($this->useIdentity) {
$override = self::OVERRIDE_SYSTEM_VALUE . ' ';
}

$sql .= '(' . implode(', ', array_map([$this, 'quoteColumnName'], $keys)) . ') ' . $override . 'VALUES ';

if ($this->isDryRunEnabled()) {
$values = array_map(function ($row) {
return '(' . implode(', ', array_map([$this, 'quoteValue'], $row)) . ')';
}, $rows);
$sql .= implode(', ', $values) . ';';
$this->output->writeln($sql);
} else {
$count_keys = count($keys);
$query = '(' . implode(', ', array_fill(0, $count_keys, '?')) . ')';
$count_vars = count($rows);
$queries = array_fill(0, $count_vars, $query);
$sql .= implode(',', $queries);
$stmt = $this->getConnection()->prepare($sql);
$vals = [];

foreach ($rows as $row) {
foreach ($row as $v) {
if (is_bool($v)) {
$vals[] = $this->castToBool($v);
} else {
$vals[] = $v;
}
}
}

$stmt->execute($vals);
}
}
}
2 changes: 1 addition & 1 deletion src/Phinx/Db/Table/Column.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class Column
*
* @var ?string
*/
protected $generated = PostgresAdapter::GENERATED_ALWAYS;
protected $generated = PostgresAdapter::GENERATED_BY_DEFAULT;

/**
* @var int|null
Expand Down
34 changes: 29 additions & 5 deletions tests/Phinx/Db/Adapter/PostgresAdapterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,16 @@ public function testAddColumnWithAutoIdentity()
foreach ($columns as $column) {
if ($column->getName() === 'id') {
$this->assertTrue($column->getIdentity());
$this->assertEquals(PostgresAdapter::GENERATED_ALWAYS, $column->getGenerated());
$this->assertEquals(PostgresAdapter::GENERATED_BY_DEFAULT, $column->getGenerated());
}
}
}

public function providerAddColumnIdentity(): array
{
return [
[PostgresAdapter::GENERATED_ALWAYS, false], //testAddColumnWithIdentityAlways
[PostgresAdapter::GENERATED_BY_DEFAULT, true], //testAddColumnWithIdentityDefault
[PostgresAdapter::GENERATED_ALWAYS, true], //testAddColumnWithIdentityAlways
[PostgresAdapter::GENERATED_BY_DEFAULT, false], //testAddColumnWithIdentityDefault
[null, true], //testAddColumnWithoutIdentity
];
}
Expand Down Expand Up @@ -2233,7 +2233,7 @@ public function testDumpCreateTable()
->save();

if ($this->usingPostgres10()) {
$expectedOutput = 'CREATE TABLE "public"."table1" ("id" INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY, "column1" CHARACTER VARYING (255) ' .
$expectedOutput = 'CREATE TABLE "public"."table1" ("id" INTEGER NOT NULL GENERATED BY DEFAULT AS IDENTITY, "column1" CHARACTER VARYING (255) ' .
'NULL, "column2" INTEGER NULL, "column3" CHARACTER VARYING (255) NOT NULL DEFAULT \'test\', CONSTRAINT ' .
'"table1_pkey" PRIMARY KEY ("id"));';
} else {
Expand Down Expand Up @@ -2265,7 +2265,7 @@ public function testDumpCreateTableWithSchema()
->save();

if ($this->usingPostgres10()) {
$expectedOutput = 'CREATE TABLE "schema1"."table1" ("id" INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY, "column1" CHARACTER VARYING (255) ' .
$expectedOutput = 'CREATE TABLE "schema1"."table1" ("id" INTEGER NOT NULL GENERATED BY DEFAULT AS IDENTITY, "column1" CHARACTER VARYING (255) ' .
'NULL, "column2" INTEGER NULL, "column3" CHARACTER VARYING (255) NOT NULL DEFAULT \'test\', CONSTRAINT ' .
'"table1_pkey" PRIMARY KEY ("id"));';
} else {
Expand Down Expand Up @@ -2312,10 +2312,19 @@ public function testDumpInsert()
]);

$expectedOutput = <<<'OUTPUT'
INSERT INTO "public"."table1" ("string_col") OVERRIDING SYSTEM VALUE VALUES ('test data');
INSERT INTO "public"."table1" ("string_col") OVERRIDING SYSTEM VALUE VALUES (null);
INSERT INTO "public"."table1" ("int_col") OVERRIDING SYSTEM VALUE VALUES (23);
OUTPUT;

if (!$this->usingPostgres10()) {
$expectedOutput = <<<'OUTPUT'
INSERT INTO "public"."table1" ("string_col") VALUES ('test data');
INSERT INTO "public"."table1" ("string_col") VALUES (null);
INSERT INTO "public"."table1" ("int_col") VALUES (23);
OUTPUT;
}

$actualOutput = $consoleOutput->fetch();
$this->assertStringContainsString(
$expectedOutput,
Expand Down Expand Up @@ -2359,8 +2368,15 @@ public function testDumpBulkinsert()
]);

$expectedOutput = <<<'OUTPUT'
INSERT INTO "public"."table1" ("string_col", "int_col") OVERRIDING SYSTEM VALUE VALUES ('test_data1', 23), (null, 42);
OUTPUT;

if (!$this->usingPostgres10()) {
$expectedOutput = <<<'OUTPUT'
INSERT INTO "public"."table1" ("string_col", "int_col") VALUES ('test_data1', 23), (null, 42);
OUTPUT;
}

$actualOutput = $consoleOutput->fetch();
$this->assertStringContainsString(
$expectedOutput,
Expand Down Expand Up @@ -2395,8 +2411,16 @@ public function testDumpCreateTableAndThenInsert()

$expectedOutput = <<<'OUTPUT'
CREATE TABLE "schema1"."table1" ("column1" CHARACTER VARYING (255) NOT NULL, "column2" INTEGER NULL, CONSTRAINT "table1_pkey" PRIMARY KEY ("column1"));
INSERT INTO "schema1"."table1" ("column1", "column2") OVERRIDING SYSTEM VALUE VALUES ('id1', 1);
OUTPUT;

if (!$this->usingPostgres10()) {
$expectedOutput = <<<'OUTPUT'
CREATE TABLE "schema1"."table1" ("column1" CHARACTER VARYING (255) NOT NULL, "column2" INTEGER NULL, CONSTRAINT "table1_pkey" PRIMARY KEY ("column1"));
INSERT INTO "schema1"."table1" ("column1", "column2") VALUES ('id1', 1);
OUTPUT;
}

$actualOutput = $consoleOutput->fetch();
$this->assertStringContainsString($expectedOutput, $actualOutput, 'Passing the --dry-run option does not dump create and then insert table queries to the output');
}
Expand Down

0 comments on commit 7bc24ba

Please sign in to comment.