Fix regression in parallel planning against inheritance tables.

Commit 51ee6f3160d2e1515ed6197594bda67eb99dc2cc accidentally changed
the behavior around inheritance hierarchies; before, we always
considered parallel paths even for very small inheritance children,
because otherwise an inheritance hierarchy with even one small child
wouldn't be eligible for parallelism.  That exception was inadverently
removed; put it back.

In passing, also adjust the degree-of-parallelism comptuation for
index-only scans not to consider the number of heap pages fetched.
Otherwise, we'll avoid parallel index-only scans on tables that are
mostly all-visible, which isn't especially logical.

Robert Haas and Amit Kapila, per a report from Ashutosh Sharma.

Discussion: http://postgr.es/m/CAE9k0PmgSoOHRd60SHu09aRVTHRSs8s6pmyhJKWHxWw9C_x+XA@mail.gmail.com

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index b263359fdedd60aac7dcfc23ecb7bad70bf7405c..4db1d16c79070802446b1580f980a6cda8da40da 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -692,7 +692,7 @@ create_plain_partial_paths(PlannerInfo *root, RelOptInfo *rel)
 {
        int                     parallel_workers;
 
-       parallel_workers = compute_parallel_worker(rel, rel->pages, 0);
+       parallel_workers = compute_parallel_worker(rel, rel->pages, -1);
 
        /* If any limit was set to zero, the user doesn't want a parallel scan. */
        if (parallel_workers <= 0)
@@ -2938,7 +2938,7 @@ create_partial_bitmap_paths(PlannerInfo *root, RelOptInfo *rel,
        pages_fetched = compute_bitmap_pages(root, rel, bitmapqual, 1.0,
                                                                                 NULL, NULL);
 
-       parallel_workers = compute_parallel_worker(rel, pages_fetched, 0);
+       parallel_workers = compute_parallel_worker(rel, pages_fetched, -1);
 
        if (parallel_workers <= 0)
                return;
@@ -2953,16 +2953,16 @@ create_partial_bitmap_paths(PlannerInfo *root, RelOptInfo *rel,
  * be scanned and the size of the index to be scanned, then choose a minimum
  * of those.
  *
- * "heap_pages" is the number of pages from the table that we expect to scan.
- * "index_pages" is the number of pages from the index that we expect to scan.
+ * "heap_pages" is the number of pages from the table that we expect to scan, or
+ * -1 if we don't expect to scan any.
+ *
+ * "index_pages" is the number of pages from the index that we expect to scan, or
+ * -1 if we don't expect to scan any.
  */
 int
-compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages,
-                                               BlockNumber index_pages)
+compute_parallel_worker(RelOptInfo *rel, double heap_pages, double index_pages)
 {
        int                     parallel_workers = 0;
-       int                     heap_parallel_workers = 1;
-       int                     index_parallel_workers = 1;
 
        /*
         * If the user has set the parallel_workers reloption, use that; otherwise
@@ -2972,23 +2972,24 @@ compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages,
                parallel_workers = rel->rel_parallel_workers;
        else
        {
-               int                     heap_parallel_threshold;
-               int                     index_parallel_threshold;
-
                /*
-                * If this relation is too small to be worth a parallel scan, just
-                * return without doing anything ... unless it's an inheritance child.
-                * In that case, we want to generate a parallel path here anyway.  It
-                * might not be worthwhile just for this relation, but when combined
-                * with all of its inheritance siblings it may well pay off.
+                * If the number of pages being scanned is insufficient to justify a
+                * parallel scan, just return zero ... unless it's an inheritance
+                * child. In that case, we want to generate a parallel path here
+                * anyway.  It might not be worthwhile just for this relation, but
+                * when combined with all of its inheritance siblings it may well pay
+                * off.
                 */
-               if (heap_pages < (BlockNumber) min_parallel_table_scan_size &&
-                       index_pages < (BlockNumber) min_parallel_index_scan_size &&
-                       rel->reloptkind == RELOPT_BASEREL)
+               if (rel->reloptkind == RELOPT_BASEREL &&
+                       ((heap_pages >= 0 && heap_pages < min_parallel_table_scan_size) ||
+                  (index_pages >= 0 && index_pages < min_parallel_index_scan_size)))
                        return 0;
 
-               if (heap_pages > 0)
+               if (heap_pages >= 0)
                {
+                       int                     heap_parallel_threshold;
+                       int                     heap_parallel_workers = 1;
+
                        /*
                         * Select the number of workers based on the log of the size of
                         * the relation.  This probably needs to be a good deal more
@@ -3008,8 +3009,11 @@ compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages,
                        parallel_workers = heap_parallel_workers;
                }
 
-               if (index_pages > 0)
+               if (index_pages >= 0)
                {
+                       int                     index_parallel_workers = 1;
+                       int                     index_parallel_threshold;
+
                        /* same calculation as for heap_pages above */
                        index_parallel_threshold = Max(min_parallel_index_scan_size, 1);
                        while (index_pages >= (BlockNumber) (index_parallel_threshold * 3))

diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 88835862022fde60fd9053e4975c24fb0dabe46b..cd6dcaab190e602da4d9ae99efabbb2076cf5237 100644 (file)
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -662,6 +662,14 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count,
 
        if (partial_path)
        {
+               /*
+                * For index only scans compute workers based on number of index pages
+                * fetched; the number of heap pages we fetch might be so small as
+                * to effectively rule out parallelism, which we don't want to do.
+                */
+               if (indexonly)
+                       rand_heap_pages = -1;
+
                /*
                 * Estimate the number of parallel workers required to scan index. Use
                 * the number of heap pages computed considering heap fetches won't be
@@ -669,8 +677,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count,
                 * order.
                 */
                path->path.parallel_workers = compute_parallel_worker(baserel,
-                                                                                          (BlockNumber) rand_heap_pages,
-                                                                                                 (BlockNumber) index_pages);
+                                                                                          rand_heap_pages, index_pages);
 
                /*
                 * Fall out if workers can't be assigned for parallel scan, because in

diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 247fd118793229c5eb53e4e5a150be49a963e895..25fe78cddd4e5ff73f57772b213c15e4683fbd89 100644 (file)
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -54,8 +54,8 @@ extern RelOptInfo *standard_join_search(PlannerInfo *root, int levels_needed,
                                         List *initial_rels);
 
 extern void generate_gather_paths(PlannerInfo *root, RelOptInfo *rel);
-extern int compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages,
-                                               BlockNumber index_pages);
+extern int compute_parallel_worker(RelOptInfo *rel, double heap_pages,
+                                               double index_pages);
 extern void create_partial_bitmap_paths(PlannerInfo *root, RelOptInfo *rel,
                                                                                Path *bitmapqual);