public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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: Fri, 5 Jun 2026 20:59:17 +0900
Message-ID: <CAPmGK14KEFMTuQ1vYwWCo8SLks5rXv-56K-V+XMy4q8uQJvq1w@mail.gmail.com> (raw)
In-Reply-To: <CAPmGK166P+ngd2ehady=_f-L4MePgBdBNxN5gi5_gSAfmV82QA@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>
	<CAPmGK166P+ngd2ehady=_f-L4MePgBdBNxN5gi5_gSAfmV82QA@mail.gmail.com>

On Mon, Jun 1, 2026 at 7:44 PM Etsuro Fujita <[email protected]> wrote:
> 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 created the patch to add that support on top of the patch I sent in
a previous email, which I'm attaching along with the base patch.  It's
the same as before, except that I fixed a typo in docs pointed out by
Michael-san off-list.

Comments welcome!

Best regards,
Etsuro Fujita


Attachments:

  [application/octet-stream] v3-0001-postgres_fdw-Disallow-UPDATE-DELETE-in-problematic-c.patch (11.6K, 2-v3-0001-postgres_fdw-Disallow-UPDATE-DELETE-in-problematic-c.patch)
  download | inline diff:
From 8a604cfded373265b3c5da0a1d9cda0207997011 Mon Sep 17 00:00:00 2001
From: Etsuro Fujita <[email protected]>
Date: Fri, 5 Jun 2026 20:20:42 +0900
Subject: [PATCH 1/2] postgres_fdw: Disallow UPDATE/DELETE in problematic
 cases.

---
 .../postgres_fdw/expected/postgres_fdw.out    | 64 +++++++++++++++++++
 contrib/postgres_fdw/option.c                 |  2 +
 contrib/postgres_fdw/postgres_fdw.c           | 58 +++++++++++++++++
 contrib/postgres_fdw/sql/postgres_fdw.sql     | 28 ++++++++
 doc/src/sgml/postgres-fdw.sgml                | 23 +++++++
 5 files changed, 175 insertions(+)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index e90289e4ab1..ff9c9e878e4 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 79b16c3f318..340698e3bca 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 ||
@@ -256,6 +257,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 0a589f8db74..3466a8e70b5 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,
@@ -4237,6 +4241,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");
@@ -4309,6 +4316,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);
@@ -4449,6 +4461,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 dfc58beb0d2..31d5ea8a47d 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 b9e1b04463e..e6fa87c4b87 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 improved in future releases.
+      </para>
+     </listitem>
+    </varlistentry>
+
    </variablelist>
   </sect3>
 
-- 
2.50.1 (Apple Git-155)



  [application/octet-stream] v3-0002-postgres_fdw-Add-IMPORT-FOREIGN-SCHEMA-support-for-n.patch (22.1K, 3-v3-0002-postgres_fdw-Add-IMPORT-FOREIGN-SCHEMA-support-for-n.patch)
  download | inline diff:
From d774c5107fd68ff9d6edb4f9f23b79b9cbf71dc8 Mon Sep 17 00:00:00 2001
From: Etsuro Fujita <[email protected]>
Date: Fri, 5 Jun 2026 20:35:19 +0900
Subject: [PATCH 2/2] postgres_fdw: Add IMPORT FOREIGN SCHEMA support for new
 option.

---
 .../postgres_fdw/expected/postgres_fdw.out    | 129 +++++++++-----
 contrib/postgres_fdw/postgres_fdw.c           | 162 ++++++++++++++----
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  17 +-
 3 files changed, 234 insertions(+), 74 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index ff9c9e878e4..a631ea76dc0 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10092,16 +10092,16 @@ CREATE TABLE import_source.t4_part2 PARTITION OF import_source.t4
 CREATE SCHEMA import_dest1;
 IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest1;
 \det+ import_dest1.*
