diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index aaffcf31271..ef1846c20cc 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7197,6 +7197,70 @@ RESET enable_material; DROP FOREIGN TABLE remt2; DROP TABLE loct1; DROP TABLE loct2; +-- Test UPDATE/DELETE on remotely-inherited foreign tables +CREATE TABLE ritest_pt (a int, b int) PARTITION BY LIST(a); +CREATE TABLE ritest_pt_p1 PARTITION OF ritest_pt FOR VALUES IN (1); +CREATE TABLE ritest_pt_p2 PARTITION OF ritest_pt FOR VALUES IN (2); +CREATE FOREIGN TABLE ritest_ft (a int, b int) SERVER loopback OPTIONS (table_name 'ritest_pt', remotely_inherited 'true'); +INSERT INTO ritest_ft VALUES (1, 10), (2, 20); +-- Use random() so that UPDATE/DELETE is not pushed down to the remote +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE ritest_ft SET b = 100 WHERE a = 1 AND random() < 1.0; + QUERY PLAN +---------------------------------------------------------------------------------------- + Update on public.ritest_ft + Remote SQL: UPDATE public.ritest_pt SET b = $2 WHERE ctid = $1 + -> Foreign Scan on public.ritest_ft + Output: 100, ctid, ritest_ft.* + Filter: (random() < '1'::double precision) + Remote SQL: SELECT a, b, ctid FROM public.ritest_pt WHERE ((a = 1)) FOR UPDATE +(6 rows) + +UPDATE ritest_ft SET b = 100 WHERE a = 1 AND random() < 1.0; -- should fail +ERROR: cannot update remotely-inherited foreign table "ritest_ft" +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE ritest_ft SET b = 300 WHERE b = 30 AND random() < 1.0; + QUERY PLAN +----------------------------------------------------------------------------------------- + Update on public.ritest_ft + Remote SQL: UPDATE public.ritest_pt SET b = $2 WHERE ctid = $1 + -> Foreign Scan on public.ritest_ft + Output: 300, ctid, ritest_ft.* + Filter: (random() < '1'::double precision) + Remote SQL: SELECT a, b, ctid FROM public.ritest_pt WHERE ((b = 30)) FOR UPDATE +(6 rows) + +UPDATE ritest_ft SET b = 300 WHERE b = 30 AND random() < 1.0; -- should work +EXPLAIN (VERBOSE, COSTS OFF) +DELETE FROM ritest_ft WHERE a = 1 AND random() < 1.0; + QUERY PLAN +---------------------------------------------------------------------------------- + Delete on public.ritest_ft + Remote SQL: DELETE FROM public.ritest_pt WHERE ctid = $1 + -> Foreign Scan on public.ritest_ft + Output: ctid + Filter: (random() < '1'::double precision) + Remote SQL: SELECT ctid FROM public.ritest_pt WHERE ((a = 1)) FOR UPDATE +(6 rows) + +DELETE FROM ritest_ft WHERE a = 1 AND random() < 1.0; -- should fail +ERROR: cannot delete from remotely-inherited foreign table "ritest_ft" +EXPLAIN (VERBOSE, COSTS OFF) +DELETE FROM ritest_ft WHERE b = 30 AND random() < 1.0; + QUERY PLAN +----------------------------------------------------------------------------------- + Delete on public.ritest_ft + Remote SQL: DELETE FROM public.ritest_pt WHERE ctid = $1 + -> Foreign Scan on public.ritest_ft + Output: ctid + Filter: (random() < '1'::double precision) + Remote SQL: SELECT ctid FROM public.ritest_pt WHERE ((b = 30)) FOR UPDATE +(6 rows) + +DELETE FROM ritest_ft WHERE b = 30 AND random() < 1.0; -- should work +-- Cleanup +DROP FOREIGN TABLE ritest_ft; +DROP TABLE ritest_pt; -- =================================================================== -- test check constraints -- =================================================================== diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index 3944aedbacc..2341d9e5b69 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -116,6 +116,7 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) */ if (strcmp(def->defname, "use_remote_estimate") == 0 || strcmp(def->defname, "updatable") == 0 || + strcmp(def->defname, "remotely_inherited") == 0 || strcmp(def->defname, "truncatable") == 0 || strcmp(def->defname, "async_capable") == 0 || strcmp(def->defname, "parallel_commit") == 0 || @@ -255,6 +256,7 @@ InitPgFdwOptions(void) /* updatable is available on both server and table */ {"updatable", ForeignServerRelationId, false}, {"updatable", ForeignTableRelationId, false}, + {"remotely_inherited", ForeignTableRelationId, false}, /* truncatable is available on both server and table */ {"truncatable", ForeignServerRelationId, false}, {"truncatable", ForeignTableRelationId, false}, diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index c42cb690c7b..52b3da4efbf 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -207,6 +207,9 @@ typedef struct PgFdwModifyState int p_nums; /* number of parameters to transmit */ FmgrInfo *p_flinfo; /* output conversion functions for them */ + /* update/delete operation stuff */ + bool resultRelValid; /* have we checked the result relation? */ + /* batch operation stuff */ int num_slots; /* number of slots to insert */ @@ -654,6 +657,7 @@ static TupleTableSlot **execute_foreign_modify(EState *estate, TupleTableSlot **planSlots, int *numSlots); static void prepare_foreign_modify(PgFdwModifyState *fmstate); +static void check_result_rel(PgFdwModifyState *fmstate, CmdType operation); static const char **convert_prep_stmt_params(PgFdwModifyState *fmstate, ItemPointer tupleid, TupleTableSlot **slots, @@ -4236,6 +4240,9 @@ create_foreign_modify(EState *estate, { Assert(subplan != NULL); + /* Initialize valid flag for the result relation */ + fmstate->resultRelValid = false; + /* Find the ctid resjunk column in the subplan's result */ fmstate->ctidAttno = ExecFindJunkAttributeInTlist(subplan->targetlist, "ctid"); @@ -4308,6 +4315,11 @@ execute_foreign_modify(EState *estate, operation == CMD_UPDATE || operation == CMD_DELETE); + /* For UPDATE/DELETE, check the result relation if not yet done. */ + if ((operation == CMD_UPDATE || operation == CMD_DELETE) && + !fmstate->resultRelValid) + check_result_rel(fmstate, operation); + /* First, process a pending asynchronous request, if any. */ if (fmstate->conn_state->pendingAreq) process_pending_request(fmstate->conn_state->pendingAreq); @@ -4448,6 +4460,52 @@ prepare_foreign_modify(PgFdwModifyState *fmstate) fmstate->p_name = p_name; } +/* + * check_result_rel + * Check if the target foreign table is safe to update/delete via + * ExecForeignUpdate/ExecForeignDelete. + */ +static void +check_result_rel(PgFdwModifyState *fmstate, CmdType operation) +{ + bool remotely_inherited; + ForeignTable *table; + ListCell *lc; + + Assert(!fmstate->resultRelValid); + Assert(operation == CMD_UPDATE || operation == CMD_DELETE); + + /* + * By default, any postgres_fdw foreign table isn't assumed + * remotely-inherited. + */ + remotely_inherited = false; + + table = GetForeignTable(RelationGetRelid(fmstate->rel)); + + foreach(lc, table->options) + { + DefElem *def = (DefElem *) lfirst(lc); + + if (strcmp(def->defname, "remotely_inherited") == 0) + remotely_inherited = defGetBoolean(def); + } + + /* + * It's unsafe to update/delete remotely-inherited foreign tables via + * ExecForeignUpdate/ExecForeignDelete. + */ + if (remotely_inherited) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg((operation == CMD_UPDATE) ? + "cannot update remotely-inherited foreign table \"%s\"" : + "cannot delete from remotely-inherited foreign table \"%s\"", + RelationGetRelationName(fmstate->rel)))); + + fmstate->resultRelValid = true; +} + /* * convert_prep_stmt_params * Create array of text strings representing parameter values diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 267d3c1a7e7..7fc099bccb7 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1778,6 +1778,34 @@ DROP FOREIGN TABLE remt2; DROP TABLE loct1; DROP TABLE loct2; +-- Test UPDATE/DELETE on remotely-inherited foreign tables +CREATE TABLE ritest_pt (a int, b int) PARTITION BY LIST(a); +CREATE TABLE ritest_pt_p1 PARTITION OF ritest_pt FOR VALUES IN (1); +CREATE TABLE ritest_pt_p2 PARTITION OF ritest_pt FOR VALUES IN (2); +CREATE FOREIGN TABLE ritest_ft (a int, b int) SERVER loopback OPTIONS (table_name 'ritest_pt', remotely_inherited 'true'); +INSERT INTO ritest_ft VALUES (1, 10), (2, 20); + +-- Use random() so that UPDATE/DELETE is not pushed down to the remote +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE ritest_ft SET b = 100 WHERE a = 1 AND random() < 1.0; +UPDATE ritest_ft SET b = 100 WHERE a = 1 AND random() < 1.0; -- should fail + +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE ritest_ft SET b = 300 WHERE b = 30 AND random() < 1.0; +UPDATE ritest_ft SET b = 300 WHERE b = 30 AND random() < 1.0; -- should work + +EXPLAIN (VERBOSE, COSTS OFF) +DELETE FROM ritest_ft WHERE a = 1 AND random() < 1.0; +DELETE FROM ritest_ft WHERE a = 1 AND random() < 1.0; -- should fail + +EXPLAIN (VERBOSE, COSTS OFF) +DELETE FROM ritest_ft WHERE b = 30 AND random() < 1.0; +DELETE FROM ritest_ft WHERE b = 30 AND random() < 1.0; -- should work + +-- Cleanup +DROP FOREIGN TABLE ritest_ft; +DROP TABLE ritest_pt; + -- =================================================================== -- test check constraints -- =================================================================== diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index b81f33732fb..f28da502923 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -630,6 +630,29 @@ OPTIONS (ADD password_required 'false'); + + remotely_inherited (boolean) + + + This option, which can be specified for a foreign table, determines if + the remote table is an inherited/partitioned table on the remote server + or a foreign table on it referencing such a table on another remote + server. + The default is false. + + + + If the updatable option is set for a foreign table + whose remote table is any of the above, + postgres_fdw currently cannot properly + update/delete it, causing unexpected results, except in cases where the + whole UPDATE/DELETE processing is pushed down to the + remote server. Such unsafe modifications can be prevented by setting + this option. This might be imporved in future releases. + + + +