| From: | Gleb Kashkin <g(dot)kashkin(at)postgrespro(dot)ru> |
|---|---|
| To: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Bug in asynchronous Append |
| Date: | 2026-07-10 10:56:46 |
| Message-ID: | 3a3f949c611835620cf675bdb989b1e7@postgrespro.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
| Attachment | Content-Type | Size |
|---|---|---|
| v1-regress.patch | text/x-diff | 4.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nisha Moond | 2026-07-10 11:33:43 | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Previous Message | Bohyun Lee | 2026-07-10 10:41:56 | [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically |