Update of partition key on foreign server

Started by Илья Гладышевover 4 years ago3 messages
#1Илья Гладышев
i.gladyshev@postgrespro.ru

Hi,

I am currently looking into a partition constraint violation that occurs on update of a partition key on a foreign server. To reproduce it you can run:

On server 1 using port 5432:

create extension postgres_fdw;
create table players (id integer, name text) partition by list(name);
create table players_0 partition of players for values in ('name1');
create server neighbor foreign data wrapper postgres_fdw options (host 'localhost', port '5433', dbname 'postgres');
create foreign table players_1 partition of players for values in ('name2') server neighbor ;
create user mapping for current_user server neighbor;
On server 2 using port 5433:

create extension postgres_fdw;
create server neighbor foreign data wrapper postgres_fdw options (host 'localhost', port '5432', dbname 'postgres');
create table players (id integer, name text) partition by list(name);
create table players_1 partition of players for values in ('name2');
create user mapping for current_user server neighbor;
create foreign table players_0 partition of players for values in ('name1') server neighbor;

insert into players values (1, 'name1');
update players set name='name2' where name='name1';
ERROR: new row for relation "players_0" violates partition constraint
DETAIL: Failing row contains (1, name2).
CONTEXT: remote SQL command: UPDATE public.players_0 SET name = 'name2'::text WHERE ((name = 'name1'::text))

From what I have read on the mailing list, I understand that this is a known problem, but I haven't found any thread discussing it in particular. Is this something that needs fixing? If it is, I want to try to do it, but I’m wondering if there are any known caveats and looking for any tips on how to implement it.

My understanding is that this should be implemented in a similar way to how the row is routed from a local partition in ExecCrossPartitionUpdate, so the update should be replaced with a foreign delete + local/foreign insert. In addition, a direct update should be forbidden when the query modifies the partition key. I’m probably missing a lot of details (feel free to point out), but is the general idea correct? I will be grateful for any feedback.

Thanks,
Ilya Gladyshev

