Hi! On Mon, Jul 6, 2026 at 4:42 PM Alexander Pyhalov <[email protected]> wrote: > Alexander Korotkov писал(а) 2026-07-04 02:05: > > > While discovering a correctness of callback_pending flag reset in > > async MergeAppend, I found bug in async plain Append [1]. I've > > included the fix as 0001 in the current patchset. Other changes to > > patchset includes. > > > > I expect that this issue can affect both MergeAppend and Append, but > current test cases don't confirm this. > If we revert to the old behavior (setting areq->callback_pending to > false), the tests results of Merge Append tests > are not changed.
Did you try the test case showed by Gleb [1]. Yet I think it's safe to follow the fix by Etsuro. In the revised patchset I put this into ExecAppendBaseAsyncProcessPending() and use for both async append and async merge append. > > 0003 contains some cleanups > > * Unify set_append_references and set_mergeappend_references > > (setrefs.c) > > * Merge duplicate Append/MergeAppend cases in explain.c > > * Merge duplicate Append/MergeAppend cases in > > planstate_tree_walker_impl (nodeFuncs.c) > > This looks good. > > > 0006 includes following optimizations and fixes > > * Fix assertion failure on MergeAppend rescan with an in-flight async > > request (same as 0001 but for MergeAppend) > > > > * Don't use ExecProcNode() for async subplans. Despite its > > effectiveness, it doesn't works correctly. When postgres_fdw subplan > > is executed by ExecProcNode() it interprets the end of async batch as > > end of the whole data. That effectively leads to skipping the > > remaining dataset after first batch (100 rows). The test is added. > > Ouch. Luckily we've catched this. postgresIterateForeignScan() doesn't > fetch tuples for async scan states... > > > * Make ExecReScanMergeAppend() clear ms_slots. Otherwise subsequent > > scans can use leftover tuples. The test is also added. > > This seems to happen only in updated version of the patch, where > ExecMergeAppendAsyncGetNext() relies on the fact that > async requests have been already sent (previously this function firstly > set slot to NULL prior to sending requests and processing them). Yes, thank you for clarification. Are you good with this approach? > > * Document that the needrequest fast-skip is a no-op for MergeAppend > > (postgres_fdw.c) > > * Document why create_merge_append_plan discards > > mark_async_capable_plan's result (createplan.c) > > > Looks good. > > ExecMergeAppendGetNextSlot() - I'd sligtly prefer to check if mplan is > member of as_asyncplans and assert that it's a member of > node->as.valid_asyncplans > in this case, but I think it doesn't matter much. OK, I changed to this way. Links 1. https://www.postgresql.org/message-id/3a3f949c611835620cf675bdb989b1e7%40postgrespro.ru ------ Regards, Alexander Korotkov Supabase
