From c1e9c490012c25f0f05561bdad0d56041ac814f6 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 15 Dec 2017 11:57:09 -0500 Subject: [PATCH 1/2] Apply identity column expression in build_column_default() --- src/backend/commands/copy.c | 16 ++-------------- src/backend/rewrite/rewriteHandler.c | 22 +++++++++++----------- src/test/regress/expected/identity.out | 30 ++++++++++++++++++++++++++++++ src/test/regress/sql/identity.sql | 19 +++++++++++++++++++ src/test/subscription/t/008_diff_schema.pl | 18 ++++++++++++++---- 5 files changed, 76 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 254be28ae4..bace390470 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -23,7 +23,6 @@ #include "access/sysattr.h" #include "access/xact.h" #include "access/xlog.h" -#include "catalog/dependency.h" #include "catalog/pg_type.h" #include "commands/copy.h" #include "commands/defrem.h" @@ -3068,19 +3067,8 @@ BeginCopyFrom(ParseState *pstate, { /* attribute is NOT to be copied from input */ /* use default value if one exists */ - Expr *defexpr; - - if (att->attidentity) - { - NextValueExpr *nve = makeNode(NextValueExpr); - - nve->seqid = getOwnedSequence(RelationGetRelid(cstate->rel), - attnum); - nve->typeId = att->atttypid; - defexpr = (Expr *) nve; - } - else - defexpr = (Expr *) build_column_default(cstate->rel, attnum); + Expr *defexpr = (Expr *) build_column_default(cstate->rel, + attnum); if (defexpr != NULL) { diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index e93552a8f3..93354ecb5b 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -844,17 +844,7 @@ rewriteTargetListIU(List *targetList, { Node *new_expr; - if (att_tup->attidentity) - { - NextValueExpr *nve = makeNode(NextValueExpr); - - nve->seqid = getOwnedSequence(RelationGetRelid(target_relation), attrno); - nve->typeId = att_tup->atttypid; - - new_expr = (Node *) nve; - } - else - new_expr = build_column_default(target_relation, attrno); + new_expr = build_column_default(target_relation, attrno); /* * If there is no default (ie, default is effectively NULL), we @@ -1123,6 +1113,16 @@ build_column_default(Relation rel, int attrno) Node *expr = NULL; Oid exprtype; + if (att_tup->attidentity) + { + NextValueExpr *nve = makeNode(NextValueExpr); + + nve->seqid = getOwnedSequence(RelationGetRelid(rel), attrno); + nve->typeId = att_tup->atttypid; + + return (Node *) nve; + } + /* * Scan to see if relation has a default for this column. */ diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out index 87ef0d3b2a..59b7eb91a8 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -104,6 +104,19 @@ SELECT * FROM itest4; 2 | (2 rows) +-- VALUES RTEs +INSERT INTO itest3 VALUES (DEFAULT, 'a'); +INSERT INTO itest3 VALUES (DEFAULT, 'b'), (DEFAULT, 'c'); +SELECT * FROM itest3; + a | b +----+--- + 7 | + 12 | + 17 | a + 22 | b + 27 | c +(5 rows) + -- OVERRIDING tests INSERT INTO itest1 VALUES (10, 'xyz'); INSERT INTO itest1 OVERRIDING USER VALUE VALUES (10, 'xyz'); @@ -237,6 +250,23 @@ SELECT * FROM itestv11; 11 | xyz (3 rows) +-- ADD COLUMN +CREATE TABLE itest13 (a int); +-- add column to empty table +ALTER TABLE itest13 ADD COLUMN b int GENERATED BY DEFAULT AS IDENTITY; +ERROR: no owned sequence found +INSERT INTO itest13 VALUES (1), (2), (3); +-- add column to populated table +ALTER TABLE itest13 ADD COLUMN c int GENERATED BY DEFAULT AS IDENTITY; +ERROR: no owned sequence found +SELECT * FROM itest13; + a +--- + 1 + 2 + 3 +(3 rows) + -- various ALTER COLUMN tests -- fail, not allowed for identity columns ALTER TABLE itest1 ALTER COLUMN a SET DEFAULT 1; diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql index 1b2d11cf34..8be086d7ea 100644 --- a/src/test/regress/sql/identity.sql +++ b/src/test/regress/sql/identity.sql @@ -54,6 +54,14 @@ CREATE TABLE itest_err_4 (a serial generated by default as identity); SELECT * FROM itest4; +-- VALUES RTEs + +INSERT INTO itest3 VALUES (DEFAULT, 'a'); +INSERT INTO itest3 VALUES (DEFAULT, 'b'), (DEFAULT, 'c'); + +SELECT * FROM itest3; + + -- OVERRIDING tests INSERT INTO itest1 VALUES (10, 'xyz'); @@ -138,6 +146,17 @@ CREATE VIEW itestv11 AS SELECT * FROM itest11; SELECT * FROM itestv11; +-- ADD COLUMN + +CREATE TABLE itest13 (a int); +-- add column to empty table +ALTER TABLE itest13 ADD COLUMN b int GENERATED BY DEFAULT AS IDENTITY; +INSERT INTO itest13 VALUES (1), (2), (3); +-- add column to populated table +ALTER TABLE itest13 ADD COLUMN c int GENERATED BY DEFAULT AS IDENTITY; +SELECT * FROM itest13; + + -- various ALTER COLUMN tests -- fail, not allowed for identity columns diff --git a/src/test/subscription/t/008_diff_schema.pl b/src/test/subscription/t/008_diff_schema.pl index b71be6e487..cca4480f52 100644 --- a/src/test/subscription/t/008_diff_schema.pl +++ b/src/test/subscription/t/008_diff_schema.pl @@ -3,7 +3,7 @@ use warnings; use PostgresNode; use TestLib; -use Test::More tests => 3; +use Test::More tests => 4; sub wait_for_caught_up { @@ -31,7 +31,7 @@ sub wait_for_caught_up "INSERT INTO test_tab VALUES (1, 'foo'), (2, 'bar')"); # Setup structure on subscriber -$node_subscriber->safe_psql('postgres', "CREATE TABLE test_tab (a int primary key, b text, c timestamptz DEFAULT now(), d bigint DEFAULT 999)"); +$node_subscriber->safe_psql('postgres', "CREATE TABLE test_tab (a int primary key, b text, c timestamptz DEFAULT now(), d bigint DEFAULT 999, e int GENERATED BY DEFAULT AS IDENTITY)"); # Setup logical replication my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; @@ -61,8 +61,8 @@ sub wait_for_caught_up wait_for_caught_up($node_publisher, $appname); $result = - $node_subscriber->safe_psql('postgres', "SELECT count(*), count(c), count(d = 999) FROM test_tab"); -is($result, qq(2|2|2), 'check extra columns contain local defaults'); + $node_subscriber->safe_psql('postgres', "SELECT count(*), count(c), count(d = 999), count(e) FROM test_tab"); +is($result, qq(2|2|2|2), 'check extra columns contain local defaults after copy'); # Change the local values of the extra columns on the subscriber, # update publisher, and check that subscriber retains the expected @@ -76,5 +76,15 @@ sub wait_for_caught_up $node_subscriber->safe_psql('postgres', "SELECT count(*), count(extract(epoch from c) = 987654321), count(d = 999) FROM test_tab"); is($result, qq(2|2|2), 'check extra columns contain locally changed data'); +# Another insert +$node_publisher->safe_psql('postgres', + "INSERT INTO test_tab VALUES (3, 'baz')"); + +wait_for_caught_up($node_publisher, $appname); + +$result = + $node_subscriber->safe_psql('postgres', "SELECT count(*), count(c), count(d = 999), count(e) FROM test_tab"); +is($result, qq(3|3|3|3), 'check extra columns contain local defaults after apply'); + $node_subscriber->stop; $node_publisher->stop; -- 2.15.1