Etsuro Fujita писал(а) 2026-10-27 14:34:
On Thu, Oct 15, 2026 at 8:34 PM Etsuro Fujita <[email protected]>
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
