From 430d0b104d6b5f3fa49a149442f1f331da280fca Mon Sep 17 00:00:00 2001
From: Shirisha SN <sshirisha@vmware.com>
Date: Fri, 6 Jun 2025 22:27:40 +0530
Subject: [PATCH v1 1/1] Allow DELETE/UPDATE on partitioned tables with foreign
 partitions

Currently, DELETE or UPDATE on a partitioned table with foreign partitions
fail with an error as below, if the FDW does not support the operation:

	`ERROR: cannot delete from foreign table`

This occurs because during executor initialization (ExecInitModifyTable),
PostgreSQL scans all partitions of the target table and checks whether each one
supports the requested operation. If any foreign partition's FDW lacks support
for DELETE or UPDATE, the operation is rejected outright, even if that
partition would not be affected by the query.

As a result, DELETE/UPDATE operations are blocked even when they only target
non-foreign partitions. This means the system errors out without considering
whether foreign partitions are actually involved in the operation. Even if no
matching rows exist in a foreign partition, the operation still fails.

This commit defers the FDW check for foreign partitions from
`ExecInitModifyTable` to `ExecDelete` and `ExecUpdate`. This change ensures
that foreign partitions are checked only when they are actually targeted by the
operation.

However, if a DELETE or UPDATE is issued on the root partition and it includes
foreign partitions that do not support the operation, it will still result in
an error. This is intentional because the responsibility for managing data in
foreign partitions lies with the user. Only after the user has removed relevant
data from those foreign partitions will such operations on the root partition
succeed.

** Mini repro: **
```
CREATE EXTENSION file_fdw;
CREATE SERVER file_server FOREIGN DATA WRAPPER file_fdw;
CREATE TABLE pt (a int, b numeric) PARTITION BY RANGE(a);
CREATE TABLE pt_part1 PARTITION OF pt FOR VALUES FROM (0) TO (10);
INSERT INTO pt SELECT 5, 0.1;
INSERT INTO pt SELECT 6, 0.2;

CREATE FOREIGN TABLE ext (a int, b numeric) SERVER file_server OPTIONS (filename '/Users/sshirisha/workspace/postgres/src/test/regress/data/test_data_float.csv', format 'csv', delimiter ',');
ALTER TABLE pt ATTACH PARTITION ext FOR VALUES FROM (10) TO (20);
postgres=# SELECT * FROM pt;
 a  |  b
----+-----
  5 | 0.1
  6 | 0.2
 15 | 0.3
 21 | 0.4
(4 rows)
```

** Before Fix: **
```
postgres=# DELETE FROM pt WHERE b = 0.2;
ERROR:  cannot delete from foreign table "ext"
postgres=# DELETE FROM pt;
ERROR:  cannot delete from foreign table "ext"

postgres=# UPDATE pt set b = 0.5 WHERE b = 0.1;
ERROR:  cannot update foreign table "ext"
postgres=# UPDATE pt SET b = 0.5;
ERROR:  cannot update foreign table "ext"
```

** After Fix: **
```
postgres=# DELETE FROM pt WHERE b = 0.2;
DELETE 1
postgres=# DELETE FROM pt;
ERROR:  cannot delete from foreign table "ext"

postgres=# UPDATE pt SET b = 0.5 WHERE b = 0.1;
UPDATE 1
postgres=# UPDATE pt SET b = 0.5;
ERROR:  cannot update foreign table "ext"
```