-                                     List of foreign tables
-    Schema    | Table |  Server  |                   FDW options                   | Description 
---------------+-------+----------+-------------------------------------------------+-------------
- import_dest1 | t1    | loopback | (schema_name 'import_source', table_name 't1')  | 
- import_dest1 | t2    | loopback | (schema_name 'import_source', table_name 't2')  | 
- import_dest1 | t3    | loopback | (schema_name 'import_source', table_name 't3')  | 
- import_dest1 | t4    | loopback | (schema_name 'import_source', table_name 't4')  | 
- import_dest1 | x 4   | loopback | (schema_name 'import_source', table_name 'x 4') | 
- import_dest1 | x 5   | loopback | (schema_name 'import_source', table_name 'x 5') | 
- import_dest1 | x 6   | loopback | (schema_name 'import_source', table_name 'x 6') | 
+                                                  List of foreign tables
+    Schema    | Table |  Server  |                                FDW options                                | Description 
+--------------+-------+----------+---------------------------------------------------------------------------+-------------
+ import_dest1 | t1    | loopback | (schema_name 'import_source', table_name 't1')                            | 
+ import_dest1 | t2    | loopback | (schema_name 'import_source', table_name 't2')                            | 
+ import_dest1 | t3    | loopback | (schema_name 'import_source', table_name 't3')                            | 
+ import_dest1 | t4    | loopback | (schema_name 'import_source', table_name 't4', remotely_inherited 'true') | 
+ import_dest1 | x 4   | loopback | (schema_name 'import_source', table_name 'x 4')                           | 
+ import_dest1 | x 5   | loopback | (schema_name 'import_source', table_name 'x 5')                           | 
+ import_dest1 | x 6   | loopback | (schema_name 'import_source', table_name 'x 6')                           | 
 (7 rows)
 
 \d import_dest1.*
@@ -10135,7 +10135,7 @@ FDW options: (schema_name 'import_source', table_name 't3')
 --------+---------+-----------+----------+---------+--------------------
  c1     | integer |           |          |         | (column_name 'c1')
 Server: loopback
-FDW options: (schema_name 'import_source', table_name 't4')
+FDW options: (schema_name 'import_source', table_name 't4', remotely_inherited 'true')
 
                            Foreign table "import_dest1.x 4"
  Column |         Type          | Collation | Nullable | Default |     FDW options     
@@ -10165,16 +10165,16 @@ CREATE SCHEMA import_dest2;
 IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest2
   OPTIONS (import_default 'true');
 \det+ import_dest2.*
-                                     List of foreign tables
-    Schema    | Table |  Server  |                   FDW options                   | Description 
---------------+-------+----------+-------------------------------------------------+-------------
- import_dest2 | t1    | loopback | (schema_name 'import_source', table_name 't1')  | 
- import_dest2 | t2    | loopback | (schema_name 'import_source', table_name 't2')  | 
- import_dest2 | t3    | loopback | (schema_name 'import_source', table_name 't3')  | 
- import_dest2 | t4    | loopback | (schema_name 'import_source', table_name 't4')  | 
- import_dest2 | x 4   | loopback | (schema_name 'import_source', table_name 'x 4') | 
- import_dest2 | x 5   | loopback | (schema_name 'import_source', table_name 'x 5') | 
- import_dest2 | x 6   | loopback | (schema_name 'import_source', table_name 'x 6') | 
+                                                  List of foreign tables
+    Schema    | Table |  Server  |                                FDW options                                | Description 
+--------------+-------+----------+---------------------------------------------------------------------------+-------------
+ import_dest2 | t1    | loopback | (schema_name 'import_source', table_name 't1')                            | 
+ import_dest2 | t2    | loopback | (schema_name 'import_source', table_name 't2')                            | 
+ import_dest2 | t3    | loopback | (schema_name 'import_source', table_name 't3')                            | 
+ import_dest2 | t4    | loopback | (schema_name 'import_source', table_name 't4', remotely_inherited 'true') | 
+ import_dest2 | x 4   | loopback | (schema_name 'import_source', table_name 'x 4')                           | 
+ import_dest2 | x 5   | loopback | (schema_name 'import_source', table_name 'x 5')                           | 
+ import_dest2 | x 6   | loopback | (schema_name 'import_source', table_name 'x 6')                           | 
 (7 rows)
 
 \d import_dest2.*
@@ -10208,7 +10208,7 @@ FDW options: (schema_name 'import_source', table_name 't3')
 --------+---------+-----------+----------+---------+--------------------
  c1     | integer |           |          |         | (column_name 'c1')
 Server: loopback
-FDW options: (schema_name 'import_source', table_name 't4')
+FDW options: (schema_name 'import_source', table_name 't4', remotely_inherited 'true')
 
                            Foreign table "import_dest2.x 4"
  Column |         Type          | Collation | Nullable | Default |     FDW options     
@@ -10237,16 +10237,16 @@ CREATE SCHEMA import_dest3;
 IMPORT FOREIGN SCHEMA import_source FROM SERVER loopback INTO import_dest3
   OPTIONS (import_collate 'false', import_generated 'false', import_not_null 'false');
 \det+ import_dest3.*
-                                     List of foreign tables
-    Schema    | Table |  Server  |                   FDW options                   | Description 
---------------+-------+----------+-------------------------------------------------+-------------
- import_dest3 | t1    | loopback | (schema_name 'import_source', table_name 't1')  | 
- import_dest3 | t2    | loopback | (schema_name 'import_source', table_name 't2')  | 
- import_dest3 | t3    | loopback | (schema_name 'import_source', table_name 't3')  | 
- import_dest3 | t4    | loopback | (schema_name 'import_source', table_name 't4')  | 
- import_dest3 | x 4   | loopback | (schema_name 'import_source', table_name 'x 4') | 
- import_dest3 | x 5   | loopback | (schema_name 'import_source', table_name 'x 5') | 
- import_dest3 | x 6   | loopback | (schema_name 'import_source', table_name 'x 6') | 
+                                                  List of foreign tables
+    Schema    | Table |  Server  |                                FDW options                                | Description 
+--------------+-------+----------+---------------------------------------------------------------------------+-------------
+ import_dest3 | t1    | loopback | (schema_name 'import_source', table_name 't1')                            | 
+ import_dest3 | t2    | loopback | (schema_name 'import_source', table_name 't2')                            | 
+ import_dest3 | t3    | loopback | (schema_name 'import_source', table_name 't3')                            | 
+ import_dest3 | t4    | loopback | (schema_name 'import_source', table_name 't4', remotely_inherited 'true') | 
+ import_dest3 | x 4   | loopback | (schema_name 'import_source', table_name 'x 4')                           | 
+ import_dest3 | x 5   | loopback | (schema_name 'import_source', table_name 'x 5')                           | 
+ import_dest3 | x 6   | loopback | (schema_name 'import_source', table_name 'x 6')                           | 
 (7 rows)
 
 \d import_dest3.*
@@ -10280,7 +10280,7 @@ FDW options: (schema_name 'import_source', table_name 't3')
 --------+---------+-----------+----------+---------+--------------------
  c1     | integer |           |          |         | (column_name 'c1')
 Server: loopback
-FDW options: (schema_name 'import_source', table_name 't4')
+FDW options: (schema_name 'import_source', table_name 't4', remotely_inherited 'true')
 
                            Foreign table "import_dest3.x 4"
  Column |         Type          | Collation | Nullable | Default |     FDW options     
@@ -10320,16 +10320,16 @@ IMPORT FOREIGN SCHEMA import_source LIMIT TO (t1, nonesuch, t4_part)
 IMPORT FOREIGN SCHEMA import_source EXCEPT (t1, "x 4", nonesuch, t4_part)
   FROM SERVER loopback INTO import_dest4;
 \det+ import_dest4.*
-                                        List of foreign tables
-    Schema    |  Table  |  Server  |                     FDW options                     | Description 
---------------+---------+----------+-----------------------------------------------------+-------------
- import_dest4 | t1      | loopback | (schema_name 'import_source', table_name 't1')      | 
- import_dest4 | t2      | loopback | (schema_name 'import_source', table_name 't2')      | 
- import_dest4 | t3      | loopback | (schema_name 'import_source', table_name 't3')      | 
- import_dest4 | t4      | loopback | (schema_name 'import_source', table_name 't4')      | 
- import_dest4 | t4_part | loopback | (schema_name 'import_source', table_name 't4_part') | 
- import_dest4 | x 5     | loopback | (schema_name 'import_source', table_name 'x 5')     | 
- import_dest4 | x 6     | loopback | (schema_name 'import_source', table_name 'x 6')     | 
+                                                   List of foreign tables
+    Schema    |  Table  |  Server  |                                FDW options                                | Description 
+--------------+---------+----------+---------------------------------------------------------------------------+-------------
+ import_dest4 | t1      | loopback | (schema_name 'import_source', table_name 't1')                            | 
+ import_dest4 | t2      | loopback | (schema_name 'import_source', table_name 't2')                            | 
+ import_dest4 | t3      | loopback | (schema_name 'import_source', table_name 't3')                            | 
+ import_dest4 | t4      | loopback | (schema_name 'import_source', table_name 't4', remotely_inherited 'true') | 
+ import_dest4 | t4_part | loopback | (schema_name 'import_source', table_name 't4_part')                       | 
+ import_dest4 | x 5     | loopback | (schema_name 'import_source', table_name 'x 5')                           | 
+ import_dest4 | x 6     | loopback | (schema_name 'import_source', table_name 'x 6')                           | 
 (7 rows)
 
 -- Assorted error cases
