Skip to content

Commit 388a81b

Browse files
author
Etsuro Fujita
committed
postgres_fdw: Fix issues with generated columns in foreign tables.
postgres_fdw imported generated columns from the remote tables as plain columns, and caused failures like "ERROR: cannot insert a non-DEFAULT value into column "foo"" when inserting into the foreign tables, as it tried to insert values into the generated columns. To fix, we do the following under the assumption that generated columns in a postgres_fdw foreign table are defined so that they represent generated columns in the underlying remote table: * Send DEFAULT for the generated columns to the foreign server on insert or update, not generated column values computed on the local server. * Add to postgresImportForeignSchema() an option "import_generated" to include column generated expressions in the definitions of foreign tables imported from a foreign server. The option is true by default. The assumption seems reasonable, because that would make a query of the postgres_fdw foreign table return values for the generated columns that are consistent with the generated expression. While here, fix another issue in postgresImportForeignSchema(): it tried to include column generated expressions as column default expressions in the foreign table definitions when the import_default option was enabled. Per bug #16631 from Daniel Cherniy. Back-patch to v12 where generated columns were added. Discussion: https://postgr.es/m/16631-e929fe9db0ffc7cf%40postgresql.org
1 parent a72ad63 commit 388a81b

File tree

5 files changed

+172
-26
lines changed

5 files changed