#2Ilya Gladyshev
i.gladyshev@postgrespro.ru
In reply to: Илья Гладышев (#1)
1 attachment(s)
Re: Update of partition key on foreign server

2 авг. 2021 г., в 15:29, Илья Гладышев <i.gladyshev@postgrespro.ru> написал(а):

Hi,

I am currently looking into a partition constraint violation that occurs on update of a partition key on a foreign server. To reproduce it you can run:

On server 1 using port 5432:

create extension postgres_fdw;
create table players (id integer, name text) partition by list(name);
create table players_0 partition of players for values in ('name1');
create server neighbor foreign data wrapper postgres_fdw options (host 'localhost', port '5433', dbname 'postgres');
create foreign table players_1 partition of players for values in ('name2') server neighbor ;
create user mapping for current_user server neighbor;
On server 2 using port 5433:

create extension postgres_fdw;
create server neighbor foreign data wrapper postgres_fdw options (host 'localhost', port '5432', dbname 'postgres');
create table players (id integer, name text) partition by list(name);
create table players_1 partition of players for values in ('name2');
create user mapping for current_user server neighbor;
create foreign table players_0 partition of players for values in ('name1') server neighbor;

insert into players values (1, 'name1');
update players set name='name2' where name='name1';
ERROR: new row for relation "players_0" violates partition constraint
DETAIL: Failing row contains (1, name2).
CONTEXT: remote SQL command: UPDATE public.players_0 SET name = 'name2'::text WHERE ((name = 'name1'::text))

From what I have read on the mailing list, I understand that this is a known problem, but I haven't found any thread discussing it in particular. Is this something that needs fixing? If it is, I want to try to do it, but I’m wondering if there are any known caveats and looking for any tips on how to implement it.

My understanding is that this should be implemented in a similar way to how the row is routed from a local partition in ExecCrossPartitionUpdate, so the update should be replaced with a foreign delete + local/foreign insert. In addition, a direct update should be forbidden when the query modifies the partition key. I’m probably missing a lot of details (feel free to point out), but is the general idea correct? I will be grateful for any feedback.

Thanks,
Ilya Gladyshev

I have developed a simple patch to fix this, while I’m not fully satisfied with it, it gets the job done. From what I have read in the docs, partition constraints are currently not checked for foreign tables, because they must be enforced on the remote server anyway (because there might be many other ways to access the data source besides FDW), and thus there’s no point in doing that locally. However, in the case of foreign partition update, checking the constraints locally can be used to allow for routing from remote sources, so I don’t feel like this point is valid in this case.

In message [1]/messages/by-id/5A93F487.4080602@lab.ntt.co.jp </messages/by-id/5A93F487.4080602@lab.ntt.co.jp&gt; there’s also mentioned possibility of existence of triggers on the foreign server, and transforming the update to delete+insert will cause these triggers to be omitted. While it is true, I feel like partition pruning already has a similar effect, as it allows to skip scanning foreign partitions. The only way to both fix the update and have the triggers work, that I came up with, is to use parent partitioned table as a target for the foreign update (FDW request would then be "UPDATE public.players …"), while this is possible, it requires the foreign server to have the same partitioned table, which seems quite restrictive to me. Please let me know if I’m missing something or there is a better way to do it.

Thanks,
Ilya Gladyshev

[1]: /messages/by-id/5A93F487.4080602@lab.ntt.co.jp </messages/by-id/5A93F487.4080602@lab.ntt.co.jp&gt;

Attachments:

0001-Partition-key-update-in-foreign-table.patchapplication/octet-stream; name=0001-Partition-key-update-in-foreign-table.patch; x-unix-mode=0644Download
From d1db7218213503a91ccb4b56034bae1c973a9a2e Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <i.gladyshev@posgrespro.ru>
Date: Thu, 5 Aug 2021 00:13:22 +0300
Subject: [PATCH] Partition key update in foreign table

Implements update of partition key for rows in foreign tables via foreign
delete + insert. This requires additional FDW apis: BeginForeignDelete
and EndForeignDelete, mainly because BeginForeignModify is, in any case,
called in the beginning of foreign update and we only know that we will
need separate remote delete+insert at ExecRun after checking partition
constraints. So postgresBeginForeignDelete keeps a separate state in
aux_delete_fmstate not to conflict with BeginForeignModify.
---
 .../postgres_fdw/expected/postgres_fdw.out    |  69 ++++++----
 contrib/postgres_fdw/postgres_fdw.c           | 125 +++++++++++++++++-
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  13 +-
 src/backend/executor/execMain.c               |   1 +
 src/backend/executor/nodeModifyTable.c        |  29 +++-
 src/include/foreign/fdwapi.h                  |   8 ++
 src/include/nodes/execnodes.h                 |   2 +
 7 files changed, 212 insertions(+), 35 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index e3ee30f1aaf..d591f1a28aa 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8122,35 +8122,44 @@ select tableoid::regclass, * FROM locp;
  locp     | 2 | qux
 (1 row)
 
--- It's not allowed to move a row from a partition that is foreign to another
-update utrtest set a = 2 where b = 'foo' returning *;
-ERROR:  new row for relation "loct" violates check constraint "loct_a_check"
-DETAIL:  Failing row contains (2, foo).
-CONTEXT:  remote SQL command: UPDATE public.loct SET a = 2 WHERE ((b = 'foo'::text)) RETURNING a, b
--- But the reverse is allowed
-update utrtest set a = 1 where b = 'qux' returning *;
+-- Moving a row from a foreign partition to local with partition key as qual
+update utrtest set a = 1 where a = 2 returning *;
+ a |  b  
+---+-----
+ 1 | qux
+(1 row)
+
+-- From local to foreign with partition key as qual
+update utrtest set a = 2 where a = 1 and b = 'foo' returning *;
+ a |  b  
+---+-----
+ 2 | foo
+(1 row)
+
+-- Doesn't work without partition key in qual
+update utrtest set a = 1 where b = 'foo' returning *;
 ERROR:  cannot route tuples into foreign table to be updated "remp"
 select tableoid::regclass, * FROM utrtest;
  tableoid | a |  b  
 ----------+---+-----
- remp     | 1 | foo
- locp     | 2 | qux
+ remp     | 1 | qux
+ locp     | 2 | foo
 (2 rows)
 
 select tableoid::regclass, * FROM remp;
  tableoid | a |  b  
 ----------+---+-----
- remp     | 1 | foo
+ remp     | 1 | qux
 (1 row)
 
 select tableoid::regclass, * FROM locp;
  tableoid | a |  b  
 ----------+---+-----
- locp     | 2 | qux
+ locp     | 2 | foo
 (1 row)
 
 -- The executor should not let unexercised FDWs shut down
-update utrtest set a = 1 where b = 'foo';
+update utrtest set a = 1 where b = 'qux';
 -- Test that remote triggers work with update tuple routing
 create trigger loct_br_insert_trigger before insert on loct
 	for each row execute procedure br_insert_trigfunc();
@@ -8159,19 +8168,21 @@ insert into utrtest values (2, 'qux');
 -- Check case where the foreign partition is a subplan target rel
 explain (verbose, costs off)
 update utrtest set a = 1 where a = 1 or a = 2 returning *;
-                                             QUERY PLAN                                             
-----------------------------------------------------------------------------------------------------
+                                              QUERY PLAN                                              
+------------------------------------------------------------------------------------------------------
  Update on public.utrtest
    Output: utrtest_1.a, utrtest_1.b
    Foreign Update on public.remp utrtest_1
+     Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1 RETURNING a, b
    Update on public.locp utrtest_2
    ->  Append
-         ->  Foreign Update on public.remp utrtest_1
-               Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b
+         ->  Foreign Scan on public.remp utrtest_1
+               Output: 1, utrtest_1.tableoid, utrtest_1.ctid, utrtest_1.*
+               Remote SQL: SELECT a, b, ctid FROM public.loct WHERE (((a = 1) OR (a = 2))) FOR UPDATE
          ->  Seq Scan on public.locp utrtest_2
                Output: 1, utrtest_2.tableoid, utrtest_2.ctid, NULL::record
                Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2))
