Bug in asynchronous Append

Started by Alexander Korotkov22 days ago9 messageshackers
Jump to latest
#1Alexander Korotkov
aekorotkov@gmail.com

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
#2Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Alexander Korotkov (#1)
Re: Bug in asynchronous Append

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

#3Alexander Pyhalov
a.pyhalov@postgrespro.ru
In reply to: Alexander Korotkov (#1)
Re: Bug in asynchronous Append

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

#4Gleb Kashkin
g.kashkin@postgrespro.ru
In reply to: Alexander Korotkov (#1)
Re: Bug in asynchronous Append

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
#5Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Gleb Kashkin (#4)
Re: Bug in asynchronous Append

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 query

See 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
#6Alexander Korotkov
aekorotkov@gmail.com
In reply to: Etsuro Fujita (#5)
Re: Bug in asynchronous Append

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 query

See 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

#7Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Alexander Korotkov (#6)
Re: Bug in asynchronous Append

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

#8Alexander Korotkov
aekorotkov@gmail.com
In reply to: Etsuro Fujita (#7)
Re: Bug in asynchronous Append

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

#9Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Alexander Korotkov (#8)
Re: Bug in asynchronous Append

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