Co-authored-by: Ashwin Agrawal <ashwin.agrawal@broadcom.com>
---
 src/backend/executor/nodeModifyTable.c       | 66 +++++++++++++++++++-
 src/test/regress/expected/partition_info.out | 58 +++++++++++++++++
 src/test/regress/sql/partition_info.sql      | 28 +++++++++
 3 files changed, 150 insertions(+), 2 deletions(-)

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 54da8e7995b..2a696cb0615 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1584,10 +1584,34 @@ ExecDelete(ModifyTableContext *context,
 	TupleTableSlot *slot = NULL;
 	TM_Result	result;
 	bool		saveOld;
+	FdwRoutine *fdwroutine;
 
 	if (tupleDeleted)
 		*tupleDeleted = false;
 
+	/*
+	 * For foreign partitions, raise error during DELETE if the FDW does not support it.
+	 * This check is deferred from ExecInitModifyTable to allow deletes
+	 * targeted on non-foreign partitions to proceed without error.
+	 */
+	if (resultRelationDesc->rd_rel->relkind == RELKIND_FOREIGN_TABLE &&
+			resultRelationDesc->rd_rel->relispartition)
+	{
+		fdwroutine = resultRelInfo->ri_FdwRoutine;
+		if (fdwroutine->ExecForeignDelete == NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("cannot delete from foreign table \"%s\"",
+							RelationGetRelationName(resultRelationDesc))));
+
+		if (fdwroutine->IsForeignRelUpdatable != NULL &&
+			(fdwroutine->IsForeignRelUpdatable(resultRelationDesc) & (1 << CMD_DELETE)) == 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					errmsg("foreign table \"%s\" does not allow deletes",
+							RelationGetRelationName(resultRelationDesc))));
+	}
+
 	/*
 	 * Prepare for the delete.  This includes BEFORE ROW triggers, so we're
 	 * done if it says we are.
@@ -2464,6 +2488,7 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 	Relation	resultRelationDesc = resultRelInfo->ri_RelationDesc;
 	UpdateContext updateCxt = {0};
 	TM_Result	result;
+	FdwRoutine *fdwroutine;
 
 	/*
 	 * abort the operation if not running transactions
@@ -2478,6 +2503,29 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 	if (!ExecUpdatePrologue(context, resultRelInfo, tupleid, oldtuple, slot, NULL))
 		return NULL;
 
+	/*
+	 * For foreign partitions, raise error during UPDATE if the FDW does not support it.
+	 * This check is deferred from ExecInitModifyTable to allow updates
+	 * targeted on non-foreign partitions to proceed without error.
+	 */
+	if (resultRelationDesc->rd_rel->relkind == RELKIND_FOREIGN_TABLE &&
+		resultRelationDesc->rd_rel->relispartition)
+	{
+		fdwroutine = resultRelInfo->ri_FdwRoutine;
+		if (fdwroutine->ExecForeignUpdate == NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("cannot update foreign table \"%s\"",
+							RelationGetRelationName(resultRelationDesc))));
+
+		if (fdwroutine->IsForeignRelUpdatable != NULL &&
+			(fdwroutine->IsForeignRelUpdatable(resultRelationDesc) & (1 << CMD_UPDATE)) == 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					errmsg("foreign table \"%s\" does not allow updates",
+							RelationGetRelationName(resultRelationDesc))));
+	}
+
 	/* INSTEAD OF ROW UPDATE Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_update_instead_row)
@@ -4783,6 +4831,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 	i = 0;
 	foreach(l, resultRelations)
 	{
+		bool 		skip_rel_check = false;
 		Index		resultRelation = lfirst_int(l);
 		List	   *mergeActions = NIL;
 
@@ -4807,9 +4856,22 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 			bms_is_member(i, node->fdwDirectModifyPlans);
 
 		/*
-		 * Verify result relation is a valid target for the current operation
+		 * Verify result relation is a valid target for the current operation.
+		 * Skip this verification only when:
+		 * - the relation is a foreign partition,
+		 * - the operation is DELETE or UPDATE, and
+		 * - the query involves multiple result relations
+		 *
+		 * In such cases, the validation is deferred to ExecDelete or
+		 * ExecUpdate, where the specific foreign partition is processed.
 		 */
-		CheckValidResultRel(resultRelInfo, operation, mergeActions);
+		rel = resultRelInfo->ri_RelationDesc;
+		skip_rel_check = (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE &&
+							rel->rd_rel->relispartition &&
+							(operation == CMD_DELETE || operation == CMD_UPDATE) &&
+							nrels > 1);
+		if (!skip_rel_check)
+			CheckValidResultRel(resultRelInfo, operation, mergeActions);
 
 		resultRelInfo++;
 		i++;
diff --git a/src/test/regress/expected/partition_info.out b/src/test/regress/expected/partition_info.out
index 42b6bc77cad..b7095744d77 100644
--- a/src/test/regress/expected/partition_info.out
+++ b/src/test/regress/expected/partition_info.out
@@ -349,3 +349,61 @@ SELECT pg_partition_root('ptif_li_child');
 DROP VIEW ptif_test_view;
 DROP MATERIALIZED VIEW ptif_test_matview;
 DROP TABLE ptif_li_parent, ptif_li_child;