-(10 rows)
+(12 rows)
 
 -- The new values are concatenated with ' triggered !'
 update utrtest set a = 1 where a = 1 or a = 2 returning *;
@@ -8208,18 +8219,20 @@ insert into utrtest values (2, 'qux');
 -- with a direct modification plan
 explain (verbose, costs off)
 update utrtest set a = 1 returning *;
-                                QUERY PLAN                                 
----------------------------------------------------------------------------
+                                  QUERY PLAN                                  
+------------------------------------------------------------------------------
  Update on public.utrtest
    Output: utrtest_1.a, utrtest_1.b
    Foreign Update on public.remp utrtest_1
+     Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1 RETURNING a, b
    Update on public.locp utrtest_2
    ->  Append
-         ->  Foreign Update on public.remp utrtest_1
-               Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b
+         ->  Foreign Scan on public.remp utrtest_1
+               Output: 1, utrtest_1.tableoid, utrtest_1.ctid, utrtest_1.*
+               Remote SQL: SELECT a, b, ctid FROM public.loct FOR UPDATE
          ->  Seq Scan on public.locp utrtest_2
                Output: 1, utrtest_2.tableoid, utrtest_2.ctid, NULL::record
-(9 rows)
+(11 rows)
 
 update utrtest set a = 1 returning *;
 ERROR:  cannot route tuples into foreign table to be updated "remp"
@@ -8268,18 +8281,20 @@ insert into utrtest values (3, 'xyzzy');
 -- with a direct modification plan
 explain (verbose, costs off)
 update utrtest set a = 3 returning *;
-                                QUERY PLAN                                 
----------------------------------------------------------------------------
+                                  QUERY PLAN                                  
+------------------------------------------------------------------------------
  Update on public.utrtest
    Output: utrtest_1.a, utrtest_1.b
    Update on public.locp utrtest_1
    Foreign Update on public.remp utrtest_2
