Re: Bug in asynchronous Append

Hi Gleb, all,

On Fri, Jul 10, 2026 at 7:56 PM Gleb Kashkin <g(dot)kashkin(at)postgrespro(dot)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