From ec941e96354e39517a8dd82eb64e8673ffef310c Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Mon, 22 Jun 2026 17:21:45 +0900 Subject: [PATCH v3] Re-index ModifyTable FDW arrays when pruning result relations ExecInitModifyTable() rebuilds the per-result-relation lists after dropping result relations removed by initial runtime pruning. The re-indexing was done for withCheckOptionLists, returningLists, updateColnosLists, mergeActionLists and mergeJoinConditions, but fdwPrivLists and fdwDirectModifyPlans were missed. As a result, a kept foreign result relation could be handed the wrong fdw_private, or ri_usesFdwDirectModify could be set from the wrong plan index, leading to wrong behavior or a crash in BeginForeignModify() and in the direct-modify path. show_modifytable_info() had the same problem: it indexed the plan-ordered node->fdwPrivLists with the post-pruning executor position, so once initial pruning removed a result relation it could read a different relation's fdw_private (often a NIL entry), producing wrong EXPLAIN output or a crash. Fix by re-indexing fdwPrivLists and fdwDirectModifyPlans alongside the other lists, saving the re-indexed private lists in ModifyTableState.mt_fdwPrivLists and reading from there in both nodeModifyTable.c and explain.c. Reported-by: Chi Zhang <798604270@qq.com> Author: Ayush Tiwari Author: Rafia Sabih Reviewed-by: Matheus Alcantara Reviewed-by: Etsuro Fujita Discussion: https://postgr.es/m/19484-a3cb82c8cde3c8fa%40postgresql.org Backpatch-through: 18 --- .../postgres_fdw/expected/postgres_fdw.out | 64 +++++++++++++++++++ contrib/postgres_fdw/sql/postgres_fdw.sql | 34 ++++++++++ src/backend/commands/explain.c | 2 +- src/backend/executor/nodeModifyTable.c | 22 ++++++- src/include/nodes/execnodes.h | 8 ++- 5 files changed, 124 insertions(+), 6 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index abfc84f8860..be455710c9b 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7132,6 +7132,70 @@ RESET enable_material; DROP FOREIGN TABLE remt2; DROP TABLE loct1; DROP TABLE loct2; +-- Test that direct modify and foreign modify work with runtime pruning of +-- result relations (bug #19484) +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; +-- Check DirectModify case +prepare fdw_part_upd(int) as + update fdw_part_update set b = b + 1 where a = $1 + returning tableoid::regclass, a, b; +explain (verbose, costs off) + execute fdw_part_upd(2); + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------- + Update on public.fdw_part_update + 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 + -> Append + Subplans Removed: 1 + -> Foreign Update on public.fdw_part_update_p2 fdw_part_update_2 + Remote SQL: UPDATE public.fdw_part_update_remote SET b = (b + 1) WHERE ((a = $1::integer)) RETURNING a, b +(7 rows) + +execute fdw_part_upd(2); + tableoid | a | b +--------------------+---+---- + fdw_part_update_p2 | 2 | 21 +(1 row) + +deallocate fdw_part_upd; +-- Check ForeignModify case +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; +explain (verbose, costs off) + execute fdw_part_upd2(2); + QUERY PLAN +-------------------------------------------------------------------------------------------------------------------------------------------------------- + Update on public.fdw_part_update + 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 + Subplans Removed: 1 + -> Foreign Scan on public.fdw_part_update_p2 fdw_part_update_2 + 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) + +execute fdw_part_upd2(2); + tableoid | a | b +--------------------+---+---- + fdw_part_update_p2 | 2 | 22 +(1 row) + +deallocate fdw_part_upd2; +reset plan_cache_mode; +drop table fdw_part_update; +drop table fdw_part_update_remote; -- =================================================================== -- test check constraints -- =================================================================== diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index f03c5aab4d9..7d2bd6700bc 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1732,6 +1732,40 @@ DROP FOREIGN TABLE remt2; DROP TABLE loct1; DROP TABLE loct2; +-- Test that direct modify and foreign modify work with runtime pruning of +-- result relations (bug #19484) +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; + +-- Check DirectModify case +prepare fdw_part_upd(int) as + update fdw_part_update set b = b + 1 where a = $1 + returning tableoid::regclass, a, b; +explain (verbose, costs off) + execute fdw_part_upd(2); +execute fdw_part_upd(2); +deallocate fdw_part_upd; + +-- Check ForeignModify case +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; +explain (verbose, costs off) + execute fdw_part_upd2(2); +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 check constraints -- =================================================================== diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 7e2792ead71..6d2624e75b8 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -4609,7 +4609,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 ea8ed94a764..cac30666663 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -4647,6 +4647,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) List *updateColnosLists = NIL; List *mergeActionLists = NIL; List *mergeJoinConditions = NIL; + List *fdwPrivLists = NIL; + Bitmapset *fdwDirectModifyPlans = NULL; ResultRelInfo *resultRelInfo; List *arowmarks; ListCell *l; @@ -4689,6 +4691,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (keep_rel) { + List *fdwPrivList = (List *) list_nth(node->fdwPrivLists, i); + resultRelations = lappend_int(resultRelations, rti); if (node->withCheckOptionLists) { @@ -4724,6 +4728,19 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mergeJoinConditions = lappend(mergeJoinConditions, mergeJoinCondition); } + + /* + * fdwPrivLists/fdwDirectModifyPlans are re-indexed to match + * resultRelations + */ + fdwPrivLists = lappend(fdwPrivLists, fdwPrivList); + if (bms_is_member(i, node->fdwDirectModifyPlans)) + { + int new_index = list_length(resultRelations) - 1; + + fdwDirectModifyPlans = bms_add_member(fdwDirectModifyPlans, + new_index); + } } i++; } @@ -4753,6 +4770,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: @@ -4828,7 +4846,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 @@ -4857,7 +4875,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 409e172bfb6..6677a03caab 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1453,13 +1453,15 @@ typedef struct ModifyTableState double mt_merge_deleted; /* - * Lists of valid updateColnosLists, mergeActionLists, and - * mergeJoinConditions. These contain only entries for unpruned - * relations, filtered from the corresponding lists in ModifyTable. + * Lists of valid updateColnosLists, mergeActionLists, + * mergeJoinConditions, and fdwPrivLists. These contain only entries for + * unpruned relations, filtered from the corresponding lists in + * ModifyTable. */ List *mt_updateColnosLists; List *mt_mergeActionLists; List *mt_mergeJoinConditions; + List *mt_fdwPrivLists; } ModifyTableState; /* ---------------- -- 2.47.3