+     Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1 RETURNING a, b
    ->  Append
          ->  Seq Scan on public.locp utrtest_1
                Output: 3, utrtest_1.tableoid, utrtest_1.ctid, NULL::record
-         ->  Foreign Update on public.remp utrtest_2
-               Remote SQL: UPDATE public.loct SET a = 3 RETURNING a, b
-(9 rows)
+         ->  Foreign Scan on public.remp utrtest_2
+               Output: 3, utrtest_2.tableoid, utrtest_2.ctid, utrtest_2.*
+               Remote SQL: SELECT a, b, ctid FROM public.loct FOR UPDATE
+(11 rows)
 
 update utrtest set a = 3 returning *; -- ERROR
 ERROR:  cannot route tuples into foreign table to be updated "remp"
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 9d443baf02a..bb945098ee4 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -209,6 +209,12 @@ typedef struct PgFdwModifyState
 	/* for update row movement if subplan result rel */
 	struct PgFdwModifyState *aux_fmstate;	/* foreign-insert state, if
 											 * created */
+	struct PgFdwModifyState *aux_delete_fmstate;	/* foreign delete state.
+													 * normally used when
+													 * tuple routing with
+													 * foreign delete takes
+													 * place and fmstate is
+													 * already taken. */
 } PgFdwModifyState;
 
 /*
@@ -375,6 +381,10 @@ static void postgresBeginForeignInsert(ModifyTableState *mtstate,
 									   ResultRelInfo *resultRelInfo);
 static void postgresEndForeignInsert(EState *estate,
 									 ResultRelInfo *resultRelInfo);
+static void postgresBeginForeignDelete(ModifyTableState *mtstate,
+									   ResultRelInfo *resultRelInfo);
+static void postgresEndForeignDelete(EState *estate,
+									 ResultRelInfo *resultRelInfo);
 static int	postgresIsForeignRelUpdatable(Relation rel);
 static bool postgresPlanDirectModify(PlannerInfo *root,
 									 ModifyTable *plan,
@@ -571,6 +581,8 @@ postgres_fdw_handler(PG_FUNCTION_ARGS)
 	routine->EndForeignModify = postgresEndForeignModify;
 	routine->BeginForeignInsert = postgresBeginForeignInsert;
 	routine->EndForeignInsert = postgresEndForeignInsert;
+	routine->BeginForeignDelete = postgresBeginForeignDelete;
+	routine->EndForeignDelete = postgresEndForeignDelete;
 	routine->IsForeignRelUpdatable = postgresIsForeignRelUpdatable;
 	routine->PlanDirectModify = postgresPlanDirectModify;
 	routine->BeginDirectModify = postgresBeginDirectModify;
@@ -2078,12 +2090,19 @@ postgresExecForeignDelete(EState *estate,
 						  TupleTableSlot *slot,
 						  TupleTableSlot *planSlot)
 {
+	PgFdwModifyState *fmstate = resultRelInfo->ri_FdwState;
 	TupleTableSlot **rslot;
 	int			numSlots = 1;
 
+	if (fmstate->aux_delete_fmstate)
+		resultRelInfo->ri_FdwState = fmstate->aux_delete_fmstate;
+
 	rslot = execute_foreign_modify(estate, resultRelInfo, CMD_DELETE,
 								   &slot, &planSlot, &numSlots);
 
+	if (fmstate->aux_delete_fmstate)
+		resultRelInfo->ri_FdwState = fmstate;
+
 	return rslot ? rslot[0] : NULL;
 }
 
@@ -2259,6 +2278,106 @@ postgresEndForeignInsert(EState *estate,
 	finish_foreign_modify(fmstate);
 }
 
+/*
+ * postgresBeginForeignDelete
+ *     Setup state for delete on foreign table.
+ *     Primarily needed when tuple routing from a foreign table is required.
+ *     Sets up state in aux_delete_state if necessary
+ */
+static void
+postgresBeginForeignDelete(ModifyTableState *mtstate,
+						   ResultRelInfo *resultRelInfo)
+{
+	ModifyTable *plan = castNode(ModifyTable, mtstate->ps.plan);
+	EState	   *estate = mtstate->ps.state;
+	PgFdwModifyState *fmstate;
+	StringInfoData sql;
+	Index		resultRelation;
+	Relation	rel = resultRelInfo->ri_RelationDesc;
+	RangeTblEntry *rte;
+	List	   *retrievedAttrs;
+
+	initStringInfo(&sql);
+
+	/*
+	 * If the foreign table is a partition that doesn't have a corresponding
+	 * RTE entry, we need to create a new RTE describing the foreign table for
+	 * use by deparseDeleteSql and create_foreign_modify() below, after first
+	 * copying the parent's RTE and modifying some fields to describe the
+	 * foreign partition to work on. However, if this is invoked by UPDATE,
+	 * the existing RTE may already correspond to this partition if it is one
+	 * of the UPDATE subplan target rels; in that case, we can just use the
+	 * existing RTE as-is.
+	 */
+	if (resultRelInfo->ri_RangeTableIndex == 0)
+	{
+		ResultRelInfo *rootResultRelInfo = resultRelInfo->ri_RootResultRelInfo;
+
+		rte = exec_rt_fetch(rootResultRelInfo->ri_RangeTableIndex, estate);
+		rte = copyObject(rte);
+		rte->relid = RelationGetRelid(rel);
+		rte->relkind = RELKIND_FOREIGN_TABLE;
+
+		/*
+		 * For UPDATE, we must use the RT index of the first subplan target
+		 * rel's RTE, because the core code would have built expressions for
+		 * the partition, such as RETURNING, using that RT index as varno of
+		 * Vars contained in those expressions.
+		 */
+		if (plan && plan->operation == CMD_UPDATE &&
+			rootResultRelInfo->ri_RangeTableIndex == plan->rootRelation)
+			resultRelation = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+		else
+			resultRelation = rootResultRelInfo->ri_RangeTableIndex;
+	}
+	else
+	{
+		resultRelation = resultRelInfo->ri_RangeTableIndex;
+		rte = exec_rt_fetch(resultRelation, estate);
+	}
+
+	deparseDeleteSql(&sql, rte, resultRelation, rel, NIL, &retrievedAttrs);
+
+	fmstate = create_foreign_modify(mtstate->ps.state,
+									rte,
+									resultRelInfo,
+									CMD_DELETE,
+									outerPlanState(mtstate)->plan,
+									sql.data,
+									NIL,
+									0,
+									false,
+									NIL);
+
+	if (resultRelInfo->ri_FdwState)
+	{
+		Assert(plan && plan->operation == CMD_UPDATE);
+		Assert(resultRelInfo->ri_usesFdwDirectModify == false);
+		((PgFdwModifyState *) resultRelInfo->ri_FdwState)->aux_delete_fmstate = fmstate;
+	}
+	else
+		resultRelInfo->ri_FdwState = fmstate;
+}
+
+/*
+ * postgresEndForeignDelete
+ *     Tears down state for delete on foreign table
+ */
+static void
+postgresEndForeignDelete(EState *estate,
+						 ResultRelInfo *resultRelInfo)
+{
+	PgFdwModifyState *fmstate = (PgFdwModifyState *) resultRelInfo->ri_FdwState;
+
+	Assert(fmstate != NULL);
+
+	if (fmstate->aux_delete_fmstate)
+		fmstate = fmstate->aux_delete_fmstate;
+
+	/* Destroy the execution state */
+	finish_foreign_modify(fmstate);
+}
+
 /*
  * postgresIsForeignRelUpdatable
  *		Determine whether a foreign table supports INSERT, UPDATE and/or
@@ -2426,8 +2545,11 @@ postgresPlanDirectModify(PlannerInfo *root,
 
 	/*
 	 * The table modification must be an UPDATE or DELETE.
+	 * Partition column update might involve tuple routing,
+	 * which is implemented in ModifyTable, hence direct modification
+	 * should be forbidden in this case.
 	 */
