public inbox for [email protected]
help / color / mirror / Atom feedFrom: Rafia Sabih <[email protected]>
To: Matheus Alcantara <[email protected]>
Cc: Ayush Tiwari <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Etsuro Fujita <[email protected]>
Subject: Re: BUG #19484: Segmentation fault triggered by FDW
Date: Sat, 30 May 2026 08:18:13 +0200
Message-ID: <CA+FpmFc10Dtnr9dr=xqK2cNYQ4-1VuOyvOT7-eqXPmiQJoyPew@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAJTYsWXY9C3B-7NZw72OKen2L2rZt=c-t6=kjTJzgj=ZaNPe8g@mail.gmail.com>
<[email protected]>
On Fri, 22 May 2026 at 22:56, Matheus Alcantara <[email protected]>
wrote:
> On Wed May 20, 2026 at 9:37 AM -03, Ayush Tiwari wrote:
> > I reproduced the crash on master. The plan EXPLAIN under
> > force_generic_plan shows runtime pruning is in effect:
> >
> > Update on pt
> > Foreign Update on pt_p2 pt_2
> > -> Append
> > Subplans Removed: 1
> > -> Foreign Update on pt_p2 pt_2
> >
> > The SEGV happens inside postgresBeginForeignModify() because
> > ExecInitModifyTable() builds re-indexed "kept" copies of several
> > parallel per-result-relation lists after dropping pruned relations -
> > withCheckOptionLists, returningLists, updateColnosLists,
> > mergeActionLists and mergeJoinConditions, however two members were
> > missed:
> >
> > - node->fdwPrivLists, read with list_nth(node->fdwPrivLists, i) when
> > BeginForeignModify() is called, and
> > - node->fdwDirectModifyPlans, checked with bms_is_member(i, ...) when
> > setting ri_usesFdwDirectModify.
> >
> > Both were still indexed against the original (pre-pruning) positions
> > while the surrounding loop's "i" is now the kept position. When the
> > foreign partition's kept-index no longer matched its original index,
> > BeginForeignModify() got the wrong fdw_private and crashed.
> >
> > Attached patch builds re-indexed kept copies for these two arrays in
> > the same loop as the other parallel lists, and uses them at the two
> > call sites.
> >
>
A good catch. However there is one issue that remains here,
in show_modifytable_info still is using the old index here fdw_private =
(List *) list_nth(node->fdwPrivLists, j) i.e. the one before pruning.
In fact I found a scenario where it is causing crash, try this
create table fdw_part_update2 (a int not null, b int) partition by list (a);
create table fdw_part_update2_p1 partition of fdw_part_update2 for values
in (1);
create table fdw_part_update2_remote (a int not null, b int);
create foreign table fdw_part_update2_p2 partition of fdw_part_update2
for values in (2)
server loopback options (table_name 'fdw_part_update2_remote');
insert into fdw_part_update2_p1 values (1, 10);
insert into fdw_part_update2_remote values (2, 20);
set plan_cache_mode = force_generic_plan;
prepare fdw_part_upd2(int) as
update fdw_part_update2 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);
Please find the attached file for the patch to fix this. This patch applies
over the earlier patch (given by Ayush) in this thread.
>
> Hi, thanks for the patch. This issue started on version 18 by commit
> cbc127917e0.
>
> The patch fixes the issue and it make sense to me. One a minor comment
> is that I think pg_indent is needed on nodeModifyTable.c
>
> --
> Matheus Alcantara
> EDB: https://www.enterprisedb.com
>
>
>
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH
Attachments:
[application/octet-stream] 0001-Fix-show_modifytable_info.patch (5.7K, 3-0001-Fix-show_modifytable_info.patch)
download | inline diff:
From 984ff4240463ae8627e734351afa4f9d131162dd Mon Sep 17 00:00:00 2001
From: Rafia Sabih <[email protected]>
Date: Sat, 30 May 2026 08:11:35 +0200
Subject: [PATCH] Fix show_modifytable_info()
show_modifytable_info() in explain.c reads the plan-indexed
node->fdwPrivLists using the post-pruning executor index j,
producing an out-of-bounds access when calling
ExplainForeignModify on a non-direct-modify FDW relation.
Fix by saving the re-indexed list to mtstate->fdwPrivLists (new field
in ModifyTableState) and reading from there in explain.c.
---
.../postgres_fdw/expected/postgres_fdw.out | 25 +++++++++++++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 7 ++++++
src/backend/commands/explain.c | 2 +-
src/backend/executor/nodeModifyTable.c | 3 ++-
src/include/nodes/execnodes.h | 3 ++-
5 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 872d871a675..5f5cb78ee65 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -9360,6 +9360,31 @@ execute fdw_part_upd(2);
(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;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index c80aaf1c1b4..dc135573a21 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2741,6 +2741,13 @@ prepare fdw_part_upd(int) as
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;
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 112c17b0d64..92326291129 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->fdwPrivLists, j);
fdwroutine->ExplainForeignModify(mtstate,
resultRelInfo,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index f69060cb5ab..a66509465b9 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -5109,7 +5109,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
List *withCheckOptionLists = NIL;
List *returningLists = NIL;
/* fdwPrivLists/fdwDirectModifyPlans are re-indexed to match resultRelations */
- List *fdwPrivLists = NIL;
+ List *fdwPrivLists = NIL;
Bitmapset *fdwDirectModifyPlans = NULL;
List *updateColnosLists = NIL;
List *mergeActionLists = NIL;
@@ -5230,6 +5230,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_updateColnosLists = updateColnosLists;
mtstate->mt_mergeActionLists = mergeActionLists;
mtstate->mt_mergeJoinConditions = mergeJoinConditions;
+ mtstate->fdwPrivLists = fdwPrivLists;
/*----------
* Resolve the target relation. This is the same as:
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 13359180d25..7b33d4d0410 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1444,7 +1444,8 @@ typedef struct ModifyTableState
bool mt_done; /* are we done? */
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 *fdwPrivLists;
/*
* Target relation mentioned in the original statement, used to fire
* statement-level triggers and as the root for tuple routing. (This
--
2.39.5 (Apple Git-154)
view thread (7+ 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], [email protected], [email protected]
Subject: Re: BUG #19484: Segmentation fault triggered by FDW
In-Reply-To: <CA+FpmFc10Dtnr9dr=xqK2cNYQ4-1VuOyvOT7-eqXPmiQJoyPew@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