From 971d185cf4bfb75d0459f2c7848e6b77afa2ee85 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Tue, 9 Jun 2026 11:55:34 -0300 Subject: [PATCH v2] Re-index ModifyTable FDW arrays when pruning result relations ExecInitModifyTable() copies parallel per-result-relation lists from the plan node into a new "kept" set after dropping pruned result relations. That re-indexing was already done for withCheckOptionLists, returningLists, updateColnosLists, mergeActionLists and mergeJoinConditions, but fdwPrivLists and fdwDirectModifyPlans were missed. Additionally, show_modifytable_info() in explain.c was reading the plan-indexed node->fdwPrivLists using the post-pruning executor index, causing out-of-bounds access. Fix by saving the re-indexed list to mtstate->mt_fdwPrivLists and reading from there. Author: Ayush Tiwari Author: Rafia Sabih Co-authored-by: Matheus Alcantara Reviewed-by: Ayush Tiwari Reported-by: Chi Zhang <798604270@qq.com> Discussion: https://www.postgresql.org/message-id/19484-a3cb82c8cde3c8fa%40postgresql.org --- .../postgres_fdw/expected/postgres_fdw.out | 51 +++++++++++++++++++ contrib/postgres_fdw/sql/postgres_fdw.sql | 29 +++++++++++ src/backend/commands/explain.c | 2 +- src/backend/executor/nodeModifyTable.c | 23 ++++++++- src/include/nodes/execnodes.h | 6 +++ 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index e90289e4ab1..5f5cb78ee65 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -9337,6 +9337,57 @@ select tableoid::regclass, * FROM locp; -- The executor should not let unexercised FDWs shut down update utrtest set a = 1 where b = 'foo'; +-- Runtime pruning of result relations must keep ModifyTable's per-relation +-- FDW arrays (fdwPrivLists, fdwDirectModifyPlans) aligned with the kept +-- resultRelations. Otherwise BeginForeignModify() reads the wrong +-- fdw_private and segfaults. +create table fdw_part_update (a int not null, b int) partition by list (a); +create table fdw_part_update_p1 partition of fdw_part_update for values in (1); +create table fdw_part_update_remote (a int not null, b int); +create foreign table fdw_part_update_p2 partition of fdw_part_update + for values in (2) + server loopback options (table_name 'fdw_part_update_remote'); +insert into fdw_part_update_p1 values (1, 10); +insert into fdw_part_update_remote values (2, 20); +set plan_cache_mode = force_generic_plan; +prepare fdw_part_upd(int) as + update fdw_part_update set b = b + 1 where a = $1 + returning tableoid::regclass, a, b; +execute fdw_part_upd(2); + tableoid | a | b +--------------------+---+---- + fdw_part_update_p2 | 2 | 21 +(1 row) + +deallocate fdw_part_upd; +prepare fdw_part_upd2(int) as + update fdw_part_update set b = b + random()::int * 0 + 1 where a = $1 + returning tableoid::regclass, a, b; +execute fdw_part_upd2(2); + tableoid | a | b +--------------------+---+---- + fdw_part_update_p2 | 2 | 22 +(1 row) + +explain (analyze, verbose, costs off, timing off, summary off) + execute fdw_part_upd2(2); + QUERY PLAN +-------------------------------------------------------------------------------------------------------------------------------------------------------- + Update on public.fdw_part_update (actual rows=1.00 loops=1) + Output: (fdw_part_update_1.tableoid)::regclass, fdw_part_update_1.a, fdw_part_update_1.b + Foreign Update on public.fdw_part_update_p2 fdw_part_update_2 + Remote SQL: UPDATE public.fdw_part_update_remote SET b = $2 WHERE ctid = $1 RETURNING a, b + -> Append (actual rows=1.00 loops=1) + Subplans Removed: 1 + -> Foreign Scan on public.fdw_part_update_p2 fdw_part_update_2 (actual rows=1.00 loops=1) + Output: ((fdw_part_update_2.b + ((random())::integer * 0)) + 1), fdw_part_update_2.tableoid, fdw_part_update_2.ctid, fdw_part_update_2.* + Remote SQL: SELECT a, b, ctid FROM public.fdw_part_update_remote WHERE ((a = $1::integer)) FOR UPDATE +(9 rows) + +deallocate fdw_part_upd2; +reset plan_cache_mode; +drop table fdw_part_update; +drop table fdw_part_update_remote; -- 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(); diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index dfc58beb0d2..dc135573a21 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2723,6 +2723,35 @@ select tableoid::regclass, * FROM locp; -- The executor should not let unexercised FDWs shut down update utrtest set a = 1 where b = 'foo'; +-- Runtime pruning of result relations must keep ModifyTable's per-relation +-- FDW arrays (fdwPrivLists, fdwDirectModifyPlans) aligned with the kept +-- resultRelations. Otherwise BeginForeignModify() reads the wrong +-- fdw_private and segfaults. +create table fdw_part_update (a int not null, b int) partition by list (a); +create table fdw_part_update_p1 partition of fdw_part_update for values in (1); +create table fdw_part_update_remote (a int not null, b int); +create foreign table fdw_part_update_p2 partition of fdw_part_update + for values in (2) + server loopback options (table_name 'fdw_part_update_remote'); +insert into fdw_part_update_p1 values (1, 10); +insert into fdw_part_update_remote values (2, 20); +set plan_cache_mode = force_generic_plan; +prepare fdw_part_upd(int) as + update fdw_part_update set b = b + 1 where a = $1 + returning tableoid::regclass, a, b; +execute fdw_part_upd(2); +deallocate fdw_part_upd; +prepare fdw_part_upd2(int) as + update fdw_part_update set b = b + random()::int * 0 + 1 where a = $1 + returning tableoid::regclass, a, b; +execute fdw_part_upd2(2); +explain (analyze, verbose, costs off, timing off, summary off) + execute fdw_part_upd2(2); +deallocate fdw_part_upd2; +reset plan_cache_mode; +drop table fdw_part_update; +drop table fdw_part_update_remote; + -- 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(); diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 112c17b0d64..a40d03d35f3 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -4821,7 +4821,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors, fdwroutine != NULL && fdwroutine->ExplainForeignModify != NULL) { - List *fdw_private = (List *) list_nth(node->fdwPrivLists, j); + List *fdw_private = (List *) list_nth(mtstate->mt_fdwPrivLists, j); fdwroutine->ExplainForeignModify(mtstate, resultRelInfo, diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 33a6735f08d..feea28214d4 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -5105,6 +5105,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) List *resultRelations = NIL; List *withCheckOptionLists = NIL; List *returningLists = NIL; + + /* + * fdwPrivLists/fdwDirectModifyPlans are re-indexed to match + * resultRelations + */ + List *fdwPrivLists = NIL; + Bitmapset *fdwDirectModifyPlans = NULL; List *updateColnosLists = NIL; List *mergeActionLists = NIL; List *mergeJoinConditions = NIL; @@ -5150,6 +5157,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (keep_rel) { + int new_index = list_length(resultRelations); + resultRelations = lappend_int(resultRelations, rti); if (node->withCheckOptionLists) { @@ -5167,6 +5176,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) returningLists = lappend(returningLists, returningList); } + if (node->fdwPrivLists) + { + List *fdwPrivList = (List *) list_nth(node->fdwPrivLists, i); + + fdwPrivLists = lappend(fdwPrivLists, fdwPrivList); + } + if (bms_is_member(i, node->fdwDirectModifyPlans)) + fdwDirectModifyPlans = bms_add_member(fdwDirectModifyPlans, + new_index); if (node->updateColnosLists) { List *updateColnosList = list_nth(node->updateColnosLists, i); @@ -5213,6 +5231,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->mt_updateColnosLists = updateColnosLists; mtstate->mt_mergeActionLists = mergeActionLists; mtstate->mt_mergeJoinConditions = mergeJoinConditions; + mtstate->mt_fdwPrivLists = fdwPrivLists; /*---------- * Resolve the target relation. This is the same as: @@ -5288,7 +5307,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) /* Initialize the usesFdwDirectModify flag */ resultRelInfo->ri_usesFdwDirectModify = - bms_is_member(i, node->fdwDirectModifyPlans); + bms_is_member(i, fdwDirectModifyPlans); /* * Verify result relation is a valid target for the current operation @@ -5317,7 +5336,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) resultRelInfo->ri_FdwRoutine != NULL && resultRelInfo->ri_FdwRoutine->BeginForeignModify != NULL) { - List *fdw_private = (List *) list_nth(node->fdwPrivLists, i); + List *fdw_private = (List *) list_nth(fdwPrivLists, i); resultRelInfo->ri_FdwRoutine->BeginForeignModify(mtstate, resultRelInfo, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 53c138310db..5871383961f 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1446,6 +1446,12 @@ typedef struct ModifyTableState int mt_nrels; /* number of entries in resultRelInfo[] */ ResultRelInfo *resultRelInfo; /* info about target relation(s) */ + /* + * Re-indexed fdw private data lists, aligned with resultRelInfo[] after + * pruning + */ + List *mt_fdwPrivLists; + /* * Target relation mentioned in the original statement, used to fire * statement-level triggers and as the root for tuple routing. (This -- 2.50.1 (Apple Git-155)