-	if (operation != CMD_UPDATE && operation != CMD_DELETE)
+	if (plan->partColsUpdated || (operation != CMD_UPDATE && operation != CMD_DELETE))
 		return false;
 
 	/*
@@ -4031,6 +4153,7 @@ create_foreign_modify(EState *estate,
 
 	/* Initialize auxiliary state */
 	fmstate->aux_fmstate = NULL;
+	fmstate->aux_delete_fmstate = NULL;
 
 	return fmstate;
 }
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 30b5175da5b..04bd5f2e54c 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2177,18 +2177,21 @@ select tableoid::regclass, * FROM utrtest;
 select tableoid::regclass, * FROM remp;
 select tableoid::regclass, * FROM locp;
 
--- It's not allowed to move a row from a partition that is foreign to another
-update utrtest set a = 2 where b = 'foo' returning *;
+-- Moving a row from a foreign partition to local with partition key as qual
+update utrtest set a = 1 where a = 2 returning *;
+
+-- From local to foreign with partition key as qual
+update utrtest set a = 2 where a = 1 and b = 'foo' returning *;
 
--- But the reverse is allowed
-update utrtest set a = 1 where b = 'qux' returning *;
+-- Doesn't work without partition key in qual
+update utrtest set a = 1 where b = 'foo' returning *;
 
 select tableoid::regclass, * FROM utrtest;
 select tableoid::regclass, * FROM remp;
 select tableoid::regclass, * FROM locp;
 
 -- The executor should not let unexercised FDWs shut down
