MIME-Version: 1.0
Date: Wed, 28 Oct 2026 14:37:35 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Matheus Alcantara
 <matheusssilv97@gmail.com>, Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Asynchronous MergeAppend
In-Reply-To: 
 <CAPmGK15=KctsePnvKMNoYm3JOpuiZRWfWEfMs1GaBk9Rj45c5w@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>
 <e8d502937d94eb6bd18e38ff1f240a05@postgrespro.ru>
 <CAPmGK14i=ss35hmf5bVM1BAS=prc8QM_nwg5rzjrjUvYE1R4Zw@mail.gmail.com>
 <f207ff5cb0d48e0cc58da679095c5e04@postgrespro.ru>
 <CAPmGK17tsf5zm5EeXaJipak1K4K=5hsfRD3D5HFBpu+cpb+7vw@mail.gmail.com>
 <CAPpHfduW0nmtxBddkJTNheQvASr-i84hybauv+wt9d==CjCdhg@mail.gmail.com>
 <CAPmGK178wmY-w+vSEQWBB4DBww=uwNbPx9s_iOgSR8-uz4kvZw@mail.gmail.com>
 <CAPpHfdt0pk2hmFcj+UZe=mU4F3mSKmcxn2iNWKWtSd++1X9JZA@mail.gmail.com>
 <CAPmGK15hqzy8Vm8i_C1_3bHC4ZHMcmM+rbOJgC-W=1X_gNoPKQ@mail.gmail.com>
 <CAPmGK16hN2BtbOyT6cO7J9skH7qF0HxFAO68NjkNCMN8C3orCw@mail.gmail.com>
 <CAPmGK15=KctsePnvKMNoYm3JOpuiZRWfWEfMs1GaBk9Rj45c5w@mail.gmail.com>
Message-ID: <a44e9ead6205f575da3df01b8c964e73@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/a44e9ead6205f575da3df01b8c964e73%40postgrespro.ru>
Precedence: bulk

Etsuro Fujita писал(а) 2026-10-27 14:34:
> On Thu, Oct 15, 2026 at 8:34 PM Etsuro Fujita <etsuro.fujita@gmail.com> 
> wrote:
>> I've already started reworking on Alexander
>> Pyhalov's original patch for that.  I will post a reworked version as
>> soon as possible.
> 
> Here is an updated version of the patch, which I created with AI
> assistance.  In the patch I removed the changes made to both the
> MergeAppendState struct and nodeMergeAppend.c so that async execution
> of MergeAppend works (almost) the same as that of Append, and then
> modified them in a more optimized way, which makes the patch pretty
> small/simple.

Hi.

I've looked through the updated patch.
It looks good, but I've found some misprints in comments:

1) postgres_fdw.c - you have a typo here
8708                 /*
8709                  * Otheewise, process the in-progress request, 
begin an asynchronous
8710                  * data fetch for the given request, and configure 
the file descriptor
8711                  * event below.
8712                  */
8713                 process_pending_request(pendingAreq);
8714                 fetch_more_data_begin(areq);

....
8718                 /*
8719                  * This is the case when an asynchronous request 
made by the
8720                  * requestor for another subplan is currently 
in-progress.  Skip the
8721                  * configuration even in the case where the 
requestor is MergeAppend;
8722                  * in which csse we just defer the configuration 
until the preceding
8723                  * request(s) are processed by 
ExecMergeAppendAsyncDrain.
8724                  */
8725                 return;

s/csse/case/

2) nodeMergeAppend.c:

199         if (mergestate->ms_nasyncplans > 0)
200         {
201                 mergestate->ms_asyncrequests = (AsyncRequest **)
202                         palloc0(nplans * sizeof(AsyncRequest *));
203

We could use palloc0_array as in surrounding code.


3) One more issue is that we change current semantics of 
ForeignAsyncConfigureWait - out-of-tree FDWs
now should handle MergeAppendState in addition to AppendState as 
requestor. Should we at least document it?

> 
> Some notes about the patch:
> 
> * As for the struct, I removed from it ms_asyncresults,
> ms_has_asyncresults, ms_asyncremian, ms_needrequest,
> ms_valid_subplans_identified and ms_valid_asyncplans; and instead
> added to it new member ms_asyncpending, which doesn't exist in
> AppendState.  As for the source file, I modified it based on the new
> data struct, reusing some of the code in the original version.
> * I adjusted the change made to create_merge_append_plan(), which
> makes the original 0001 patch useless, so I removed it from the patch.
> * I deduplicated the code in ExecAppendAsyncEventWait() and
> ExecMergeAppendAsyncEventWait() as mentioned upthread.  I didn't do so
> for other functions/parts, though, because I don't strongly feel the
> need for that.  (I didn't include any of the proposed refactoring,
> either, for the reason I mentioned upthread.)
> * I modified postgresForeignAsyncConfigureWait() a bit further to work
> better with async MergeAppend, and rewrote comments there.
> * I did some editorialization as well, including renaming/relocating
> functions like GetAppendEventSet() in the original version to match
> other functions.
> * I used the regression tests in the v22 patch as-proposed.  I think
> some comments should be updated, but I haven't done that yet.
> 
> What do you think about that?  Maybe I'm missing something, though.
> 


-- 
Best regards,
Alexander Pyhalov,
Postgres Professional