... I think we all agree that we should get rid of nreaders from the GatherMergeState and need to do some code re-factor. But if I understood correctly that Robert's concern was to do that re-factor as separate commit.
Maybe. It depends on how valuable it's to keep Gather and GatherMerge similar - having nreaders in one and not the other seems a bit weird. But maybe the refactoring would remove it from both nodes?
Also, it does not really solve the issue that we're using 'nreaders' or 'nworkers_launched' to access array with one extra element.
With the latest patches, into gather_merge_init() there is one loop where we
initialize the tuple array (which is to hold the tuple came from the each worker)
and slot (which is to hold the current tuple slot).
We hold the tuple array for the worker because when we read tuples from workers, it's a good idea to read several at once for efficiency when possible: this minimizes context-switching overhead.
For leader we don't initialize the slot because for leader we directly call then ExecProcNode(), which returns the slot. Also there is no point in holding the tuple array for the leader.
With the latest patch's, gather_merge_clear_slots() will only clean the tuple table
slot and the tuple array buffer for the workers (nworkers_launched).
Only time we loop through the "nworkers_launched + 1" is while reading the tuple
from the each gather merge input. Personally I don't see any problem with that,
but I am very much open for suggestions.
regards
-- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