-update utrtest set a = 1 where b = 'foo';
+update utrtest set a = 1 where b = 'qux';
 
 -- Test that remote triggers work with update tuple routing
 create trigger loct_br_insert_trigger before insert on loct
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b3ce4bae530..e8dabcff507 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1235,6 +1235,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
 	resultRelInfo->ri_projectNewInfoValid = false;
 	resultRelInfo->ri_FdwState = NULL;
 	resultRelInfo->ri_usesFdwDirectModify = false;
+	resultRelInfo->ri_usesFdwForeignDelete = false;
 	resultRelInfo->ri_ConstraintExprs = NULL;
 	resultRelInfo->ri_GeneratedExprs = NULL;
 	resultRelInfo->ri_projectReturning = NULL;
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index d328856ae5b..b238712d318 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1484,6 +1484,14 @@ ExecCrossPartitionUpdate(ModifyTableState *mtstate,
 		MemoryContextSwitchTo(oldcxt);
 	}
 
+	if (!resultRelInfo->ri_usesFdwDirectModify &&
+		resultRelInfo->ri_FdwRoutine != NULL &&
+		resultRelInfo->ri_FdwRoutine->BeginForeignDelete != NULL)
+	{
+		resultRelInfo->ri_usesFdwForeignDelete = true;
+		resultRelInfo->ri_FdwRoutine->BeginForeignDelete(mtstate, resultRelInfo);
+	}
+
 	/*
 	 * Row movement, part 1.  Delete the tuple, but skip RETURNING processing.
 	 * We want to return rows from INSERT.
@@ -1612,6 +1620,7 @@ ExecUpdate(ModifyTableState *mtstate,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	List	   *recheckIndexes = NIL;
+	bool		partition_constraint_failed;
 
 	/*
 	 * abort the operation if not running transactions
@@ -1638,6 +1647,10 @@ ExecUpdate(ModifyTableState *mtstate,
 			return NULL;		/* "do nothing" */
 	}
 
+	ExecMaterializeSlot(slot);
+	partition_constraint_failed = resultRelationDesc->rd_rel->relispartition &&
+		!ExecPartitionCheck(resultRelInfo, slot, estate, false);
+
 	/* INSTEAD OF ROW UPDATE Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_update_instead_row)
@@ -1646,7 +1659,14 @@ ExecUpdate(ModifyTableState *mtstate,
 								  oldtuple, slot))
 			return NULL;		/* "do nothing" */
 	}
