MIME-Version: 1.0
Date: Tue, 23 Dec 2025 11:50:39 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: Alena Rybakina <a.rybakina@postgrespro.ru>, Pgsql Hackers
 <pgsql-hackers@postgresql.org>
Subject: Re: Asynchronous MergeAppend
In-Reply-To: <DF28LV8UTUOO.71JUZDH1N8F9@gmail.com>
References: <59be194c5a409fb9fc9f2031581b8a44@postgrespro.ru>
 <764dd8b8-6374-4f5a-aac7-d8e3f6ebe5fd@postgrespro.ru>
 <159b591411bb2c81332018927acbd509@postgrespro.ru>
 <DDZ2ULUYDQJ4.MXMP02V4GIG@gmail.com>
 <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>
Message-ID: <d5c0bfca6ee15d62df1ba72e540b4ddf@postgrespro.ru>
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/d5c0bfca6ee15d62df1ba72e540b4ddf%40postgrespro.ru>
Precedence: bulk

Matheus Alcantara писал(а) 2025-12-19 16:45:
> On Thu Dec 18, 2025 at 6:56 AM -03, Alexander Pyhalov wrote:
>>> +	noccurred = WaitEventSetWait(node->ms_eventset, -1 /* no timeout */ 
>>> ,
>>> occurred_event,
>>> +								 nevents, WAIT_EVENT_APPEND_READY);
>>> 
>>> Should we use the same WAIT_EVENT_APPEND_READY or create a new wait
>>> event for merge append?
>> 
>> I'm not sure that new wait event is needed - for observability I think
>> it's not critical
>> to distinguish Append and MergeAppend when they waited for foreign
>> scans.  But also it's perhaps
>> doesn't do any harm to record specific wait event.
>> 
> Ok, I think that we can keep this way for now and let's see if a new
> wait event is really needed.
> 
>>> I've created Appender and AppenderState types that are used by
>>> Append/MergeAppend and AppendState/MergeAppendState respectively (I
>>> should have think in a better name for these base type, ideas are
>>> welcome). The execAppend.c was created to have the functions that can
>>> be
>>> reused by Append and MergeAppend execution. I've tried to remove
>>> duplicated code blocks that was almost the same and that didn't 
>>> require
>>> much refactoring.
>> 
>> Overall I like new Appender node. Splitting code in this way really
>> helps to avoid code duplication.
>> However, some similar code is still needed, because logic of getting 
>> new
>> tuples is different.
>> 

Hi.

I've looked through updated patch. Tested it (also with our fdw). 
Overall looks good.

In execAppend.c there's still reference to as_valid_subplans. Also we 
could perhaps use palloc0_array() in some more places, for example, for 
for state->asyncrequests and state->asyncresults.
-- 
Best regards,
Alexander Pyhalov,
Postgres Professional