Skip to content

Commit bbc0cd8

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 43644bd commit bbc0cd8

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
@@ -1710,6 +1710,7 @@ deparseInsertSql(StringInfo buf, RangeTblEntry *rte,
17101710
List *withCheckOptionList, List *returningList,
17111711
List **retrieved_attrs)
17121712
{
1713+
TupleDesc tupdesc = RelationGetDescr(rel);
17131714
AttrNumber pindex;
17141715
bool first;
17151716
ListCell *lc;
@@ -1739,12 +1740,20 @@ deparseInsertSql(StringInfo buf, RangeTblEntry *rte,
17391740
first = true;
17401741
foreach(lc, targetAttrs)
17411742
{
1743+
int attnum = lfirst_int(lc);
1744+
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
1745+
17421746
if (!first)
17431747
appendStringInfoString(buf, ", ");
17441748
first = false;
17451749

1746-
appendStringInfo(buf, "$%d", pindex);
1747-
pindex++;
1750+
if (attr->attgenerated)
1751+
appendStringInfoString(buf, "DEFAULT");
1752+
else
1753+
{
1754+
appendStringInfo(buf, "$%d", pindex);
1755+
pindex++;
1756+
}
17481757
}
17491758

17501759
appendStringInfoChar(buf, ')');
@@ -1774,6 +1783,7 @@ deparseUpdateSql(StringInfo buf, RangeTblEntry *rte,
17741783
List *withCheckOptionList, List *returningList,
17751784
List **retrieved_attrs)
17761785
{
1786+
TupleDesc tupdesc = RelationGetDescr(rel);
17771787
AttrNumber pindex;
17781788
bool first;
17791789
ListCell *lc;
@@ -1787,14 +1797,20 @@ deparseUpdateSql(StringInfo buf, RangeTblEntry *rte,
17871797
foreach(lc, targetAttrs)
17881798
{
17891799
int attnum = lfirst_int(lc);
1800+
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
17901801

17911802
if (!first)
17921803
appendStringInfoString(buf, ", ");
17931804
first = false;
17941805

17951806
deparseColumnRef(buf, rtindex, attnum, rte, false);
1796-
appendStringInfo(buf, " = $%d", pindex);
1797-
pindex++;
1807+
if (attr->attgenerated)
1808+
appendStringInfoString(buf, " = DEFAULT");
1809+
else
1810+
{
1811+
appendStringInfo(buf, " = $%d", pindex);
1812+
pindex++;
1813+
}
17981814
}
17991815
appendStringInfoString(buf, " WHERE ctid = $1");
18001816

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 76 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6365,13 +6365,36 @@ select * from rem1;
63656365
-- ===================================================================
63666366
-- test generated columns
63676367
-- ===================================================================
6368-
create table gloc1 (a int, b int);
6368+
create table gloc1 (
6369+
a int,
6370+
b int generated always as (a * 2) stored);
63696371
alter table gloc1 set (autovacuum_enabled = 'false');
63706372
create foreign table grem1 (
63716373
a int,
63726374
b int generated always as (a * 2) stored)
63736375
server loopback options(table_name 'gloc1');
6376+
explain (verbose, costs off)
6377+
insert into grem1 (a) values (1), (2);
6378+
QUERY PLAN
6379+
-------------------------------------------------------------------
6380+
Insert on public.grem1
6381+
Remote SQL: INSERT INTO public.gloc1(a, b) VALUES ($1, DEFAULT)
6382+
-> Values Scan on "*VALUES*"
6383+
Output: "*VALUES*".column1, NULL::integer
6384+
(4 rows)
6385+
63746386
insert into grem1 (a) values (1), (2);
6387+
explain (verbose, costs off)
6388+
update grem1 set a = 22 where a = 2;
6389+
QUERY PLAN
6390+
---------------------------------------------------------------------------------
6391+
Update on public.grem1
6392+
Remote SQL: UPDATE public.gloc1 SET a = $2, b = DEFAULT WHERE ctid = $1
6393+
-> Foreign Scan on public.grem1
6394+
Output: 22, b, ctid
6395+
Remote SQL: SELECT b, ctid FROM public.gloc1 WHERE ((a = 2)) FOR UPDATE
6396+
(5 rows)
6397+
63756398
update grem1 set a = 22 where a = 2;
63766399
select * from gloc1;
63776400
a | b
@@ -6387,6 +6410,24 @@ select * from grem1;
63876410
22 | 44
63886411
(2 rows)
63896412

6413+
delete from grem1;
6414+
-- test copy from
6415+
copy grem1 from stdin;
6416+
select * from gloc1;
6417+
a | b
6418+
---+---
6419+
1 | 2
6420+
2 | 4
6421+
(2 rows)
6422+
6423+
select * from grem1;
6424+
a | b
6425+
---+---
6426+
1 | 2
6427+
2 | 4
6428+
(2 rows)
6429+
6430+
delete from grem1;
63906431
-- ===================================================================
63916432
-- test local triggers
63926433
-- ===================================================================
@@ -8286,6 +8327,7 @@ CREATE TABLE import_source.t3 (c1 timestamptz default now(), c2 typ1);
82868327
CREATE TABLE import_source."x 4" (c1 float8, "C 2" text, c3 varchar(42));
82878328
CREATE TABLE import_source."x 5" (c1 float8);
82888329
ALTER TABLE import_source."x 5" DROP COLUMN c1;
8330+
CREATE TABLE import_source."x 6" (c1 int, c2 int generated always as (c1 * 2) stored);
82898331
CREATE TABLE import_source.t4 (c1 int) PARTITION BY RANGE (c1);
82908332
CREATE TABLE import_source.t4_part PARTITION OF import_source.t4
82918333
FOR VALUES FROM (1) TO (100);
@@ -8301,7 +8343,8 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest1;
83018343
import_dest1 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
83028344
import_dest1 | x 4 | loopback | (schema_name 'import_source', table_name 'x 4') |
83038345
import_dest1 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8304-
(6 rows)
8346+
import_dest1 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8347+
(7 rows)
83058348

83068349
\d import_dest1.*
83078350
Foreign table "import_dest1.t1"
@@ -8351,6 +8394,14 @@ FDW options: (schema_name 'import_source', table_name 'x 4')
83518394
Server: loopback
83528395
FDW options: (schema_name 'import_source', table_name 'x 5')
83538396

8397+
Foreign table "import_dest1.x 6"
8398+
Column | Type | Collation | Nullable | Default | FDW options
8399+
--------+---------+-----------+----------+-------------------------------------+--------------------
8400+
c1 | integer | | | | (column_name 'c1')
8401+
c2 | integer | | | generated always as (c1 * 2) stored | (column_name 'c2')
8402+
Server: loopback
8403+
FDW options: (schema_name 'import_source', table_name 'x 6')
8404+
83548405
-- Options
83558406
CREATE SCHEMA import_dest2;
83568407
IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest2
@@ -8365,7 +8416,8 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest2
83658416
import_dest2 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
83668417
import_dest2 | x 4 | loopback | (schema_name 'import_source', table_name 'x 4') |
83678418
import_dest2 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8368-
(6 rows)
8419+
import_dest2 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8420+
(7 rows)
83698421

83708422
\d import_dest2.*
83718423
Foreign table "import_dest2.t1"
@@ -8415,9 +8467,17 @@ FDW options: (schema_name 'import_source', table_name 'x 4')
84158467
Server: loopback
84168468
FDW options: (schema_name 'import_source', table_name 'x 5')
84178469

8470+
Foreign table "import_dest2.x 6"
8471+
Column | Type | Collation | Nullable | Default | FDW options
8472+
--------+---------+-----------+----------+-------------------------------------+--------------------
8473+
c1 | integer | | | | (column_name 'c1')
8474+
c2 | integer | | | generated always as (c1 * 2) stored | (column_name 'c2')
8475+
Server: loopback
8476+
FDW options: (schema_name 'import_source', table_name 'x 6')
8477+
84188478
CREATE SCHEMA import_dest3;
84198479
IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
8420-
OPTIONS (import_collate 'false', import_not_null 'false');
8480+
OPTIONS (import_collate 'false', import_generated 'false', import_not_null 'false');
84218481
\det+ import_dest3.*
84228482
List of foreign tables
84238483
Schema | Table | Server | FDW options | Description
@@ -8428,7 +8488,8 @@ IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
84288488
import_dest3 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
84298489
import_dest3 | x 4 | loopback | (schema_name 'import_source', table_name 'x 4') |
84308490
import_dest3 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8431-
(6 rows)
8491+
import_dest3 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8492+
(7 rows)
84328493

84338494
\d import_dest3.*
84348495
Foreign table "import_dest3.t1"
@@ -8478,6 +8539,14 @@ FDW options: (schema_name 'import_source', table_name 'x 4')
84788539
Server: loopback
84798540
FDW options: (schema_name 'import_source', table_name 'x 5')
84808541

8542+
Foreign table "import_dest3.x 6"
8543+
Column | Type | Collation | Nullable | Default | FDW options
8544+
--------+---------+-----------+----------+---------+--------------------
8545+
c1 | integer | | | | (column_name 'c1')
8546+
c2 | integer | | | | (column_name 'c2')
8547+
Server: loopback
8548+
FDW options: (schema_name 'import_source', table_name 'x 6')
8549+
84818550
-- Check LIMIT TO and EXCEPT
84828551
CREATE SCHEMA import_dest4;
84838552
IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch)
@@ -8500,7 +8569,8 @@ IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch)
85008569
import_dest4 | t3 | loopback | (schema_name 'import_source', table_name 't3') |
85018570
import_dest4 | t4 | loopback | (schema_name 'import_source', table_name 't4') |
85028571
import_dest4 | x 5 | loopback | (schema_name 'import_source', table_name 'x 5') |
8503-
(5 rows)
8572+
import_dest4 | x 6 | loopback | (schema_name 'import_source', table_name 'x 6') |
8573+
(6 rows)
85048574

85058575
-- Assorted error cases
85068576
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
@@ -3567,6 +3567,9 @@ create_foreign_modify(EState *estate,
35673567

35683568
Assert(!attr->attisdropped);
35693569

3570+
/* Ignore generated columns; they are set to DEFAULT */
3571+
if (attr->attgenerated)
3572+
continue;
35703573
getTypeOutputInfo(attr->atttypid, &typefnoid, &isvarlena);
35713574
fmgr_info(typefnoid, &fmstate->p_flinfo[fmstate->p_nums]);
35723575
fmstate->p_nums++;
@@ -3752,6 +3755,7 @@ convert_prep_stmt_params(PgFdwModifyState *fmstate,
37523755
/* get following parameters from slot */
37533756
if (slot != NULL && fmstate->target_attrs != NIL)
37543757
{
3758+
TupleDesc tupdesc = RelationGetDescr(fmstate->rel);
37553759
int nestlevel;
37563760
ListCell *lc;
37573761

@@ -3760,9 +3764,13 @@ convert_prep_stmt_params(PgFdwModifyState *fmstate,
37603764
foreach(lc, fmstate->target_attrs)
37613765
{
37623766
int attnum = lfirst_int(lc);
3767+
Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1);
37633768
Datum value;
37643769
bool isnull;
37653770

3771+
/* Ignore generated columns; they are set to DEFAULT */
3772+
if (attr->attgenerated)
3773+
continue;
37663774
value = slot_getattr(slot, attnum, &isnull);
37673775
if (isnull)
37683776
p_values[pindex] = NULL;
@@ -4673,6 +4681,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
46734681
List *commands = NIL;
46744682
bool import_collate = true;
46754683
bool import_default = false;
4684+
bool import_generated = true;
46764685
bool import_not_null = true;
46774686
ForeignServer *server;
46784687
UserMapping *mapping;
@@ -4692,6 +4701,8 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
46924701
import_collate = defGetBoolean(def);
46934702
else if (strcmp(def->defname, "import_default") == 0)
46944703
import_default = defGetBoolean(def);
4704+
else if (strcmp(def->defname, "import_generated") == 0)
4705+
import_generated = defGetBoolean(def);
46954706
else if (strcmp(def->defname, "import_not_null") == 0)
46964707
import_not_null = defGetBoolean(def);
46974708
else
@@ -4753,13 +4764,24 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
47534764
* include a schema name for types/functions in other schemas, which
47544765
* is what we want.
47554766
*/
4767+
appendStringInfoString(&buf,
4768+
"SELECT relname, "
4769+
" attname, "
4770+
" format_type(atttypid, atttypmod), "
4771+
" attnotnull, ");
4772+
4773+
/* Generated columns are supported since Postgres 12 */
4774+
if (PQserverVersion(conn) >= 120000)
4775+
appendStringInfoString(&buf,
4776+
" attgenerated, "
4777+
" pg_get_expr(adbin, adrelid), ");
4778+
else
4779+
appendStringInfoString(&buf,
4780+
" NULL, "
4781+
" pg_get_expr(adbin, adrelid), ");
4782+
47564783
if (import_collate)
47574784
appendStringInfoString(&buf,
4758-
"SELECT relname, "
4759-
" attname, "
4760-
" format_type(atttypid, atttypmod), "
4761-
" attnotnull, "
4762-
" pg_get_expr(adbin, adrelid), "
47634785
" collname, "
47644786
" collnsp.nspname "
47654787
"FROM pg_class c "
@@ -4776,11 +4798,6 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
47764798
" collnsp.oid = collnamespace ");
47774799
else
47784800
appendStringInfoString(&buf,
4779-
"SELECT relname, "
4780-
" attname, "
4781-
" format_type(atttypid, atttypmod), "
4782-
" attnotnull, "
4783-
" pg_get_expr(adbin, adrelid), "
47844801
" NULL, NULL "
47854802
"FROM pg_class c "
47864803
" JOIN pg_namespace n ON "
@@ -4856,6 +4873,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
48564873
char *attname;
48574874
char *typename;
48584875
char *attnotnull;
4876+
char *attgenerated;
48594877
char *attdefault;
48604878
char *collname;
48614879
char *collnamespace;
@@ -4867,12 +4885,14 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
48674885
attname = PQgetvalue(res, i, 1);
48684886
typename = PQgetvalue(res, i, 2);
48694887
attnotnull = PQgetvalue(res, i, 3);
4870-
attdefault = PQgetisnull(res, i, 4) ? (char *) NULL :
4888+
attgenerated = PQgetisnull(res, i, 4) ? (char *) NULL :
48714889
PQgetvalue(res, i, 4);
4872-
collname = PQgetisnull(res, i, 5) ? (char *) NULL :
4890+
attdefault = PQgetisnull(res, i, 5) ? (char *) NULL :
48734891
PQgetvalue(res, i, 5);
4874-
collnamespace = PQgetisnull(res, i, 6) ? (char *) NULL :
4892+
collname = PQgetisnull(res, i, 6) ? (char *) NULL :
48754893
PQgetvalue(res, i, 6);
4894+
collnamespace = PQgetisnull(res, i, 7) ? (char *) NULL :
4895+
PQgetvalue(res, i, 7);
48764896

48774897
if (first_item)
48784898
first_item = false;
@@ -4900,9 +4920,20 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
49004920
quote_identifier(collname));
49014921

49024922
/* Add DEFAULT if needed */
4903-
if (import_default && attdefault != NULL)
4923+
if (import_default && attdefault != NULL &&
4924+
(!attgenerated || !attgenerated[0]))
49044925
appendStringInfo(&buf, " DEFAULT %s", attdefault);
49054926

4927+
/* Add GENERATED if needed */
4928+
if (import_generated && attgenerated != NULL &&
4929+
attgenerated[0] == ATTRIBUTE_GENERATED_STORED)
4930+
{
4931+
Assert(attdefault != NULL);
4932+
appendStringInfo(&buf,
4933+
" GENERATED ALWAYS AS (%s) STORED",
4934+
attdefault);
4935+
}
4936+
49064937
/* Add NOT NULL if needed */
49074938
if (import_not_null && attnotnull[0] == 't')
49084939
appendStringInfoString(&buf, " NOT NULL");

0 commit comments

Comments
 (0)