MIME-Version: 1.0
Date: Mon, 03 Aug 2026 09:41:33 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Matheus Alcantara <matheusssilv97@gmail.com>, Pgsql Hackers
 <pgsql-hackers@postgresql.org>
Subject: Re: Asynchronous MergeAppend
In-Reply-To: 
 <CAPpHfdsAL9oXrSP25c1XYS-ew0doUST=FU3LWj6aEG0iDw2rSA@mail.gmail.com>
References: <59be194c5a409fb9fc9f2031581b8a44@postgrespro.ru>
 <2fb1d9923b6995492e7b163e6cb95402@postgrespro.ru>
 <DE662JHGRO2O.3KJBBG2R3IT17@gmail.com>
 <efbfbcf00b6b790e8f80c13a83417a23@postgrespro.ru>
 <CAFY6G8d3Yvxa_kRQA24BsJhwqfmSCv1ujiv_7b6g5isf-ZTs_Q@mail.gmail.com>
 <b546d8bd5e2218f4f917fda4c93ead21@postgrespro.ru>
 <DED05POJZS2W.2EZ60AOBMDDAE@gmail.com>
 <bcdefce7e3db9566d0619ad2eed2a299@postgrespro.ru>
 <DF0RCOYB19RC.35BW2FHPO4N4S@gmail.com>
 <a16423ca928cde3650bbc219982a5ffc@postgrespro.ru>
 <DF28LV8UTUOO.71JUZDH1N8F9@gmail.com>
 <d5c0bfca6ee15d62df1ba72e540b4ddf@postgrespro.ru>
 <DFAQTF8P9AU0.2ZELZFXOCD0XL@gmail.com>
 <c264abe661f72d6627f987fc9aa4e456@postgrespro.ru>
 <DFBN6K9K9IUG.23WZ7OBL8DRAQ@gmail.com>
 <c0d55d3f68bfd5650fe3143d3a31c84c@postgrespro.ru>
 <782a968c8e01ec6db3b2da2120adf73b@postgrespro.ru>
 <554a73f7ffea8b22b3f81a4804b5fc34@postgrespro.ru>
 <e13717179328abed3e189f4ca1e63087@postgrespro.ru>
 <CAPpHfdu+3Eud0CBpdFT+osWiT=e=zOQUtBsx8Z5okKrqhgVAJg@mail.gmail.com>
 <42a9a941-e768-4fd8-8067-12958c5a1d70@gmail.com>
 <CAPpHfdsO8zYpDW==D6T5N0cJ+AzK7a_OyXJoYU1kFi=xZFTLuQ@mail.gmail.com>
 <DHMH23M7UOFS.12W6PUDI1I3NH@gmail.com>
 <CAPpHfdssDymMVohXw5XwdPoJ9UvSv1iN2zRcmsEhhfXXks01Zg@mail.gmail.com>
 <10c97af0ce34ebdb81708b1c49ed6038@postgrespro.ru>
 <fa7ae004ec282fde84c29f36e0c85979@postgrespro.ru>
 <CAPpHfdtNOvgKfUFLY84dLtT=m17d=976duxUvvhYGHkLhN8GAw@mail.gmail.com>
 <af6f5a17e291169f5929744acc8ddd3d@postgrespro.ru>
 <CAPpHfdsAL9oXrSP25c1XYS-ew0doUST=FU3LWj6aEG0iDw2rSA@mail.gmail.com>
Message-ID: <e8d502937d94eb6bd18e38ff1f240a05@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/e8d502937d94eb6bd18e38ff1f240a05%40postgrespro.ru>
Precedence: bulk

Alexander Korotkov писал(а) 2026-08-01 00:03:
> Hi!
> 
> On Mon, Jul 6, 2026 at 4:42 PM Alexander Pyhalov
> <a.pyhalov@postgrespro.ru> 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.

Yes, he found this case when we tested your original fix for 
ExecReScanAppend() behavior.
I'm fine with following  Etsuro's fix.

> 
>> > 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?

Yes, I'm fine with this.
> 
>> >  * 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.

It seems you've missed the attachment.

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional