Fix parallel query so it doesn't spoil row estimates above Gather.
authorRobert Haas
Sat, 1 Apr 2017 01:01:20 +0000 (21:01 -0400)
committerRobert Haas
Sat, 1 Apr 2017 01:10:30 +0000 (21:10 -0400)
Commit 45be99f8cd5d606086e0a458c9c72910ba8a613d 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

src/backend/nodes/outfuncs.c
src/backend/optimizer/plan/createplan.c
src/backend/optimizer/util/pathnode.c
src/include/nodes/relation.h

index 0ccd75b74a81b48e817d0048b9b236dc1bd3085e..f8d43dbf993ac94d6efdad0854f61f4e4525965a 100644 (file)
@@ -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
index 427eedea91e0336ff6a5740677d7674684b76bfe..71678d08dcc3fc9963346cc5d0c71637b69f2bf7 100644 (file)
@@ -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);
 
index ce7ad545a95fcebc008f6a5254eaff3b1ac0d574..fc22951e68ffc17db1bb2075e90d43034d1ebb62 100644 (file)
@@ -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;
    }
 
index b97b700ee45bff62e61cc258c87db640340494f0..48862d93aec5abbf27fed04b196e98106d034f60 100644 (file)
@@ -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;
 
 /*