+172
-26
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,6 +1708,7 @@ deparseInsertSql(StringInfo buf, RangeTblEntry *rte,
17081708
List *withCheckOptionList, List *returningList,
17091709
List **retrieved_attrs)
17101710
{
1711+
TupleDesc tupdesc = RelationGetDescr(rel);
17111712
AttrNumber pindex;
17121713
bool first;
17131714
ListCell *lc;
@@ -1737,12 +1738,20 @@ deparseInsertSql(StringInfo buf, RangeTblEntry *rte,
17371738
first = true;
17381739
foreach(lc, targetAttrs)
17391740
{
1741+
int attnum = lfirst_int(lc);
1742+
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
1743+
17401744
if (!first)
17411745
appendStringInfoString(buf, ", ");
17421746
first = false;
17431747

1744-
appendStringInfo(buf, "$%d", pindex);
1745-
pindex++;
1748+
if (attr->attgenerated)
1749+
appendStringInfoString(buf, "DEFAULT");
1750+
else
1751+
{
1752+
appendStringInfo(buf, "$%d", pindex);
1753+
pindex++;
1754+
}
17461755
}
17471756

17481757
appendStringInfoChar(buf, ')');
@@ -1772,6 +1781,7 @@ deparseUpdateSql(StringInfo buf, RangeTblEntry *rte,
17721781
List *withCheckOptionList, List *returningList,
17731782
List **retrieved_attrs)
17741783
{
1784+
TupleDesc tupdesc = RelationGetDescr(rel);
17751785
AttrNumber pindex;
17761786
bool first;
17771787
ListCell *lc;
@@ -1785,14 +1795,20 @@ deparseUpdateSql(StringInfo buf, RangeTblEntry *rte,
17851795
foreach(lc, targetAttrs)
17861796
{
17871797
int attnum = lfirst_int(lc);
1798+
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
17881799

17891800
if (!first)
17901801
appendStringInfoString(buf, ", ");
17911802
first = false;
17921803

17931804
deparseColumnRef(buf, rtindex, attnum, rte, false);
1794-
appendStringInfo(buf, " = $%d", pindex);
1795-
pindex++;
1805+
if (attr->attgenerated)
1806+
appendStringInfoString(buf, " = DEFAULT");
1807+
else
1808+
{
1809+
appendStringInfo(buf, " = $%d", pindex);
1810+
pindex++;
1811+
}
17961812
}
17971813
appendStringInfoString(buf, " WHERE ctid = $1");
17981814

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 76 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6395,13 +6395,36 @@ select * from rem1;
63956395
-- ===================================================================
63966396
-- test generated columns
63976397
-- ===================================================================
6398-
create table gloc1 (a int, b int);
6398+
create table gloc1 (
6399+
a int,
6400+
b int generated always as (a * 2) stored);
63996401
alter table gloc1 set (autovacuum_enabled = 'false');
64006402
create foreign table grem1 (
64016403
a int,
64026404
b int generated always as (a * 2) stored)
64036405
server loopback options(table_name 'gloc1');
6406+
explain (verbose, costs off)
6407+
insert into grem1 (a) values (1), (2);
6408+
QUERY PLAN
6409+
-------------------------------------------------------------------
6410+
Insert on public.grem1
6411+
Remote SQL: INSERT INTO public.gloc1(a, b) VALUES ($1, DEFAULT)
6412+
-> Values Scan on "*VALUES*"
6413+
Output: "*VALUES*".column1, NULL::integer
6414+
(4 rows)
6415+
64046416
insert into grem1 (a) values (1), (2);
6417+
explain (verbose, costs off)
6418+
update grem1 set a = 22 where a = 2;
6419+
QUERY PLAN
6420+
---------------------------------------------------------------------------------
6421+
Update on public.grem1
6422+
Remote SQL: UPDATE public.gloc1 SET a = $2, b = DEFAULT WHERE ctid = $1
6423+
-> Foreign Scan on public.grem1
6424+
Output: 22, b, ctid
6425+
Remote SQL: SELECT b, ctid FROM public.gloc1 WHERE ((a = 2)) FOR UPDATE
6426+
(5 rows)
6427+
64056428
update grem1 set a = 22 where a = 2;
64066429
select * from gloc1;
64076430
a | b
@@ -6417,6 +6440,24 @@ select * from grem1;
64176440
22 | 44
64186441
(2 rows)
64196442

6443+
delete from grem1;
6444+
-- test copy from
6445+
copy grem1 from stdin;
6446+
select * from gloc1;
6447+
a | b
6448+
---+---
6449+
1 | 2
6450+
2 | 4
6451+
(2 rows)
6452+
6453+
select * from grem1;
6454+
a | b
6455+
---+---
6456+
1 | 2
6457+
2 | 4
6458+
(2 rows)
6459+
6460+
delete from grem1;
64206461
-- ===================================================================
64216462
-- test local triggers
64226463
-- ===================================================================
@@ -8316,6 +8357,7 @@ CREATE TABLE import_source.t3 (c1 timestamptz default now(), c2 typ1);
83168357
CREATE TABLE import_source."x 4" (c1 float8, "C 2" text, c3 varchar(42));
83178358
CREATE TABLE import_source."x 5" (c1 float8);
83188359
ALTER TABLE import_source."x 5" DROP COLUMN c1;
8360+
CREATE TABLE import_source."x 6" (c1 int, c2 int generated always as (c1 * 2) stored);
83198361
CREATE TABLE import_source.t4 (c1 int) PARTITION BY RANGE (c1);
83208362
CREATE TABLE import_source.t4_part PARTITION OF import_source.t4
83218363
FOR VALUES FROM (1) TO (100);
@@ -8331,7 +8373,8 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest1;
83318373
import_dest1 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
83328374
import_dest1 | x 4 | loopback | (schema_name 'import_source', table_name 'x 4') |
83338375
import_dest1 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8334-
(6 rows)
8376+
import_dest1 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8377+
(7 rows)
83358378

83368379
\d import_dest1.*
83378380
Foreign table "import_dest1.t1"
@@ -8381,6 +8424,14 @@ FDW options: (schema_name 'import_source', table_name 'x 4')
83818424
Server: loopback
83828425
FDW options: (schema_name 'import_source', table_name 'x 5')
83838426

8427+
Foreign table "import_dest1.x 6"
8428+
Column | Type | Collation | Nullable | Default | FDW options
8429+
--------+---------+-----------+----------+-------------------------------------+--------------------
8430+
c1 | integer | | | | (column_name 'c1')
8431+
c2 | integer | | | generated always as (c1 * 2) stored | (column_name 'c2')
8432+
Server: loopback
8433+
FDW options: (schema_name 'import_source', table_name 'x 6')
8434+
83848435
-- Options
83858436
CREATE SCHEMA import_dest2;
83868437
IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest2
@@ -8395,7 +8446,8 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest2
83958446
import_dest2 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
83968447
import_dest2 | x 4 | loopback | (schema_name 'import_source', table_name 'x 4') |
83978448
import_dest2 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8398-
(6 rows)
8449+
import_dest2 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8450+
(7 rows)
83998451

84008452
\d import_dest2.*
84018453
Foreign table "import_dest2.t1"
@@ -8445,9 +8497,17 @@ FDW options: (schema_name 'import_source', table_name 'x 4')
84458497
Server: loopback
84468498
FDW options: (schema_name 'import_source', table_name 'x 5')
84478499

8500+
Foreign table "import_dest2.x 6"
8501+
Column | Type | Collation | Nullable | Default | FDW options
8502+
--------+---------+-----------+----------+-------------------------------------+--------------------
8503+
c1 | integer | | | | (column_name 'c1')
8504+
c2 | integer | | | generated always as (c1 * 2) stored | (column_name 'c2')
8505+
Server: loopback
8506+
FDW options: (schema_name 'import_source', table_name 'x 6')
8507+
84488508
CREATE SCHEMA import_dest3;
84498509
IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
8450-
OPTIONS (import_collate 'false', import_not_null 'false');
8510+
OPTIONS (import_collate 'false', import_generated 'false', import_not_null 'false');
84518511
\det+ import_dest3.*
84528512
List of foreign tables
84538513
Schema | Table | Server | FDW options | Description
@@ -8458,7 +8518,8 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
84588518
import_dest3 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
84598519
import_dest3 | x 4 | loopback | (schema_name 'import_source', table_name 'x 4') |
84608520
import_dest3 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8461-
(6 rows)
8521+
import_dest3 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8522+
(7 rows)
84628523

84638524
\d import_dest3.*
84648525
Foreign table "import_dest3.t1"
@@ -8508,6 +8569,14 @@ FDW options: (schema_name 'import_source', table_name 'x 4')
85088569
Server: loopback
85098570
FDW options: (schema_name 'import_source', table_name 'x 5')
85108571

8572+
Foreign table "import_dest3.x 6"
8573+
Column | Type | Collation | Nullable | Default | FDW options
8574+
--------+---------+-----------+----------+---------+--------------------
8575+
c1 | integer | | | | (column_name 'c1')
8576+
c2 | integer | | | | (column_name 'c2')
8577+
Server: loopback
8578+
FDW options: (schema_name 'import_source', table_name 'x 6')
8579+
85118580
-- Check LIMIT TO and EXCEPT
85128581
CREATE SCHEMA import_dest4;
85138582
IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch)
@@ -8530,7 +8599,8 @@ IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch)
85308599
import_dest4 | t3 | loopback | (schema_name 'import_source', table_name 't3') |
85318600
import_dest4 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
85328601
import_dest4 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8533-
(5 rows)
8602+
import_dest4 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8603+
(6 rows)
85348604

85358605
-- Assorted error cases
85368606
IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest4;

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3621,6 +3621,9 @@ create_foreign_modify(EState *estate,
36213621

36223622
Assert(!attr->attisdropped);
36233623

3624+
/* Ignore generated columns; they are set to DEFAULT */
3625+
if (attr->attgenerated)
3626+
continue;
36243627
getTypeOutputInfo(attr->atttypid, &typefnoid, &isvarlena);
36253628
fmgr_info(typefnoid, &fmstate->p_flinfo[fmstate->p_nums]);
36263629
fmstate->p_nums++;
@@ -3806,6 +3809,7 @@ convert_prep_stmt_params(PgFdwModifyState *fmstate,
38063809
/* get following parameters from slot */
38073810
if (slot != NULL && fmstate->target_attrs != NIL)
38083811
{
3812+
TupleDesc tupdesc = RelationGetDescr(fmstate->rel);
38093813
int nestlevel;
38103814
ListCell *lc;
38113815

@@ -3814,9 +3818,13 @@ convert_prep_stmt_params(PgFdwModifyState *fmstate,
38143818
foreach(lc, fmstate->target_attrs)
38153819
{
38163820
int attnum = lfirst_int(lc);
3821+
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
38173822
Datum value;
38183823
bool isnull;
38193824

3825+
/* Ignore generated columns; they are set to DEFAULT */
3826+
if (attr->attgenerated)
3827+
continue;
38203828
value = slot_getattr(slot, attnum, &isnull);
38213829
if (isnull)
38223830
p_values[pindex] = NULL;
@@ -4728,6 +4736,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
47284736
List *commands = NIL;
47294737
bool import_collate = true;
47304738
bool import_default = false;
4739+
bool import_generated = true;
47314740
bool import_not_null = true;
47324741
ForeignServer *server;
47334742
UserMapping *mapping;
@@ -4747,6 +4756,8 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
47474756
import_collate = defGetBoolean(def);
47484757
else if (strcmp(def->defname, "import_default") == 0)
47494758
import_default = defGetBoolean(def);
4759+
else if (strcmp(def->defname, "import_generated") == 0)
4760+
import_generated = defGetBoolean(def);
47504761
else if (strcmp(def->defname, "import_not_null") == 0)
47514762
import_not_null = defGetBoolean(def);
47524763
else
@@ -4808,13 +4819,24 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
48084819
* include a schema name for types/functions in other schemas, which
48094820
* is what we want.
48104821
*/
4822+
appendStringInfoString(&buf,
4823+
"SELECT relname, "
4824+
" attname, "
4825+
" format_type(atttypid, atttypmod), "
4826+
" attnotnull, ");
4827+
4828+
/* Generated columns are supported since Postgres 12 */
4829+
if (PQserverVersion(conn) >= 120000)
4830+
appendStringInfoString(&buf,
4831+
" attgenerated, "
4832+
" pg_get_expr(adbin, adrelid), ");
4833+
else
4834+
appendStringInfoString(&buf,
4835+
" NULL, "
4836+
" pg_get_expr(adbin, adrelid), ");
4837+
48114838
if (import_collate)
48124839
appendStringInfoString(&buf,
4813-
"SELECT relname, "
4814-
" attname, "
4815-
" format_type(atttypid, atttypmod), "
4816-
" attnotnull, "
4817-
" pg_get_expr(adbin, adrelid), "
48184840
" collname, "
48194841
" collnsp.nspname "
48204842
"FROM pg_class c "
@@ -4831,11 +4853,6 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
48314853
" collnsp.oid = collnamespace ");
48324854
else
48334855
appendStringInfoString(&buf,
4834-
"SELECT relname, "
4835-
" attname, "
4836-
" format_type(atttypid, atttypmod), "
4837-
" attnotnull, "
4838-
" pg_get_expr(adbin, adrelid), "
48394856
" NULL, NULL "
48404857
"FROM pg_class c "
48414858
" JOIN pg_namespace n ON "
@@ -4911,6 +4928,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
49114928
char *attname;
49124929
char *typename;
49134930
char *attnotnull;
4931+
char *attgenerated;
49144932
char *attdefault;
49154933
char *collname;
49164934
char *collnamespace;
@@ -4922,12 +4940,14 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
49224940
attname = PQgetvalue(res, i, 1);
49234941
typename = PQgetvalue(res, i, 2);
49244942
attnotnull = PQgetvalue(res, i, 3);
4925-
attdefault = PQgetisnull(res, i, 4) ? (char *) NULL :
4943+
attgenerated = PQgetisnull(res, i, 4) ? (char *) NULL :
49264944
PQgetvalue(res, i, 4);
4927-
collname = PQgetisnull(res, i, 5) ? (char *) NULL :
4945+
attdefault = PQgetisnull(res, i, 5) ? (char *) NULL :
49284946
PQgetvalue(res, i, 5);
4929-
collnamespace = PQgetisnull(res, i, 6) ? (char *) NULL :
4947+
collname = PQgetisnull(res, i, 6) ? (char *) NULL :
49304948
PQgetvalue(res, i, 6);
4949+
collnamespace = PQgetisnull(res, i, 7) ? (char *) NULL :
4950+
PQgetvalue(res, i, 7);
49314951

49324952
if (first_item)
49334953
first_item = false;
@@ -4955,9 +4975,20 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
49554975
quote_identifier(collname));
49564976

49574977
/* Add DEFAULT if needed */
4958-
if (import_default && attdefault != NULL)
4978+
if (import_default && attdefault != NULL &&
4979+
(!attgenerated || !attgenerated[0]))
49594980
appendStringInfo(&buf, " DEFAULT %s", attdefault);
49604981

4982+
/* Add GENERATED if needed */
4983+
if (import_generated && attgenerated != NULL &&
4984+
attgenerated[0] == ATTRIBUTE_GENERATED_STORED)
4985+
{
4986+
Assert(attdefault != NULL);
4987+
appendStringInfo(&buf,
4988+
" GENERATED ALWAYS AS (%s) STORED",
4989+
attdefault);
4990+
}
4991+
49614992
/* Add NOT NULL if needed */
49624993
if (import_not_null && attnotnull[0] == 't')
49634994
appendStringInfoString(&buf, " NOT NULL");

0 commit comments

Comments
 (0)