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


Reply via email to