Subject: BUG #19484: Segmentation fault triggered by FDW

From: PG Bug reporting form <[email-masked]>
Date: Mon, 18 May 2026 06:38:34 +0000

The following bug has been logged on the website:

Bug reference:      19484
Logged by:          Chi Zhang
Email address:      [email-masked]
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;

```

This is the log:

```
2026-05-18 13:40:41.888 CST [21729] LOG:  database system is ready to accept
connections
  2026-05-18 13:41:03.317 CST [21932] LOG:  unexpected EOF on client
connection with an open transaction
  2026-05-18 13:41:03.317 CST [21729] LOG:  client backend (PID 21931) was
terminated by signal 11: Segmentation fault
  2026-05-18 13:41:03.317 CST [21729] DETAIL:  Failed process was running:
EXECUTE upd(2);
  2026-05-18 13:41:03.317 CST [21729] LOG:  terminating any other active
server processes
  2026-05-18 13:41:03.319 CST [21729] LOG:  all server processes terminated;
reinitializing
  2026-05-18 13:41:03.345 CST [21936] LOG:  database system was interrupted;
last known up at 2026-05-18 13:40:41 CST
  2026-05-18 13:41:03.509 CST [21936] LOG:  database system was not properly
shut down; automatic recovery in progress
  2026-05-18 13:41:03.513 CST [21936] LOG:  redo starts at 0/98371040
  2026-05-18 13:41:03.531 CST [21936] LOG:  invalid record length at
0/987B6E68: expected at least 24, got 0
  2026-05-18 13:41:03.531 CST [21936] LOG:  redo done at 0/987B6E40 system
usage: CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.01 s
  2026-05-18 13:41:03.537 CST [21937] LOG:  checkpoint starting:
end-of-recovery fast wait
  2026-05-18 13:41:03.654 CST [21937] LOG:  checkpoint complete:
end-of-recovery fast wait: wrote 975 buffers (6.0%), wrote 3 SLRU buffers; 0
WAL file(s) added, 0 removed, 0
recycled; write=0.081 s, sync=0.030 s, total=0.121 s; sync files=325,
longest=0.005 s, average=0.001 s; distance=4375 kB, estimate=4375 kB;
lsn=0/987B6E68, redo lsn=0/987B6E68
  2026-05-18 13:41:03.660 CST [21729] LOG:  database system is ready to