@@ -10363,6 +10363,49 @@ QUERY:  CREATE FOREIGN TABLE t5 (
 OPTIONS (schema_name 'import_source', table_name 't5');
 CONTEXT:  importing foreign table "t5"
 ROLLBACK;
+-- Check that the remotely_inherited option is set when needed.
+CREATE TABLE import_source.inhchild (c1 int);
+CREATE TABLE import_source.t6 (c1 int);
+ALTER TABLE import_source.inhchild INHERIT import_source.t6;
+CREATE TABLE import_source.t7 (c1 int);
+ALTER TABLE import_source.inhchild INHERIT import_source.t7;
+ALTER TABLE import_source.inhchild NO INHERIT import_source.t7;
+CREATE FOREIGN TABLE import_source.t8 (c1 int) SERVER loopback
+  OPTIONS (remotely_inherited 'true');
+CREATE SCHEMA import_dest6;
+IMPORT FOREIGN SCHEMA import_source LIMIT TO (t6, t7, t8)
+  FROM SERVER loopback INTO import_dest6;
+\det+ import_dest6.*
+                                                  List of foreign tables
+    Schema    | Table |  Server  |                                FDW options                                | Description 
+--------------+-------+----------+---------------------------------------------------------------------------+-------------
+ import_dest6 | t6    | loopback | (schema_name 'import_source', table_name 't6', remotely_inherited 'true') | 
+ import_dest6 | t7    | loopback | (schema_name 'import_source', table_name 't7')                            | 
+ import_dest6 | t8    | loopback | (schema_name 'import_source', table_name 't8', remotely_inherited 'true') | 
+(3 rows)
+
+\d import_dest6.*
+                    Foreign table "import_dest6.t6"
+ Column |  Type   | Collation | Nullable | Default |    FDW options     
+--------+---------+-----------+----------+---------+--------------------
+ c1     | integer |           |          |         | (column_name 'c1')
+Server: loopback
+FDW options: (schema_name 'import_source', table_name 't6', remotely_inherited 'true')
+
+                    Foreign table "import_dest6.t7"
+ Column |  Type   | Collation | Nullable | Default |    FDW options     
+--------+---------+-----------+----------+---------+--------------------
+ c1     | integer |           |          |         | (column_name 'c1')
+Server: loopback
+FDW options: (schema_name 'import_source', table_name 't7')
+
+                    Foreign table "import_dest6.t8"
+ Column |  Type   | Collation | Nullable | Default |    FDW options     
+--------+---------+-----------+----------+---------+--------------------
+ c1     | integer |           |          |         | (column_name 'c1')
+Server: loopback
+FDW options: (schema_name 'import_source', table_name 't8', remotely_inherited 'true')
+
 BEGIN;
 CREATE SERVER fetch101 FOREIGN DATA WRAPPER postgres_fdw OPTIONS( fetch_size '101' );
 SELECT count(*)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 3466a8e70b5..8a5471b2b05 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -726,6 +726,9 @@ static bool import_fetched_statistics(const char *schemaname,
 static void map_field_to_arg(PGresult *res, int row, int field,
 							 int arg, Datum *values, char *nulls);
 static bool import_spi_query_ok(void);
+static void append_import_schema_restrictions(StringInfo buf,
+											  ImportForeignSchemaStmt *stmt,
+											  PGconn *conn);
 static void produce_tuple_asynchronously(AsyncRequest *areq, bool fetch);
 static void fetch_more_data_begin(AsyncRequest *areq);
 static void complete_pending_request(AsyncRequest *areq);
@@ -6389,7 +6392,10 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 	PGconn	   *conn;
 	StringInfoData buf;
 	PGresult   *res;
-	int			numrows,
+	char	  **inherited = NULL;
+	int			numinherited,
+				inherited_idx,
+				numrows,
 				i;
 	ListCell   *lc;
 
@@ -6444,6 +6450,60 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 	PQclear(res);
 	resetStringInfo(&buf);
 
+	/*
+	 * First, fetch/save all remotely-inherited table names from this schema,
+	 * possibly restricted by EXCEPT or LIMIT TO.
+	 */
+	appendStringInfoString(&buf,
+						   "SELECT relname "
+						   "FROM pg_class c "
+						   "  JOIN pg_namespace n ON "
+						   "    relnamespace = n.oid "
+						   "  LEFT JOIN (pg_foreign_table t "
+						   "    JOIN pg_foreign_server s ON "
+						   "      s.oid = t.ftserver "
+						   "    JOIN pg_foreign_data_wrapper w ON "
+						   "      w.oid = s.srvfdw) ON "
+						   "    t.ftrelid = c.oid "
+						   "WHERE (c.relkind = "
+						   CppAsString2(RELKIND_PARTITIONED_TABLE) " "
+						   "  OR (c.relkind IN ("
+						   CppAsString2(RELKIND_RELATION) ","
+						   CppAsString2(RELKIND_FOREIGN_TABLE) ") "
+						   "    AND c.relhassubclass "
+						   "    AND EXISTS (SELECT 1 FROM pg_inherits "
+						   "      WHERE inhparent = c.oid)) "
+						   "  OR (c.relkind = "
+						   CppAsString2(RELKIND_FOREIGN_TABLE) " "
+						   "    AND w.fdwname = \'postgres_fdw\' "
+						   "    AND t.ftoptions @> "
+						   "      ARRAY[\'remotely_inherited=true\'])) "
+						   "  AND n.nspname = ");
+	deparseStringLiteral(&buf, stmt->remote_schema);
+
+	/* Append EXCEPT/LIMIT TO restrictions */
+	append_import_schema_restrictions(&buf, stmt, conn);
+
+	/* Append ORDER BY at the end of query to ensure output ordering */
+	appendStringInfoString(&buf, " ORDER BY c.relname");
+
+	/* Fetch the data */
+	res = pgfdw_exec_query(conn, buf.data, NULL);
+	if (PQresultStatus(res) != PGRES_TUPLES_OK)
+		pgfdw_report_error(res, conn, buf.data);
+
+	/* Save the data */
+	numinherited = PQntuples(res);
+	if (numinherited > 0)
+	{
+		inherited = (char **) palloc0(numinherited * sizeof(char *));
+		for (i = 0; i < numinherited; i++)
+			inherited[i] = pstrdup(PQgetvalue(res, i, 0));
+	}
+
+	PQclear(res);
+	resetStringInfo(&buf);
+
 	/*
 	 * Fetch all table data from this schema, possibly restricted by EXCEPT or
 	 * LIMIT TO.  (We don't actually need to pay any attention to EXCEPT/LIMIT
@@ -6511,35 +6571,8 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 						   "  AND n.nspname = ");
 	deparseStringLiteral(&buf, stmt->remote_schema);
 
-	/* Partitions are supported since Postgres 10 */
-	if (PQserverVersion(conn) >= 100000 &&
-		stmt->list_type != FDW_IMPORT_SCHEMA_LIMIT_TO)
-		appendStringInfoString(&buf, " AND NOT c.relispartition ");
-
-	/* Apply restrictions for LIMIT TO and EXCEPT */
-	if (stmt->list_type == FDW_IMPORT_SCHEMA_LIMIT_TO ||
-		stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
-	{
-		bool		first_item = true;
-
-		appendStringInfoString(&buf, " AND c.relname ");
-		if (stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
-			appendStringInfoString(&buf, "NOT ");
-		appendStringInfoString(&buf, "IN (");
-
-		/* Append list of table names within IN clause */
-		foreach(lc, stmt->table_list)
-		{
-			RangeVar   *rv = (RangeVar *) lfirst(lc);
-
-			if (first_item)
-				first_item = false;
-			else
-				appendStringInfoString(&buf, ", ");
-			deparseStringLiteral(&buf, rv->relname);
-		}
-		appendStringInfoChar(&buf, ')');
-	}
+	/* Append EXCEPT/LIMIT TO restrictions */
+	append_import_schema_restrictions(&buf, stmt, conn);
 
 	/* Append ORDER BY at the end of query to ensure output ordering */
 	appendStringInfoString(&buf, " ORDER BY c.relname, a.attnum");
@@ -6551,6 +6584,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 
 	/* Process results */
 	numrows = PQntuples(res);
+	inherited_idx = 0;
 	/* note: incrementation of i happens in inner loop's while() test */
 	for (i = 0; i < numrows;)
 	{
@@ -6647,17 +6681,85 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 		appendStringInfoString(&buf, ", table_name ");
 		deparseStringLiteral(&buf, tablename);
 
+		/*
+		 * Also add the remotely_inherited option if needed, to prevent unsafe
+		 * modifications to the foreign table (see check_result_rel()).
+		 *
+		 * By the definitions of the fetch queries using the same snapshot on
+		 * the remote server, the inherited is guaranteed to be a strictly
+		 * proper subset of the data processed here with the same order as it,
+		 * so we determine whether the foreign table is remotely-inherited or
+		 * not, by doing a merge join to it.
+		 */
+		if (numinherited > 0 && inherited_idx < numinherited &&
+			strcmp(tablename, inherited[inherited_idx]) == 0)
+		{
+			appendStringInfoString(&buf, ", remotely_inherited \'true\'");
+			inherited_idx++;
+		}
+
 		appendStringInfoString(&buf, ");");
 
 		commands = lappend(commands, pstrdup(buf.data));
 	}
 	PQclear(res);
 
+	if (numinherited > 0)
+	{
+		Assert(inherited != NULL);
+		for (i = 0; i < numinherited; i++)
+		{
+			Assert(inherited[i] != NULL);
+			pfree(inherited[i]);
+		}
+		pfree(inherited);
+	}
+
 	ReleaseConnection(conn);
 
 	return commands;
 }
 
+/*
+ * Append EXCEPT/LIMIT TO restrictions to a query.
+ */
+static void
+append_import_schema_restrictions(StringInfo buf,
+								  ImportForeignSchemaStmt *stmt,
+								  PGconn *conn)
+{
+	/* Partitions are supported since Postgres 10 */
+	if (PQserverVersion(conn) >= 100000 &&
+		stmt->list_type != FDW_IMPORT_SCHEMA_LIMIT_TO)
+		appendStringInfoString(buf, " AND NOT c.relispartition ");
+
+	/* Apply restrictions for LIMIT TO and EXCEPT */
+	if (stmt->list_type == FDW_IMPORT_SCHEMA_LIMIT_TO ||
+		stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
+	{
+		bool		first_item = true;
+		ListCell   *lc;
+
+		appendStringInfoString(buf, " AND c.relname ");
+		if (stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
+			appendStringInfoString(buf, "NOT ");
+		appendStringInfoString(buf, "IN (");
+
+		/* Append list of table names within IN clause */
+		foreach(lc, stmt->table_list)
+		{
+			RangeVar   *rv = (RangeVar *) lfirst(lc);
+
+			if (first_item)
+				first_item = false;
+			else
+				appendStringInfoString(buf, ", ");
+			deparseStringLiteral(buf, rv->relname);
+		}
+		appendStringInfoChar(buf, ')');
+	}
+}
+
 /*
  * Check if reltarget is safe enough to push down semi-join.  Reltarget is not
  * safe, if it contains references to inner rel relids, which do not belong to
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 31d5ea8a47d..12b7ad2235b 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3275,8 +3275,23 @@ IMPORT FOREIGN SCHEMA import_source LIMIT TO (t5)
 
 ROLLBACK;
 
-BEGIN;
+-- Check that the remotely_inherited option is set when needed.
+CREATE TABLE import_source.inhchild (c1 int);
+CREATE TABLE import_source.t6 (c1 int);
+ALTER TABLE import_source.inhchild INHERIT import_source.t6;
+CREATE TABLE import_source.t7 (c1 int);
+ALTER TABLE import_source.inhchild INHERIT import_source.t7;
+ALTER TABLE import_source.inhchild NO INHERIT import_source.t7;
+CREATE FOREIGN TABLE import_source.t8 (c1 int) SERVER loopback
+  OPTIONS (remotely_inherited 'true');
+
+CREATE SCHEMA import_dest6;
+IMPORT FOREIGN SCHEMA import_source LIMIT TO (t6, t7, t8)
+  FROM SERVER loopback INTO import_dest6;
+\det+ import_dest6.*
+\d import_dest6.*
 
+BEGIN;
 
 CREATE SERVER fetch101 FOREIGN DATA WRAPPER postgres_fdw OPTIONS( fetch_size '101' );
 
-- 
2.50.1 (Apple Git-155)



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: <CAPmGK14KEFMTuQ1vYwWCo8SLks5rXv-56K-V+XMy4q8uQJvq1w@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