v19-0001-Fix-corruption-of-async-request-state-on-Append-.patch

application/octet-stream

Filename: v19-0001-Fix-corruption-of-async-request-state-on-Append-.patch
Type: application/octet-stream
Part: 0
Message: Re: Asynchronous MergeAppend

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v19-0001
Subject: Fix corruption of async request state on Append rescan
File+
contrib/postgres_fdw/expected/postgres_fdw.out 16 0
contrib/postgres_fdw/sql/postgres_fdw.sql 11 0
src/backend/executor/nodeAppend.c 34 2
From 1c3637f8b986148745ca96685d652227e7bf4bda Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sat, 4 Jul 2026 00:36:45 +0300
Subject: [PATCH v19 1/6] Fix corruption of async request state on Append
 rescan

ExecReScanAppend() unconditionally cleared callback_pending for every
async subplan in as_asyncplans, regardless of whether a request was
genuinely still in flight.  For a subplan whose remote fetch had been
sent but not yet consumed, this desynchronized our local bookkeeping
from the async-capable node's own view of the same fact -- e.g.
postgres_fdw's PgFdwConnState.pendingAreq, which tracks the
outstanding request on a possibly shared connection independently of
our AsyncRequest.callback_pending flag.

If that connection is later reused by another subplan (which happens
routinely under runtime partition pruning, once the subplan holding
the stale request is excluded from a round and a sibling sharing its
connection gets its own ReScan), postgres_fdw's pgfdw_exec_query()
drains the still-registered pendingAreq before sending a new command,
and process_pending_request() asserts that its callback_pending flag
is set.  Since we had already cleared it, this assertion fails; in a
non-assert build the connection's state is left inconsistent instead.

Fix this by leaving a still-pending request alone in
ExecReScanAppend(), and instead draining it lazily in
ExecAppendAsyncBegin(), right before the request is reused: call
ExecReScan() on the subplan there (letting the async-capable node
settle any outstanding state on its own terms) and only then reset our
bookkeeping.  This mirrors the fix already applied to MergeAppend's
async support elsewhere.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../postgres_fdw/expected/postgres_fdw.out    | 16 +++++++++
 contrib/postgres_fdw/sql/postgres_fdw.sql     | 11 ++++++
 src/backend/executor/nodeAppend.c             | 36 +++++++++++++++++--
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0805c56cb1b..55e49b7d67f 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11761,6 +11761,22 @@ SELECT * FROM result_tbl ORDER BY a;
 (3 rows)
 
 DELETE FROM result_tbl;
+-- Check that a pending async request on a shared connection isn't corrupted
+-- when the Append is rescanned with the subplan holding it pruned out.  Here
+-- async_p2 and async_p3 share a connection, and per outer row exactly one of
+-- them is pruned while async_p1 (a different connection) always matches; the
+-- inner LIMIT leaves the other shared-connection request outstanding across
+-- the rescan, and reusing that connection for the next round must drain it
+-- rather than trip over stale state.
+SELECT o.x FROM (VALUES (2505), (3505)) o(x),
+  LATERAL (SELECT a FROM async_pt WHERE a = 1505 OR a = o.x LIMIT 1) s
+ORDER BY o.x;
+  x   
+------
+ 2505
+ 3505
+(2 rows)
+
 -- Test COPY TO when foreign table is partition
 COPY async_pt TO stdout; --error
 ERROR:  cannot copy from foreign table "async_p1"
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 8162c5496bf..7aad91b0718 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -4055,6 +4055,17 @@ INSERT INTO result_tbl SELECT * FROM async_pt WHERE b === 505;
 SELECT * FROM result_tbl ORDER BY a;
 DELETE FROM result_tbl;
 
+-- Check that a pending async request on a shared connection isn't corrupted
+-- when the Append is rescanned with the subplan holding it pruned out.  Here
+-- async_p2 and async_p3 share a connection, and per outer row exactly one of
+-- them is pruned while async_p1 (a different connection) always matches; the
+-- inner LIMIT leaves the other shared-connection request outstanding across
+-- the rescan, and reusing that connection for the next round must drain it
+-- rather than trip over stale state.
+SELECT o.x FROM (VALUES (2505), (3505)) o(x),
+  LATERAL (SELECT a FROM async_pt WHERE a = 1505 OR a = o.x LIMIT 1) s
+ORDER BY o.x;
+
 -- Test COPY TO when foreign table is partition
 COPY async_pt TO stdout; --error
 
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 987358e27fa..6a5a14cd576 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -469,7 +469,21 @@ ExecReScanAppend(AppendState *node)
 		{
 			AsyncRequest *areq = node->as_asyncrequests[i];
 
-			areq->callback_pending = false;
+			/*
+			 * Leave a request that is still marked as pending a callback
+			 * alone: it may genuinely still be in flight, or it may have an
+			 * unconsumed result already sitting on a connection shared with
+			 * another subplan (as can happen with postgres_fdw).  Blindly
+			 * clearing callback_pending here would desync our bookkeeping
+			 * from the async-capable node's own, which can lead it to
+			 * mishandle that connection later (e.g. postgres_fdw asserts that
+			 * a request it still considers in-process has callback_pending
+			 * set).  Such a request is instead drained lazily, right before
+			 * it would be reused, in ExecAppendAsyncBegin().
+			 */
+			if (areq->callback_pending)
+				continue;
+
 			areq->request_complete = false;
 			areq->result = NULL;
 		}
@@ -915,7 +929,25 @@ ExecAppendAsyncBegin(AppendState *node)
 		AsyncRequest *areq = node->as_asyncrequests[i];
 
 		Assert(areq->request_index == i);
-		Assert(!areq->callback_pending);
+
+		/*
+		 * This request may still be marked as pending a callback, if
+		 * ExecReScanAppend() left it alone because it might have been
+		 * genuinely in flight (or had an unconsumed result waiting on a
+		 * connection shared with another subplan).  Drain it now, before
+		 * reusing it: ExecReScan() lets the async-capable node settle any
+		 * such outstanding state (e.g. postgres_fdw's
+		 * postgresReScanForeignScan() will wait for an in-progress request on
+		 * its own connection and consume its result), after which it's safe
+		 * to reset our own bookkeeping and issue a fresh request.
+		 */
+		if (areq->callback_pending)
+		{
+			ExecReScan(node->appendplans[i]);
+			areq->callback_pending = false;
+			areq->request_complete = false;
+			areq->result = NULL;
+		}
 
 		/* Do the actual work. */
 		ExecAsyncRequest(areq);
-- 
2.39.5 (Apple Git-154)