From 1c3637f8b986148745ca96685d652227e7bf4bda Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Sat, 4 Jul 2026 00:36:45 +0300 Subject: [PATCH v1] 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) --- .../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)