-	else if (resultRelInfo->ri_FdwRoutine)
+
+	/*
+	 * if no tuple routing from remote source is required or if it's not
+	 * supported by FDW
+	 */
+	else if (resultRelInfo->ri_FdwRoutine &&
+			 (!partition_constraint_failed ||
+			  resultRelInfo->ri_FdwRoutine->BeginForeignDelete == NULL))
 	{
 		/*
 		 * GENERATED expressions might reference the tableoid column, so
@@ -1683,7 +1703,6 @@ ExecUpdate(ModifyTableState *mtstate,
 	else
 	{
 		LockTupleMode lockmode;
-		bool		partition_constraint_failed;
 		bool		update_indexes;
 
 		/*
@@ -3184,6 +3203,12 @@ ExecEndModifyTable(ModifyTableState *node)
 			resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state,
 														   resultRelInfo);
 
+		if (resultRelInfo->ri_usesFdwForeignDelete &&
+			resultRelInfo->ri_FdwRoutine != NULL &&
+			resultRelInfo->ri_FdwRoutine->EndForeignDelete != NULL)
+			resultRelInfo->ri_FdwRoutine->EndForeignDelete(node->ps.state,
+														   resultRelInfo);
+
 		/*
 		 * Cleanup the initialized batch slots. This only matters for FDWs
 		 * with batching, but the other cases will have ri_NumSlotsInitialized
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index a801cd30576..cc498932d8e 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -113,6 +113,12 @@ typedef void (*BeginForeignInsert_function) (ModifyTableState *mtstate,
 typedef void (*EndForeignInsert_function) (EState *estate,
 										   ResultRelInfo *rinfo);
 
+typedef void (*BeginForeignDelete_function) (ModifyTableState *mtstate,
+											 ResultRelInfo *rinfo);
+
+typedef void (*EndForeignDelete_function) (EState *estate,
+										   ResultRelInfo *rinfo);
+
 typedef int (*IsForeignRelUpdatable_function) (Relation rel);
 
 typedef bool (*PlanDirectModify_function) (PlannerInfo *root,
@@ -237,6 +243,8 @@ typedef struct FdwRoutine
 	EndForeignModify_function EndForeignModify;
 	BeginForeignInsert_function BeginForeignInsert;
 	EndForeignInsert_function EndForeignInsert;
+	BeginForeignDelete_function BeginForeignDelete;
+	EndForeignDelete_function EndForeignDelete;
 	IsForeignRelUpdatable_function IsForeignRelUpdatable;
 	PlanDirectModify_function PlanDirectModify;
 	BeginDirectModify_function BeginDirectModify;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 37cb4f3d59a..a858d94b143 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -460,6 +460,8 @@ typedef struct ResultRelInfo
 	/* true when modifying foreign table directly */
 	bool		ri_usesFdwDirectModify;
 
+	bool		ri_usesFdwForeignDelete;
+
 	/* batch insert stuff */
 	int			ri_NumSlots;	/* number of slots in the array */
 	int			ri_NumSlotsInitialized; /* number of initialized slots */
-- 
2.30.1 (Apple Git-130)

#3Etsuro Fujita
etsuro.fujita@gmail.com
In reply to: Ilya Gladyshev (#2)
Re: Update of partition key on foreign server

On Thu, Aug 26, 2021 at 11:10 PM Ilya Gladyshev
<i.gladyshev@postgrespro.ru> wrote:

I have developed a simple patch to fix this, while I’m not fully satisfied with it, it gets the job done.

Thanks for working on this!

In message [1] there’s also mentioned possibility of existence of triggers on the foreign server, and transforming the update to delete+insert will cause these triggers to be omitted.

Yeah, I still think so.

While it is true, I feel like partition pruning already has a similar effect, as it allows to skip scanning foreign partitions.

I don’t fully understand this part. Could you elaborate a bit more on it?

The only way to both fix the update and have the triggers work, that I came up with, is to use parent partitioned table as a target for the foreign update (FDW request would then be "UPDATE public.players …"), while this is possible, it requires the foreign server to have the same partitioned table, which seems quite restrictive to me.

Yeah, that would be too restrictive.

Best regards,
Etsuro Fujita