Skip to content

Commit 04e9678

Browse files
tglsfdctglsfdc
committed
Code review for nodeGatherMerge.c.
Comment the fields of GatherMergeState, and organize them a bit more sensibly. Comment GMReaderTupleBuffer more usefully too. Improve assorted other comments that were obsolete or just not very good English. Get rid of the use of a GMReaderTupleBuffer for the leader process; that was confusing, since only the "done" field was used, and that in a way redundant with need_to_scan_locally. In gather_merge_init, avoid calling load_tuple_array for already-known-exhausted workers. I'm not sure if there's a live bug there, but the case is unlikely to be well tested due to timing considerations. Remove some useless code, such as duplicating the tts_isempty test done by TupIsNull. Remove useless initialization of ps.qual, replacing that with an assertion that we have no qual to check. (If we did, the code would fail to check it.) Avoid applying heap_copytuple to a null tuple. While that fails to crash, it's confusing and it makes the code less legible not more so IMO. Propagate a couple of these changes into nodeGather.c, as well. Back-patch to v10, partly because of the possibility that the gather_merge_init change is fixing a live bug, but mostly to keep the branches in sync to ease future bug fixes.
1 parent 41b0dd9 commit 04e9678

File tree

3 files changed

+138
-114
lines changed

3 files changed

+138
-114
lines changed

‎src/backend/executor/nodeGather.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ ExecInitGather(Gather *node, EState *estate, int eflags)
7171
gatherstate->ps.plan = (Plan *) node;
7272
gatherstate->ps.state = estate;
7373
gatherstate->ps.ExecProcNode = ExecGather;
74+
75+
gatherstate->initialized = false;
7476
gatherstate->need_to_scan_locally = !node->single_copy;
7577
gatherstate->tuples_needed = -1;
7678

@@ -82,10 +84,10 @@ ExecInitGather(Gather *node, EState *estate, int eflags)
8284
ExecAssignExprContext(estate, &gatherstate->ps);
8385

8486
/*
85-
* initialize child expressions
87+
* Gather doesn't support checking a qual (it's always more efficient to
88+
* do it in the child node).
8689
*/
87-
gatherstate->ps.qual =
88-
ExecInitQual(node->plan.qual, (PlanState *) gatherstate);
90+
Assert(!node->plan.qual);
8991

9092
/*
9193
* tuple table initialization
@@ -169,15 +171,16 @@ ExecGather(PlanState *pstate)
169171
*/
170172
pcxt = node->pei->pcxt;
171173
LaunchParallelWorkers(pcxt);
174+
/* We save # workers launched for the benefit of EXPLAIN */
172175
node->nworkers_launched = pcxt->nworkers_launched;
176+
node->nreaders = 0;
177+
node->nextreader = 0;
173178

174179
/* Set up tuple queue readers to read the results. */
175180
if (pcxt->nworkers_launched > 0)
176181
{
177-
node->nreaders = 0;
178-
node->nextreader = 0;
179-
node->reader =
180-
palloc(pcxt->nworkers_launched * sizeof(TupleQueueReader *));
182+
node->reader = palloc(pcxt->nworkers_launched *
183+
sizeof(TupleQueueReader *));
181184

182185
for (i = 0; i < pcxt->nworkers_launched; ++i)
183186
{
@@ -316,8 +319,8 @@ gather_readnext(GatherState *gatherstate)
316319
tup = TupleQueueReaderNext(reader, true, &readerdone);
317320

318321
/*
319-
* If this reader is done, remove it. If all readers are done, clean
320-
* up remaining worker state.
322+
* If this reader is done, remove it, and collapse the array. If all
323+
* readers are done, clean up remaining worker state.
321324
*/
322325
if (readerdone)
323326
{

0 commit comments

Comments
 (0)