Skip to content

Commit

Permalink
Fix parallel query so it doesn't spoil row estimates above Gather.
Browse files Browse the repository at this point in the history
Commit 45be99f removed GatherPath's
num_workers field, but this is entirely bogus.  Normally, a path's
parallel_workers flag is supposed to indicate the number of workers
that it wants, and should be 0 for a non-partial path.  In that
commit, I mistakenly thought that GatherPath could also use that field
to indicate the number of workers that it would try to start, but
that's disastrous, because then it can propagate up to higher nodes in
the plan tree, which will then get incorrect rowcounts because the
parallel_workers flag is involved in computing those values.  Repair
by putting the separate field back.

Report by Tomas Vondra.  Patch by me, reviewed by Amit Kapila.

Discussion: http://postgr.es/m/f91b4a44-f739-04bd-c4b6-f135bd643669@2ndquadrant.com
  • Loading branch information
robertmhaas committed Apr 1, 2017
1 parent 9b6e8d8 commit fb1879c
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/backend/nodes/outfuncs.c
Expand Up @@ -1795,6 +1795,7 @@ _outGatherPath(StringInfo str, const GatherPath *node)

WRITE_NODE_FIELD(subpath);
WRITE_BOOL_FIELD(single_copy);
WRITE_INT_FIELD(num_workers);
}

static void
Expand Down
2 changes: 1 addition & 1 deletion src/backend/optimizer/plan/createplan.c
Expand Up @@ -1395,7 +1395,7 @@ create_gather_plan(PlannerInfo *root, GatherPath *best_path)

gather_plan = make_gather(tlist,
NIL,
best_path->path.parallel_workers,
best_path->num_workers,
best_path->single_copy,
subplan);

Expand Down
7 changes: 4 additions & 3 deletions src/backend/optimizer/util/pathnode.c
Expand Up @@ -1681,16 +1681,17 @@ create_gather_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
required_outer);
pathnode->path.parallel_aware = false;
pathnode->path.parallel_safe = false;
pathnode->path.parallel_workers = subpath->parallel_workers;
pathnode->path.parallel_workers = 0;
pathnode->path.pathkeys = NIL; /* Gather has unordered result */

pathnode->subpath = subpath;
pathnode->num_workers = subpath->parallel_workers;
pathnode->single_copy = false;

if (pathnode->path.parallel_workers == 0)
if (pathnode->num_workers == 0)
{
pathnode->path.parallel_workers = 1;
pathnode->path.pathkeys = subpath->pathkeys;
pathnode->num_workers = 1;
pathnode->single_copy = true;
}

Expand Down
1 change: 1 addition & 0 deletions src/include/nodes/relation.h
Expand Up @@ -1189,6 +1189,7 @@ typedef struct GatherPath
Path path;
Path *subpath; /* path for each worker */
bool single_copy; /* path must not be executed >1x */
int num_workers; /* number of workers sought to help */
} GatherPath;

/*
Expand Down

0 comments on commit fb1879c

Please sign in to comment.