+-- Test UPDATE/DELETE on partition table with foreign partitions
+\getenv abs_srcdir PG_ABS_SRCDIR
+CREATE EXTENSION file_fdw;
+CREATE SERVER file_server FOREIGN DATA WRAPPER file_fdw;
+CREATE TABLE ptif_root (a int2, b numeric) PARTITION BY range (a);
+CREATE TABLE ptif_child PARTITION OF ptif_root FOR VALUES FROM (0) TO (10);
+INSERT INTO ptif_root SELECT 5, 0.1;
+INSERT INTO ptif_root SELECT 6, 0.2;
+\set filename :abs_srcdir '/data/agg.csv'
+CREATE FOREIGN TABLE agg_csv (
+        a int2,
+        b numeric
+) SERVER file_server
+OPTIONS (format 'csv', filename :'filename', header 'false', delimiter ';', quote '@', escape '"', null '');
+ALTER TABLE ptif_root ATTACH PARTITION agg_csv FOR VALUES FROM (10) TO (20);
+SELECT * FROM ptif_root;
+  a  |    b    
+-----+---------
+   5 |     0.1
+   6 |     0.2
+  56 |     7.8
+ 100 |  99.097
+   0 | 0.09561
+  42 |  324.78
+(6 rows)
+
+DELETE FROM ptif_root WHERE b = 0.1;
+DELETE FROM ptif_root;
+ERROR:  cannot delete from foreign table "agg_csv"
+SELECT * FROM ptif_root;
+  a  |    b    
+-----+---------
+   6 |     0.2
+  56 |     7.8
+ 100 |  99.097
+   0 | 0.09561
+  42 |  324.78
+(5 rows)
+
+UPDATE ptif_root SET b = 0.6 WHERE b = 0.2;
+UPDATE ptif_root SET b = 0.10;
+ERROR:  cannot update foreign table "agg_csv"
+SELECT * FROM ptif_root;
+  a  |    b    
+-----+---------
+   6 |     0.6
+  56 |     7.8
+ 100 |  99.097
+   0 | 0.09561
+  42 |  324.78
+(5 rows)
+
+-- cleanup
+DROP EXTENSION file_fdw CASCADE;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to server file_server
+drop cascades to foreign table agg_csv
+DROP TABLE ptif_root;
diff --git a/src/test/regress/sql/partition_info.sql b/src/test/regress/sql/partition_info.sql
index b5060bec7f0..e57fe821c41 100644
--- a/src/test/regress/sql/partition_info.sql
+++ b/src/test/regress/sql/partition_info.sql
@@ -127,3 +127,31 @@ SELECT pg_partition_root('ptif_li_child');
 DROP VIEW ptif_test_view;
 DROP MATERIALIZED VIEW ptif_test_matview;
 DROP TABLE ptif_li_parent, ptif_li_child;
+
+-- Test UPDATE/DELETE on partition table with foreign partitions
+\getenv abs_srcdir PG_ABS_SRCDIR
+CREATE EXTENSION file_fdw;
+CREATE SERVER file_server FOREIGN DATA WRAPPER file_fdw;
+
+CREATE TABLE ptif_root (a int2, b numeric) PARTITION BY range (a);
+CREATE TABLE ptif_child PARTITION OF ptif_root FOR VALUES FROM (0) TO (10);
+INSERT INTO ptif_root SELECT 5, 0.1;
+INSERT INTO ptif_root SELECT 6, 0.2;
+\set filename :abs_srcdir '/data/agg.csv'
+CREATE FOREIGN TABLE agg_csv (
+        a int2,
+        b numeric
+) SERVER file_server
+OPTIONS (format 'csv', filename :'filename', header 'false', delimiter ';', quote '@', escape '"', null '');
+ALTER TABLE ptif_root ATTACH PARTITION agg_csv FOR VALUES FROM (10) TO (20);
+SELECT * FROM ptif_root;
+DELETE FROM ptif_root WHERE b = 0.1;
+DELETE FROM ptif_root;
+SELECT * FROM ptif_root;
+UPDATE ptif_root SET b = 0.6 WHERE b = 0.2;
+UPDATE ptif_root SET b = 0.10;
+SELECT * FROM ptif_root;
+
+-- cleanup
+DROP EXTENSION file_fdw CASCADE;
+DROP TABLE ptif_root;
-- 
2.39.5 (Apple Git-154)

