public inbox for [email protected]
help / color / mirror / Atom feedFrom: Etsuro Fujita <[email protected]>
To: Nikita Malakhov <[email protected]>
Cc: Jehan-Guillaume de Rorthais <[email protected]>
Cc: [email protected]
Subject: Re: [(known) BUG] DELETE/UPDATE more than one row in partitioned foreign table
Date: Mon, 1 Jun 2026 19:44:34 +0900
Message-ID: <CAPmGK166P+ngd2ehady=_f-L4MePgBdBNxN5gi5_gSAfmV82QA@mail.gmail.com> (raw)
In-Reply-To: <CAN-LCVPgq0zfOU+BLrHnr2Sex_zndNjzWoAiONWD=R4ULQ2BAA@mail.gmail.com>
References: <20250718175314.4513c00a@karst>
<CAPmGK15CQK-oYFMAyq+rR0rQapUHtvAGuGgY5ahERHzZ4tmC8g@mail.gmail.com>
<20250729174852.14f23557@karst>
<CAPmGK16v_We-k30qQaP+AARTr3n_dRg6yFuHP39sjV5uE_ne0Q@mail.gmail.com>
<CAN-LCVMz58ukZ7ubGXiLuTeFE7wWmSwDw4URpF0q1ejzRvqbzg@mail.gmail.com>
<CAN-LCVM2iOWkfFt22yVEGOrp-76YP3-BVKQg+A20TENkVh8o1w@mail.gmail.com>
<CAN-LCVPgq0zfOU+BLrHnr2Sex_zndNjzWoAiONWD=R4ULQ2BAA@mail.gmail.com>
Hi Nikita,
On Fri, May 15, 2026 at 2:23 AM Nikita Malakhov <[email protected]> wrote:
> CFbot was unhappy with previous patch set, so here's updated one
Thanks for working on this issue!
I took a quick look at the patch set. IIUC I think it's created based
on what I proposed in the original thread, which is invasive and thus
not back-patchable, so what you are proposing here isn't
back-patchable, either, I think.
I think we should first work on a back-patchable solution. So I'd
like to re-propose the patch that I proposed in this thread before to
disallow UPDATE/DELETE in problematic cases [1]. Attached is a new
version of the patch. Changes are:
* Renamed the new table option inherited to remotely_inherited, to
avoid confusion with local inheritance.
* Moved the logic to prevent problematic UPDATE/DELETE from a planner
function to an executor function, to avoid throwing an error
unnecessarily when there are no target rows to update/delete.
* Added docs to postgres-fdw.sgml.
I'm planning to add the postgresImportForeignSchema() support in the
next version.
I think the remotely_inherited option would be useful when adding the
support for the UPDATE/DELETE, as it could be used to address one of
Tom Lane's comments about what I proposed in the original thread that
it adds the tabloid condition to a remote UPDATE/DELETE query whether
the target table is inherited or not: that could be avoid if the
option is set to false.
What do you think about that?
Best regards,
Etsuro Fujita
[1] https://www.postgresql.org/message-id/CAPmGK15CQK-oYFMAyq%2BrR0rQapUHtvAGuGgY5ahERHzZ4tmC8g%40mail.g...
Attachments:
[application/octet-stream] postgres_fdw-disallow-upddel-in-problematic-cases-v2.patch (11.0K, 2-postgres_fdw-disallow-upddel-in-problematic-cases-v2.patch)
download | inline diff:
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');
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>remotely_inherited</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ 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 <literal>false</literal>.
+ </para>
+
+ <para>
+ If the <literal>updatable</literal> option is set for a foreign table
+ whose remote table is any of the above,
+ <filename>postgres_fdw</filename> currently cannot properly
+ update/delete it, causing unexpected results, except in cases where the
+ whole <command>UPDATE/DELETE</command> 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect3>
view thread (11+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: [(known) BUG] DELETE/UPDATE more than one row in partitioned foreign table
In-Reply-To: <CAPmGK166P+ngd2ehady=_f-L4MePgBdBNxN5gi5_gSAfmV82QA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox