BUG #19484: Segmentation fault triggered by FDW

Hi,

On Wed, 20 May 2026 at 03:59, PG Bug reporting form <noreply@postgresql.org>
wrote:

The following bug has been logged on the website:

Bug reference: 19484
Logged by: Chi Zhang
Email address: 798604270@qq.com
PostgreSQL version: 18.4
Operating system: Ubuntu 24.04
Description:

Hi,

I found the following test case triggers a segmentation fault:

```
\set ON_ERROR_STOP on

CREATE EXTENSION postgres_fdw;

CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (
host '/path/to/pg_socket',
port '5432',
dbname :'dbname'
);

CREATE USER MAPPING FOR postgres SERVER loopback
OPTIONS (user 'postgres');

CREATE SCHEMA r;
CREATE TABLE r.remote_p2 (a int NOT NULL, b int);

CREATE TABLE pt (a int NOT NULL, b int) PARTITION BY LIST (a);
CREATE TABLE pt_p1 PARTITION OF pt FOR VALUES IN (1);
CREATE FOREIGN TABLE pt_p2 PARTITION OF pt FOR VALUES IN (2)
SERVER loopback
OPTIONS (schema_name 'r', table_name 'remote_p2');

INSERT INTO pt_p1 VALUES (1, 10);
INSERT INTO r.remote_p2 VALUES (2, 20);

SET plan_cache_mode = force_generic_plan;

PREPARE upd(int) AS
UPDATE pt
SET b = b + 1
WHERE a = $1
RETURNING tableoid::regclass, a, b;

EXPLAIN (costs off) EXECUTE upd(2);
EXECUTE upd(2);
SELECT * FROM r.remote_p2 ORDER BY a;

Thanks for the very precise repro, that made this easy to track down.

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.

Regards,
Ayush

Hi,

On Wed, May 20, 2026 at 5:37 AM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

On Wed, 20 May 2026 at 03:59, PG Bug reporting form <noreply@postgresql.org> wrote:

I found the following test case triggers a segmentation fault:

[snip]

Thanks for the very precise repro, that made this easy to track down.

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.

Thanks Chi for the report, and Ayush for the analysis and patch! Will review.

Best regards,
Etsuro Fujita

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.

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

On Fri, 22 May 2026 at 22:56, Matheus Alcantara <matheusssilv97@gmail.com>
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

On Sat May 30, 2026 at 3:18 AM -03, Rafia Sabih wrote:

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.

Thanks for catching this, Rafia. The fix is correct —
show_modifytable_info() was indeed still reading from node->fdwPrivLists
using the post-pruning index j, which causes an out-of-bounds access
when partitions are pruned.

I think both patches should be squashed into a single one since they fix
the same underlying issue. I've done this locally and also ran pg_indent
over the result. Attached is the combined patch.

One minor naming observation: the new fdwPrivLists field in
ModifyTableState doesn't follow the mt_ prefix convention used by the
other re-indexed lists (mt_updateColnosLists, mt_mergeActionLists,
mt_mergeJoinConditions). Should we rename it to mt_fdwPrivLists for
consistency?

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Hi,

On Tue, 9 Jun 2026 at 20:40, Matheus Alcantara <matheusssilv97@gmail.com>
wrote:

I think both patches should be squashed into a single one since they fix
the same underlying issue. I've done this locally and also ran pg_indent
over the result. Attached is the combined patch.

Thanks for this!

One minor naming observation: the new fdwPrivLists field in
ModifyTableState doesn't follow the mt_ prefix convention used by the
other re-indexed lists (mt_updateColnosLists, mt_mergeActionLists,
mt_mergeJoinConditions). Should we rename it to mt_fdwPrivLists for
consistency?

I think yes, it makes sense to rename it.

Regards,
Ayush

On Wed Jun 10, 2026 at 2:15 AM -03, Ayush Tiwari wrote:

One minor naming observation: the new fdwPrivLists field in
ModifyTableState doesn't follow the mt_ prefix convention used by the
other re-indexed lists (mt_updateColnosLists, mt_mergeActionLists,
mt_mergeJoinConditions). Should we rename it to mt_fdwPrivLists for
consistency?

I think yes, it makes sense to rename it.

Attached v2 renamed, thanks.

(Also CC Amit on this since he committed cbc127917e0 which I believe
that is when the issue started)

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Hi Matheus,

On Wed, Jun 10, 2026 at 8:09 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:

On Wed Jun 10, 2026 at 2:15 AM -03, Ayush Tiwari wrote:

One minor naming observation: the new fdwPrivLists field in
ModifyTableState doesn't follow the mt_ prefix convention used by the
other re-indexed lists (mt_updateColnosLists, mt_mergeActionLists,
mt_mergeJoinConditions). Should we rename it to mt_fdwPrivLists for
consistency?

I think yes, it makes sense to rename it.

Attached v2 renamed, thanks.

(Also CC Amit on this since he committed cbc127917e0 which I believe
that is when the issue started)

Thanks for adding me. I'll take a look at this early next week.

--
Thanks, Amit Langote

On Wed, Jun 10, 2026 at 10:09 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Wed, Jun 10, 2026 at 8:09 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:

On Wed Jun 10, 2026 at 2:15 AM -03, Ayush Tiwari wrote:

One minor naming observation: the new fdwPrivLists field in
ModifyTableState doesn't follow the mt_ prefix convention used by the
other re-indexed lists (mt_updateColnosLists, mt_mergeActionLists,
mt_mergeJoinConditions). Should we rename it to mt_fdwPrivLists for
consistency?

I think yes, it makes sense to rename it.

Attached v2 renamed, thanks.

(Also CC Amit on this since he committed cbc127917e0 which I believe
that is when the issue started)

Thanks for adding me. I'll take a look at this early next week.

I looked, and the patch seems straightforward enough.

Before committing it, I'd like to wait briefly to see if Fujita-san
has any thoughts on the FDW-side concerns, since he has already chimed
in on the thread.

--
Thanks, Amit Langote

Amit-san,

On Wed, Jun 10, 2026 at 10:27 PM Amit Langote <amitlangote09@gmail.com> wrote:

Before committing it, I'd like to wait briefly to see if Fujita-san
has any thoughts on the FDW-side concerns, since he has already chimed
in on the thread.

I'll review the patch as early as I can next week, but if you want to
commit it quickly, please go ahead.

Thanks!

Best regards,
Etsuro Fujita

Hi Fujita-san,

On Thu, Jun 11, 2026 at 7:41 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Amit-san,

On Wed, Jun 10, 2026 at 10:27 PM Amit Langote <amitlangote09@gmail.com> wrote:

Before committing it, I'd like to wait briefly to see if Fujita-san
has any thoughts on the FDW-side concerns, since he has already chimed
in on the thread.

I'll review the patch as early as I can next week, but if you want to
commit it quickly, please go ahead.

No problem; I'll hold off.

--
Thanks, Amit Langote

Fujita-san,

On Thu, Jun 11, 2026 at 7:44 PM Amit Langote <amitlangote09@gmail.com> wrote:

Hi Fujita-san,

On Thu, Jun 11, 2026 at 7:41 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Amit-san,

On Wed, Jun 10, 2026 at 10:27 PM Amit Langote <amitlangote09@gmail.com> wrote:

Before committing it, I'd like to wait briefly to see if Fujita-san
has any thoughts on the FDW-side concerns, since he has already chimed
in on the thread.

I'll review the patch as early as I can next week, but if you want to
commit it quickly, please go ahead.

No problem; I'll hold off.

A small clarification on this one: the patch makes no changes to
postgres_fdw code, only adds a regression test. The bug, which I
introduced, is in how nodeModifyTable.c supplies fdw_private from the
pruned/validated fdwPrivLists in ModifyTableState rather than the
original list in ModifyTable, so the postgres_fdw side itself is
unchanged. The substantive change is the executor re-indexing.

Still holding off on committing per our exchange, but no rush given
you may want to prioritize the v19 open items. Happy to go ahead
whenever, or wait for your comments.

--
Thanks, Amit Langote

Amit-san,

On Thu, Jun 18, 2026 at 6:16 PM Amit Langote <amitlangote09@gmail.com> wrote:

A small clarification on this one: the patch makes no changes to
postgres_fdw code, only adds a regression test. The bug, which I
introduced, is in how nodeModifyTable.c supplies fdw_private from the
pruned/validated fdwPrivLists in ModifyTableState rather than the
original list in ModifyTable, so the postgres_fdw side itself is
unchanged. The substantive change is the executor re-indexing.

Thanks for the clarification!

Sorry, I accidentally sent this before I finished writing.

On Thu, Jun 18, 2026 at 7:50 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Amit-san,

On Thu, Jun 18, 2026 at 6:16 PM Amit Langote <amitlangote09@gmail.com> wrote:

A small clarification on this one: the patch makes no changes to
postgres_fdw code, only adds a regression test. The bug, which I
introduced, is in how nodeModifyTable.c supplies fdw_private from the
pruned/validated fdwPrivLists in ModifyTableState rather than the
original list in ModifyTable, so the postgres_fdw side itself is
unchanged. The substantive change is the executor re-indexing.

Thanks for the clarification!

Still holding off on committing per our exchange, but no rush given
you may want to prioritize the v19 open items. Happy to go ahead
whenever, or wait for your comments.

Actually, I've just started reviewing the patch. I'll let you know
the results (if any) by tomorrow. Thanks for the consideration as
well.

Best regards,
Etsuro Fujita

On Thu, Jun 18, 2026 at 19:55 Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Sorry, I accidentally sent this before I finished writing.

On Thu, Jun 18, 2026 at 7:50 PM Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:

Amit-san,

On Thu, Jun 18, 2026 at 6:16 PM Amit Langote <amitlangote09@gmail.com>

wrote:

A small clarification on this one: the patch makes no changes to
postgres_fdw code, only adds a regression test. The bug, which I
introduced, is in how nodeModifyTable.c supplies fdw_private from the
pruned/validated fdwPrivLists in ModifyTableState rather than the
original list in ModifyTable, so the postgres_fdw side itself is
unchanged. The substantive change is the executor re-indexing.

Thanks for the clarification!

Still holding off on committing per our exchange, but no rush given
you may want to prioritize the v19 open items. Happy to go ahead
whenever, or wait for your comments.

Actually, I've just started reviewing the patch. I'll let you know
the results (if any) by tomorrow. Thanks for the consideration as
well.

Ah, thanks for the heads up.

- Amit

Amit-san,

On Thu, Jun 18, 2026 at 7:57 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Thu, Jun 18, 2026 at 19:55 Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Actually, I've just started reviewing the patch. I'll let you know
the results (if any) by tomorrow.

Ah, thanks for the heads up.

Here are my review comments:

@@ -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);
+           }

As node->fdwPrivLists is always created, the if-test is useless, so I
removed it like the attached. Also, this is nitpicking but, in the
attached I moved a comment added above
("fdwPrivLists/fdwDirectModifyPlans are re-indexed to match
resultRelations") to a more appropriate place, and did some minor
editorialization.

+-- 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.

This comment isn't 100% correct, because this issue happens with
DirectModify as well. I think we could expand the comment to mention
that as well, but the comment is already too much/detailed IMO; I
don't think we add such a comment for a test case, so how about
simplifying the comment like this: "Test that direct modify and
foreign modify work with runtime pruning of result relations (bug
#19484)"

+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);

I think it's expensive to do explain with the analyze option enabled,
just for conforming the plan. To save cycles, I removed the option.
(As the test case for DirectModify was missing the explain test, I
added it, for consistency.) The test cases are placed in the "test
tuple routing for foreign-table partitions" section, but they are
basic ones to test writable foreign tables, so I moved them to the
"test writable foreign table stuff" section.

@@ -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;

For v18, I think we should put this at the end of the ModifyTableState
struct, to avoid ABI breakage.

That's it. Sorry for the delay.

Best regards,
Etsuro Fujita

Fujita-san,

On Fri, Jun 19, 2026 at 8:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Thu, Jun 18, 2026 at 7:57 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Thu, Jun 18, 2026 at 19:55 Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Actually, I've just started reviewing the patch. I'll let you know
the results (if any) by tomorrow.

Ah, thanks for the heads up.

Here are my review comments:

Thanks a lot for the review.

@@ -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);
+           }

As node->fdwPrivLists is always created, the if-test is useless, so I
removed it like the attached. Also, this is nitpicking but, in the
attached I moved a comment added above
("fdwPrivLists/fdwDirectModifyPlans are re-indexed to match
resultRelations") to a more appropriate place, and did some minor
editorialization.

Looks good.

+-- 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.

This comment isn't 100% correct, because this issue happens with
DirectModify as well. I think we could expand the comment to mention
that as well, but the comment is already too much/detailed IMO; I
don't think we add such a comment for a test case, so how about
simplifying the comment like this: "Test that direct modify and
foreign modify work with runtime pruning of result relations (bug
#19484)"

I agree that the details shouldn't leak into test code and have been
trying to follow that principle. Also, bug numbers shouldn't be
mentioned because one can always use `git blame` to find them.
However, since different committers have different styles, I'm fine
with it; kept in the attached updated patch.

+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);

I think it's expensive to do explain with the analyze option enabled,
just for conforming the plan. To save cycles, I removed the option.
(As the test case for DirectModify was missing the explain test, I
added it, for consistency.) The test cases are placed in the "test
tuple routing for foreign-table partitions" section, but they are
basic ones to test writable foreign tables, so I moved them to the
"test writable foreign table stuff" section.

Sounds good.

@@ -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;

For v18, I think we should put this at the end of the ModifyTableState
struct, to avoid ABI breakage.

Good point on ABI. Rather than placing it differently per branch, how
about putting mt_fdwPrivLists at the end of the struct alongside
mt_updateColnosLists/mt_mergeActionLists/mt_mergeJoinConditions, which
are the same kind of unpruned-filtered lists? That keeps it ABI-safe
and lets master and 18 share an identical change. I'd fold it into
their existing comment too.

That's it. Sorry for the delay.

No problem.

Updated patch attached.

--
Thanks, Amit Langote

Amit-san,

On Mon, Jun 22, 2026 at 5:28 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Fri, Jun 19, 2026 at 8:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

+-- 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.

This comment isn't 100% correct, because this issue happens with
DirectModify as well. I think we could expand the comment to mention
that as well, but the comment is already too much/detailed IMO; I
don't think we add such a comment for a test case, so how about
simplifying the comment like this: "Test that direct modify and
foreign modify work with runtime pruning of result relations (bug
#19484)"

I agree that the details shouldn't leak into test code and have been
trying to follow that principle. Also, bug numbers shouldn't be
mentioned because one can always use `git blame` to find them.
However, since different committers have different styles, I'm fine
with it; kept in the attached updated patch.

Thanks!

@@ -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;

For v18, I think we should put this at the end of the ModifyTableState
struct, to avoid ABI breakage.

Good point on ABI. Rather than placing it differently per branch, how
about putting mt_fdwPrivLists at the end of the struct alongside
mt_updateColnosLists/mt_mergeActionLists/mt_mergeJoinConditions, which
are the same kind of unpruned-filtered lists? That keeps it ABI-safe
and lets master and 18 share an identical change. I'd fold it into
their existing comment too.

That's a good idea! So +1

Updated patch attached.

LGTM.

Best regards,
Etsuro Fujita

On Tue, Jun 23, 2026 at 5:27 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Mon, Jun 22, 2026 at 5:28 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Fri, Jun 19, 2026 at 8:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

+-- 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.

This comment isn't 100% correct, because this issue happens with
DirectModify as well. I think we could expand the comment to mention
that as well, but the comment is already too much/detailed IMO; I
don't think we add such a comment for a test case, so how about
simplifying the comment like this: "Test that direct modify and
foreign modify work with runtime pruning of result relations (bug
#19484)"

I agree that the details shouldn't leak into test code and have been
trying to follow that principle. Also, bug numbers shouldn't be
mentioned because one can always use `git blame` to find them.
However, since different committers have different styles, I'm fine
with it; kept in the attached updated patch.

Thanks!

@@ -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;

For v18, I think we should put this at the end of the ModifyTableState
struct, to avoid ABI breakage.

Good point on ABI. Rather than placing it differently per branch, how
about putting mt_fdwPrivLists at the end of the struct alongside
mt_updateColnosLists/mt_mergeActionLists/mt_mergeJoinConditions, which
are the same kind of unpruned-filtered lists? That keeps it ABI-safe
and lets master and 18 share an identical change. I'd fold it into
their existing comment too.

That's a good idea! So +1

Updated patch attached.

LGTM.

Pushed, thanks for checking.

--
Thanks, Amit Langote