accept connections
```

I built the Postgres from source code
901ed9b352b41f034e17bc540725082a488fce31 of github commit.

---

From: Ayush Tiwari <[email-masked]>
Date: Wed, 20 May 2026 18:07:16 +0530

Hi,

> 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

---

From: Etsuro Fujita <[email-masked]>
Date: Wed, 20 May 2026 10:46:03 -0700

Hi,

On Wed, May 20, 2026 at 5:37 AM Ayush Tiwari
<[email-masked]> wrote:
> On Wed, 20 May 2026 at 03:59, PG Bug reporting form <[email-masked]> wrote:

[snip]

> the same loop as the other parallel lists, and uses them at the two

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

Best regards,
Etsuro Fujita

---

From: "Matheus Alcantara" <[email-masked]>
Date: Fri, 22 May 2026 17:56:05 -0300

> the same loop as the other parallel lists, and uses them at the two

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

---

From: Rafia Sabih <[email-masked]>
Date: Sat, 30 May 2026 08:18:13 +0200

> > the same loop as the other parallel lists, and uses them at the two
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.

-- 
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

---

From: "Matheus Alcantara" <[email-masked]>
Date: Tue, 09 Jun 2026 12:10:42 -0300

> 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
From 4020e86e2ec85b6fd58397b5f4f467d1baf5ad87 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <[email-masked]>
Date: Tue, 9 Jun 2026 11:55:34 -0300
Subject: [PATCH] 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->fdwPrivLists and reading from there.

Author: Ayush Tiwari <[email-masked]>
Author: Rafia Sabih <[email-masked]>
Co-authored-by: Matheus Alcantara <[email-masked]>
Reported-by: Chi Zhang <[email-masked]>
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..3e43c97896e 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 33a6735f08d..a631c345c5e 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->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..f64c2cc5f34 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	   *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)

---

From: Ayush Tiwari <[email-masked]>
Date: Wed, 10 Jun 2026 10:45:27 +0530

Hi,

> over the result. Attached is the combined patch.

Thanks for this!

> mt_mergeJoinConditions). Should we rename it to mt_fdwPrivLists for

I think yes, it makes sense to rename it.

Regards,
Ayush

---

From: "Matheus Alcantara" <[email-masked]>
Date: Wed, 10 Jun 2026 08:08:56 -0300

> 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
From 971d185cf4bfb75d0459f2c7848e6b77afa2ee85 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <[email-masked]>
Date: Tue, 9 Jun 2026 11:55:34 -0300
Subject: [PATCH v2] Re-index ModifyTable FDW arrays when pruning result
 relations

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.

Reviewed-by: Ayush Tiwari <[email-masked]>

 
index 112c17b0d64..a40d03d35f3 100644
+			List	   *fdw_private = (List *) list_nth(mtstate->mt_fdwPrivLists, j);
 
index 33a6735f08d..feea28214d4 100644
 
 		if (keep_rel)
 		{
 
+	mtstate->mt_fdwPrivLists = fdwPrivLists;
 
 	/*----------

 		/*
 
index 53c138310db..5871383961f 100644
 
+	/*
+	List	   *mt_fdwPrivLists;

---

From: Amit Langote <[email-masked]>
Date: Wed, 10 Jun 2026 22:09:12 +0900

Hi Matheus,

> that is when the issue started)

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

-- 
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Wed, 10 Jun 2026 22:27:22 +0900

> 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

---

From: Etsuro Fujita <[email-masked]>
Date: Thu, 11 Jun 2026 19:40:49 +0900

Amit-san,

> has any thoughts on the FDW-side concerns, since he has already chimed

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

---

From: Amit Langote <[email-masked]>
Date: Thu, 11 Jun 2026 19:44:20 +0900

Hi Fujita-san,

> commit it quickly, please go ahead.

No problem; I'll hold off.

-- 
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Thu, 18 Jun 2026 18:15:44 +0900

Fujita-san,

> 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

---

From: Etsuro Fujita <[email-masked]>
Date: Thu, 18 Jun 2026 19:50:59 +0900

Amit-san,

> unchanged. The substantive change is the executor re-indexing.

Thanks for the clarification!

---

From: Etsuro Fujita <[email-masked]>
Date: Thu, 18 Jun 2026 19:55:05 +0900

Sorry, I accidentally sent this before I finished writing.

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

---

From: Amit Langote <[email-masked]>
Date: Thu, 18 Jun 2026 19:57:07 +0900

> the results (if any) by tomorrow.  Thanks for the consideration as

Ah, thanks for the heads up.

- Amit

---

From: Etsuro Fujita <[email-masked]>
Date: Fri, 19 Jun 2026 20:59:39 +0900

Amit-san,

On Thu, Jun 18, 2026 at 7:57 PM Amit Langote <[email-masked]> wrote:
>> 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.

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

+   /*

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

---

From: Amit Langote <[email-masked]>
Date: Mon, 22 Jun 2026 17:28:00 +0900

Fujita-san,

> Here are my review comments:

Thanks a lot for the review.

> resultRelations") to a more appropriate place, and did some minor

Looks good.

> foreign modify work with runtime pruning of result relations (bug

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.

> "test writable foreign table stuff" section.

Sounds good.

> 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

---

From: Etsuro Fujita <[email-masked]>
Date: Tue, 23 Jun 2026 17:27:39 +0900

Amit-san,

Thanks!

> their existing comment too.

That's a good idea!  So +1

> Updated patch attached.

LGTM.

Best regards,
Etsuro Fujita

---

From: Amit Langote <[email-masked]>
Date: Wed, 24 Jun 2026 09:00:59 +0900

> That's a good idea!  So +1

Pushed, thanks for checking.

--
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Wed, 24 Jun 2026 10:43:32 +0900

> Pushed, thanks for checking.

crake is now red on REL_18:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2026-06-24%2000%3A02%3A03

It reports sizeof(ModifyTableState) grew by 8 bytes.

That’s from adding mt_fdwPrivLists at the end of the struct. Placing it
last keeps existing field offsets stable, which is what extensions reading
the node rely on, but it does grow the struct, which is what the checker
flags. I believe we’ve added trailing struct members in back branches
before for exactly this reason. Is a flagged sizeof increase acceptable
here under that precedent, or do we now want to avoid any flagged ABI
change on a back branch?

If we’d rather avoid it, I have a no-field alternative for REL_18:
show_modifytable_info() is the only reader of the re-indexed list, and it
can recompute the mapping from node->resultRelations and
node->fdwPrivLists, both plan-ordered and untouched by pruning. master
would keep the field as committed, so the branches would differ slightly.

I’m inclined toward the no-field version for REL_18 to keep the farm green,
unless someone would rather keep the same code in both branches and accept
the size change. I’ll hold off briefly for any thoughts before doing that.
- Amit

---

From: Tom Lane <[email-masked]>
Date: Tue, 23 Jun 2026 21:53:55 -0400

> here under that precedent, or do we now want to avoid any flagged ABI

I don't believe that the addition of ABI checking was meant to change
our back-patching policies, just to make sure we don't make
unintentional ABI changes.  As you say, we've added trailing fields
before.  The critical question is whether it's likely that any
extensions create their own ModifyTableState nodes (and might make
them too small and/or fail to fill the new field correctly).

I was about to say that that seems unlikely to me, but a check of
Debian Code Search immediately found pg_rewrite doing it.  So yeah,
looks like we'd better adopt the no-ABI-break solution.

			regards, tom lane

---

From: Richard Guo <[email-masked]>
Date: Wed, 24 Jun 2026 11:23:12 +0900

On Wed, Jun 24, 2026 at 10:43 AM Amit Langote <[email-masked]> wrote:
> If we’d rather avoid it, I have a no-field alternative for REL_18: show_modifytable_info() is the only reader of the re-indexed list, and it can recompute the mapping from node->resultRelations and node->fdwPrivLists, both plan-ordered and untouched by pruning.

This sounds like a nice solution.  Since show_modifytable_info() is
only used for EXPLAIN, I guess the recompute overhead on the
re-indexed list should be fine.

- Richard

---

From: Amit Langote <[email-masked]>
Date: Wed, 24 Jun 2026 12:50:28 +0900

> looks like we'd better adopt the no-ABI-break solution.

Ah, ok, will post the patch to do so in the REL_18_STABLE branch.

Thanks for the comment.

-- 
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Wed, 24 Jun 2026 13:25:16 +0900

> re-indexed list should be fine.

Thanks for chiming in, here is the patch to do so.

--
Thanks, Amit Langote

---

From: Etsuro Fujita <[email-masked]>
Date: Wed, 24 Jun 2026 18:15:25 +0900

Amit-san,

> Thanks for chiming in, here is the patch to do so.

Thanks for the patch!

+           /*
+            * node->fdwPrivLists is indexed by the original, pre-pruning
+            * result relation order and is parallel to node->resultRelations.
+            * Initial pruning may have dropped earlier relations, so the kept
+            * index j need not match the original position; find this
+            * relation's entry by its range table index instead.
+            */
+           forboth(lc1, node->resultRelations, lc2, node->fdwPrivLists)
+           {
+               if (lfirst_int(lc1) == (int) rti)
+               {
+                   fdw_private = (List *) lfirst(lc2);
+                   break;
+               }
+           }

I think it's good to skip this for efficiency, when there are no
pruned result relations.

Other than that the patch looks good to me.

I didn't know pg_rewrite's use of ModifyTableState, which was
different than I expected.  Sorry for that.

Best regards,
Etsuro Fujita

---

From: Amit Langote <[email-masked]>
Date: Wed, 24 Jun 2026 20:20:29 +0900

Fujita-san,

> I think it's good to skip this for efficiency, when there are no

Thanks for the review. v2 attached implements your suggestion: it
indexes node->fdwPrivLists directly with j when no result relations
were pruned, and only falls back to matching by range table index when
pruning appears to have dropped some.

> different than I expected.  Sorry for that.

No need to apologize at all. Tom's point about it was new to me too.

I will push v2 to REL_18 tomorrow barring objections.

--
Thanks, Amit Langote

---

From: Etsuro Fujita <[email-masked]>
Date: Thu, 25 Jun 2026 02:02:41 +0900

Amit-san,

> pruning appears to have dropped some.

Thank you for doing that work!

This might be nitpicking, but:

+           if (list_length(node->resultRelations) == mtstate->mt_nrels)
+               fdw_private = (List *) list_nth(node->fdwPrivLists, j);
+           else
+           {
+               Index       rti = resultRelInfo->ri_RangeTableIndex;
+               ListCell   *lc1;
+               ListCell   *lc2;
+
+               fdw_private = NIL;
+               forboth(lc1, node->resultRelations, lc2, node->fdwPrivLists)
+               {
+                   if (lfirst_int(lc1) == (int) rti)
+                   {
+                       fdw_private = (List *) lfirst(lc2);
+                       break;
+                   }
+               }
+           }

I'd put the if-test outside of the outer loop to save cycles.

Other than that v2 looks good to me.

(The forboth loop actually causes an n-squared calculation, but it's
done only when pruning occurs, in which case the number of remaining
result relations would be reduced, so that wouldn't be a problem.)

Best regards,
Etsuro Fujita

---

From: Amit Langote <[email-masked]>
Date: Thu, 25 Jun 2026 08:24:08 +0900

Fujita-san,

> I'd put the if-test outside of the outer loop to save cycles.

Right, it's loop-invariant. v3 attached computes a boolean
(nopruning), like labeltargets, once before the loop and uses it
inside.

> result relations would be reduced, so that wouldn't be a problem.)

Right, though strictly the inner forboth scans node->resultRelations,
which pruning leaves at its original length, so it's the original
relation count that bounds the scan rather than the reduced one.
Either way it's EXPLAIN-only with small counts, so it's not a concern.

Thanks again for the review.

-- 
Thanks, Amit Langote

---

From: Etsuro Fujita <[email-masked]>
Date: Thu, 25 Jun 2026 15:15:22 +0900

Amit-san,

> Either way it's EXPLAIN-only with small counts, so it's not a concern.

That's right.

The v3 patch looks good to me.  Thanks for updating the patch!

Best regards,
Etsuro Fujita

---

From: Amit Langote <[email-masked]>
Date: Thu, 25 Jun 2026 20:47:22 +0900

Fujita-san,

-- 
Thanks, Amit Langote

---

From: Amit Langote <[email-masked]>
Date: Thu, 25 Jun 2026 21:21:15 +0900

> Pushed, thanks for checking.

And crake turns green:

https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=crake&br=REL_18_STABLE

-- 
Thanks, Amit Langote