Bug in asynchronous Append
Hi!
ExecReScanAppend() unconditionally resets callback_pending for all
AsyncRequests. The problem is that postgres_fdw keeps its own knowledge
for the same fact: PgFdwConnState.pendingAreq – a pointer to "pending async
request" for a given connection. That connection can be shared by several
partitions/foreign tables (postgres_fdw caches one connection per
server+usermapping pair). The blind reset in nodeAppend.c only touches the
local AsyncRequest.callback_pending; it never touches
PgFdwConnState.pendingAreq, which correctly points to the still-dangling
request.
Later, when another partition sharing that same connection gets its own
ReScan (for instance, its chgParam changed because of the LATERAL
parameter, and it already has a cursor open), it sends "CLOSE cursor" via
pgfdw_exec_query(). Before sending any new command on the connection, that
function first drains whatever request is still outstanding on it:
if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);
And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix. The
fix postpones the reset of the callback_pending flag
to ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
along with ExecReScan(), which completes the async fetch.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
v1-0001-Fix-corruption-of-async-request-state-on-Append-r.patchapplication/octet-stream; name=v1-0001-Fix-corruption-of-async-request-state-on-Append-r.patchDownload+61-3
Hi Alexander,
On Sat, Jul 4, 2026 at 7:00 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
ExecReScanAppend() unconditionally resets callback_pending for all AsyncRequests. The problem is that postgres_fdw keeps its own knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer to "pending async request" for a given connection. That connection can be shared by several partitions/foreign tables (postgres_fdw caches one connection per server+usermapping pair). The blind reset in nodeAppend.c only touches the local AsyncRequest.callback_pending; it never touches PgFdwConnState.pendingAreq, which correctly points to the still-dangling request.
Later, when another partition sharing that same connection gets its own ReScan (for instance, its chgParam changed because of the LATERAL parameter, and it already has a cursor open), it sends "CLOSE cursor" via pgfdw_exec_query(). Before sending any new command on the connection, that function first drains whatever request is still outstanding on it:
if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix. The fix postpones the reset of the callback_pending flag to ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup along with ExecReScan(), which completes the async fetch.
Interesting! Thanks for the report and patch! Will review.
Best regards,
Etsuro Fujita
Alexander Korotkov писал(а) 2026-07-04 01:00:
Hi!
ExecReScanAppend() unconditionally resets callback_pending for all
AsyncRequests. The problem is that postgres_fdw keeps its own
knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer
to "pending async request" for a given connection. That connection
can be shared by several partitions/foreign tables (postgres_fdw
caches one connection per server+usermapping pair). The blind reset in
nodeAppend.c only touches the local AsyncRequest.callback_pending; it
never touches PgFdwConnState.pendingAreq, which correctly points to
the still-dangling request.Later, when another partition sharing that same connection gets its
own ReScan (for instance, its chgParam changed because of the LATERAL
parameter, and it already has a cursor open), it sends "CLOSE cursor"
via pgfdw_exec_query(). Before sending any new command on the
connection, that function first drains whatever request is still
outstanding on it:if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix.
The fix postpones the reset of the callback_pending flag to
ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
along with ExecReScan(), which completes the async fetch.
Hi. The analysis seems correct to me as well as fix.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Alexander Korotkov писал(а) 2026-07-04 01:00:
Hi!
ExecReScanAppend() unconditionally resets callback_pending for all
AsyncRequests. The problem is that postgres_fdw keeps its own
knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer
to "pending async request" for a given connection. That connection
can be shared by several partitions/foreign tables (postgres_fdw
caches one connection per server+usermapping pair). The blind reset in
nodeAppend.c only touches the local AsyncRequest.callback_pending; it
never touches PgFdwConnState.pendingAreq, which correctly points to
the still-dangling request.Later, when another partition sharing that same connection gets its
own ReScan (for instance, its chgParam changed because of the LATERAL
parameter, and it already has a cursor open), it sends "CLOSE cursor"
via pgfdw_exec_query(). Before sending any new command on the
connection, that function first drains whatever request is still
outstanding on it:if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix.
The fix postpones the reset of the callback_pending flag to
ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
along with ExecReScan(), which completes the async fetch.------
Regards,
Alexander Korotkov
Supabase
Hi!
It seems, that there is a major issue with the patch, the draining
doesn't
throw away the result in cases when it is required. Consider this case:
-- Expose stale rows from a pending async request that was valid for a
-- previous rescan but is pruned out for the current one.
CREATE VIEW base_tbl2_slow AS
SELECT t.a, t.b, t.c
FROM base_tbl2 t, LATERAL pg_sleep(0.2);
ALTER FOREIGN TABLE async_p2 OPTIONS (SET table_name 'base_tbl2_slow');
SELECT o.x, s.a
FROM (VALUES (2505), (3505)) o(x),
LATERAL (
SELECT a
FROM async_pt
WHERE a = o.x OR (o.x = 2505 AND a = 1505)
LIMIT 1
) s
ORDER BY o.x;
x | a
------+------
2505 | 1505
3505 | 2505
(2 rows)
1) Append executes two async fscans, one of which is slow
2) The first fscan finishes and the append execution stops due to LIMIT
1
3) Second fscan async request is still pending
4) Next append rescan prunes async_p2 for new outer value
5) Old async request for async_p2 is allowed to drain (receive a
callback)
6) The stale tuple from async_p2 is accepted by Append, no
pruning/filter check
7) We get a row that should be impossible (3505 | 2505) in this query
See full reproducer in the attached patch.
Attachments:
v1-regress.patchtext/x-diff; name=v1-regress.patchDownload+82-2
Hi Gleb, all,
On Fri, Jul 10, 2026 at 7:56 PM Gleb Kashkin <g.kashkin@postgrespro.ru> wrote:
Alexander Korotkov писал(а) 2026-07-04 01:00:
ExecReScanAppend() unconditionally resets callback_pending for all
AsyncRequests. The problem is that postgres_fdw keeps its own
knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer
to "pending async request" for a given connection. That connection
can be shared by several partitions/foreign tables (postgres_fdw
caches one connection per server+usermapping pair). The blind reset in
nodeAppend.c only touches the local AsyncRequest.callback_pending; it
never touches PgFdwConnState.pendingAreq, which correctly points to
the still-dangling request.Later, when another partition sharing that same connection gets its
own ReScan (for instance, its chgParam changed because of the LATERAL
parameter, and it already has a cursor open), it sends "CLOSE cursor"
via pgfdw_exec_query(). Before sending any new command on the
connection, that function first drains whatever request is still
outstanding on it:if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix.
The fix postpones the reset of the callback_pending flag to
ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
along with ExecReScan(), which completes the async fetch.
It seems, that there is a major issue with the patch, the draining
doesn't
throw away the result in cases when it is required. Consider this case:-- Expose stale rows from a pending async request that was valid for a
-- previous rescan but is pruned out for the current one.
CREATE VIEW base_tbl2_slow AS
SELECT t.a, t.b, t.c
FROM base_tbl2 t, LATERAL pg_sleep(0.2);
ALTER FOREIGN TABLE async_p2 OPTIONS (SET table_name 'base_tbl2_slow');
SELECT o.x, s.a
FROM (VALUES (2505), (3505)) o(x),
LATERAL (
SELECT a
FROM async_pt
WHERE a = o.x OR (o.x = 2505 AND a = 1505)
LIMIT 1
) s
ORDER BY o.x;
x | a
------+------
2505 | 1505
3505 | 2505
(2 rows)1) Append executes two async fscans, one of which is slow
2) The first fscan finishes and the append execution stops due to LIMIT
1
3) Second fscan async request is still pending
4) Next append rescan prunes async_p2 for new outer value
5) Old async request for async_p2 is allowed to drain (receive a
callback)
6) The stale tuple from async_p2 is accepted by Append, no
pruning/filter check
7) We get a row that should be impossible (3505 | 2505) in this querySee full reproducer in the attached patch.
Thanks for sharing this counter example!
Alexander, sorry to say this, but the idea of delaying the processing
of async requests still pending until ExecAppendAsyncBegin is
completely wrong; that breaks the correctness, as demonstrated by the
example. To guarantee it, I think we should process such requests
*during* ExecReScanAppend, like the attached. For that, I added a new
function ExecAppendAsyncProcessPending, which wouldn't be efficient in
some cases; I think we could optimize the function, but I couldn't
come up with ideas for doing so in a back-patchable way, so I'd like
to leave that for v20. Will work. (In typical cases where the LIMIT
clause isn't included, all of the async subplans would be drained
until ExecReScanAppend, so I don't think that the function would cause
any noticeable overhead.)
This would be my fault. Thanks to all of you!
Best regards,
Etsuro Fujita
Attachments:
Fix-bug-in-async-append-efujita.patchapplication/octet-stream; name=Fix-bug-in-async-append-efujita.patchDownload+72-1
Hi, Etsuro!
On Wed, Jul 15, 2026 at 2:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
Hi Gleb, all,
On Fri, Jul 10, 2026 at 7:56 PM Gleb Kashkin <g.kashkin@postgrespro.ru> wrote:
Alexander Korotkov писал(а) 2026-07-04 01:00:
ExecReScanAppend() unconditionally resets callback_pending for all
AsyncRequests. The problem is that postgres_fdw keeps its own
knowledge for the same fact: PgFdwConnState.pendingAreq – a pointer
to "pending async request" for a given connection. That connection
can be shared by several partitions/foreign tables (postgres_fdw
caches one connection per server+usermapping pair). The blind reset in
nodeAppend.c only touches the local AsyncRequest.callback_pending; it
never touches PgFdwConnState.pendingAreq, which correctly points to
the still-dangling request.Later, when another partition sharing that same connection gets its
own ReScan (for instance, its chgParam changed because of the LATERAL
parameter, and it already has a cursor open), it sends "CLOSE cursor"
via pgfdw_exec_query(). Before sending any new command on the
connection, that function first drains whatever request is still
outstanding on it:if (state && state->pendingAreq)
process_pending_request(state->pendingAreq);And process_pending_request() starts with:
Assert(areq->callback_pending);
– which fails, because the flag was corrupted some rounds earlier.
The attached patch contains both the reproduction case and the fix.
The fix postpones the reset of the callback_pending flag to
ExecAppendAsyncBegin(). ExecAppendAsyncBegin() performs this cleanup
along with ExecReScan(), which completes the async fetch.It seems, that there is a major issue with the patch, the draining
doesn't
throw away the result in cases when it is required. Consider this case:-- Expose stale rows from a pending async request that was valid for a
-- previous rescan but is pruned out for the current one.
CREATE VIEW base_tbl2_slow AS
SELECT t.a, t.b, t.c
FROM base_tbl2 t, LATERAL pg_sleep(0.2);
ALTER FOREIGN TABLE async_p2 OPTIONS (SET table_name 'base_tbl2_slow');
SELECT o.x, s.a
FROM (VALUES (2505), (3505)) o(x),
LATERAL (
SELECT a
FROM async_pt
WHERE a = o.x OR (o.x = 2505 AND a = 1505)
LIMIT 1
) s
ORDER BY o.x;
x | a
------+------
2505 | 1505
3505 | 2505
(2 rows)1) Append executes two async fscans, one of which is slow
2) The first fscan finishes and the append execution stops due to LIMIT
1
3) Second fscan async request is still pending
4) Next append rescan prunes async_p2 for new outer value
5) Old async request for async_p2 is allowed to drain (receive a
callback)
6) The stale tuple from async_p2 is accepted by Append, no
pruning/filter check
7) We get a row that should be impossible (3505 | 2505) in this querySee full reproducer in the attached patch.
Thanks for sharing this counter example!
Alexander, sorry to say this, but the idea of delaying the processing
of async requests still pending until ExecAppendAsyncBegin is
completely wrong; that breaks the correctness, as demonstrated by the
example. To guarantee it, I think we should process such requests
*during* ExecReScanAppend, like the attached. For that, I added a new
function ExecAppendAsyncProcessPending, which wouldn't be efficient in
some cases; I think we could optimize the function, but I couldn't
come up with ideas for doing so in a back-patchable way, so I'd like
to leave that for v20. Will work. (In typical cases where the LIMIT
clause isn't included, all of the async subplans would be drained
until ExecReScanAppend, so I don't think that the function would cause
any noticeable overhead.)This would be my fault. Thanks to all of you!
No worries. Thank you for an update and thank you for your patch.
I'm going to push (and backpatch to PG 14 where async append was
introduced) it if no objections.
------
Regards,
Alexander Korotkov
Supabase
Hi Alexander,
On Sun, Jul 19, 2026 at 1:00 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
No worries. Thank you for an update and thank you for your patch.
I'm going to push (and backpatch to PG 14 where async append was
introduced) it if no objections.
Thanks, but this is my fault, so is it possible for me to do so
myself? Anyway, I think the patch will need a bit more work.
Best regards,
Etsuro Fujita
On Sat, Jul 18, 2026 at 10:06 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
On Sun, Jul 19, 2026 at 1:00 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
No worries. Thank you for an update and thank you for your patch.
I'm going to push (and backpatch to PG 14 where async append was
introduced) it if no objections.Thanks, but this is my fault, so is it possible for me to do so
myself? Anyway, I think the patch will need a bit more work.
Sure, please go ahead.
------
Regards,
Alexander Korotkov
Supabase
On Sun, Jul 19, 2026 at 4:16 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
On Sat, Jul 18, 2026 at 10:06 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
On Sun, Jul 19, 2026 at 1:00 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
No worries. Thank you for an update and thank you for your patch.
I'm going to push (and backpatch to PG 14 where async append was
introduced) it if no objections.Thanks, but this is my fault, so is it possible for me to do so
myself? Anyway, I think the patch will need a bit more work.Sure, please go ahead.
Thanks, will do. I have some other priorities in the coming two
weeks, so I'm planning to work on the patch a bit more and
push/backpatch it early next month in time for the August releases.
Best regards,
Etsuro Fujita