Skip to content

Commit 581df21

Browse files
author
Richard Guo
committed
Fix rowcount estimate for gather (merge) paths
In the case of a parallel plan, when computing the number of tuples processed per worker, we divide the total number of tuples by the parallel_divisor obtained from get_parallel_divisor(), which accounts for the leader's contribution in addition to the number of workers. Accordingly, when estimating the number of tuples for gather (merge) nodes, we should multiply the number of tuples per worker by the same parallel_divisor to reverse the division. However, currently we use parallel_workers rather than parallel_divisor for the multiplication. This could result in an underestimation of the number of tuples for gather (merge) nodes, especially when there are fewer than four workers. This patch fixes this issue by using the same parallel_divisor for the multiplication. There is one ensuing plan change in the regression tests, but it looks reasonable and does not compromise its original purpose of testing parallel-aware hash join. In passing, this patch removes an unnecessary assignment for path.rows in create_gather_merge_path, and fixes an uninitialized-variable issue in generate_useful_gather_paths. No backpatch as this could result in plan changes. Author: Anthonin Bonnefoy Reviewed-by: Rafia Sabih, Richard Guo Discussion: https://postgr.es/m/CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com
1 parent d2cba4f commit 581df21

File tree

6 files changed

+33
-20
lines changed

6 files changed

+33
-20
lines changed

‎src/backend/optimizer/path/allpaths.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3079,8 +3079,7 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
30793079
* of partial_pathlist because of the way add_partial_path works.
30803080
*/
30813081
cheapest_partial_path = linitial(rel->partial_pathlist);
3082-
rows =
3083-
cheapest_partial_path->rows * cheapest_partial_path->parallel_workers;
3082+
rows = compute_gather_rows(cheapest_partial_path);
30843083
simple_gather_path = (Path *)
30853084
create_gather_path(root, rel, cheapest_partial_path, rel->reltarget,
30863085
NULL, rowsp);
@@ -3098,7 +3097,7 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
30983097
if (subpath->pathkeys == NIL)
30993098
continue;
31003099

3101-
rows = subpath->rows * subpath->parallel_workers;
3100+
rows = compute_gather_rows(subpath);
31023101
path = create_gather_merge_path(root, rel, subpath, rel->reltarget,
31033102
subpath->pathkeys, NULL, rowsp);
31043103
add_path(rel, &path->path);
@@ -3282,7 +3281,6 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
32823281
subpath,
32833282
useful_pathkeys,
32843283
-1.0);
3285-
rows = subpath->rows * subpath->parallel_workers;
32863284
}
32873285
else
32883286
subpath = (Path *) create_incremental_sort_path(root,
@@ -3291,6 +3289,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
32913289
useful_pathkeys,
32923290
presorted_keys,
32933291
-1);
3292+
rows = compute_gather_rows(subpath);
32943293
path = create_gather_merge_path(root, rel,
32953294
subpath,
32963295
rel->reltarget,

‎src/backend/optimizer/path/costsize.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6473,3 +6473,21 @@ compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
64736473

64746474
return pages_fetched;
64756475
}
6476+
6477+
/*
6478+
* compute_gather_rows
6479+
* Estimate number of rows for gather (merge) nodes.
6480+
*
6481+
* In a parallel plan, each worker's row estimate is determined by dividing the
6482+
* total number of rows by parallel_divisor, which accounts for the leader's
6483+
* contribution in addition to the number of workers. Accordingly, when
6484+
* estimating the number of rows for gather (merge) nodes, we multiply the rows
6485+
* per worker by the same parallel_divisor to undo the division.
6486+
*/
6487+
double
6488+
compute_gather_rows(Path *path)
6489+
{
6490+
Assert(path->parallel_workers > 0);
6491+
6492+
return clamp_row_est(path->rows * get_parallel_divisor(path));
6493+
}

‎src/backend/optimizer/plan/planner.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5370,8 +5370,7 @@ create_ordered_paths(PlannerInfo *root,
53705370
root->sort_pathkeys,
53715371
presorted_keys,
53725372
limit_tuples);
5373-
total_groups = input_path->rows *
5374-
input_path->parallel_workers;
5373+
total_groups = compute_gather_rows(sorted_path);
53755374
sorted_path = (Path *)
53765375
create_gather_merge_path(root, ordered_rel,
53775376
sorted_path,
@@ -7543,8 +7542,6 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel)
75437542
(presorted_keys == 0 || !enable_incremental_sort))
75447543
continue;
75457544

7546-
total_groups = path->rows * path->parallel_workers;
7547-
75487545
/*
75497546
* We've no need to consider both a sort and incremental sort. We'll
75507547
* just do a sort if there are no presorted keys and an incremental
@@ -7561,7 +7558,7 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel)
75617558
groupby_pathkeys,
75627559
presorted_keys,
75637560
-1.0);
7564-
7561+
total_groups = compute_gather_rows(path);
75657562
path = (Path *)
75667563
create_gather_merge_path(root,
75677564
rel,

‎src/backend/optimizer/util/pathnode.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1899,7 +1899,6 @@ create_gather_merge_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
18991899
pathnode->num_workers = subpath->parallel_workers;
19001900
pathnode->path.pathkeys = pathkeys;
19011901
pathnode->path.pathtarget = target ? target : rel->reltarget;
1902-
pathnode->path.rows += subpath->rows;
19031902

19041903
if (pathkeys_contained_in(pathkeys, subpath->pathkeys))
19051904
{

‎src/include/optimizer/cost.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,5 +212,6 @@ extern PathTarget *set_pathtarget_cost_width(PlannerInfo *root, PathTarget *targ
212212
extern double compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
213213
Path *bitmapqual, double loop_count,
214214
Cost *cost_p, double *tuples_p);
215+
extern double compute_gather_rows(Path *path);
215216

216217
#endif /* COST_H */

‎src/test/regress/expected/join_hash.out

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -508,18 +508,17 @@ set local hash_mem_multiplier = 1.0;
508508
set local enable_parallel_hash = on;
509509
explain (costs off)
510510
select count(*) from simple r join extremely_skewed s using (id);
511-
QUERY PLAN
512-
-----------------------------------------------------------------------
513-
Finalize Aggregate
511+
QUERY PLAN
512+
-----------------------------------------------------------------
513+
Aggregate
514514
-> Gather
515515
Workers Planned: 1
516-
-> Partial Aggregate
517-
-> Parallel Hash Join
518-
Hash Cond: (r.id = s.id)
519-
-> Parallel Seq Scan on simple r
520-
-> Parallel Hash
521-
-> Parallel Seq Scan on extremely_skewed s
522-
(9 rows)
516+
-> Parallel Hash Join
517+
Hash Cond: (r.id = s.id)
518+
-> Parallel Seq Scan on simple r
519+
-> Parallel Hash
520+
-> Parallel Seq Scan on extremely_skewed s
521+
(8 rows)
523522

524523
select count(*) from simple r join extremely_skewed s using (id);
525524
count

0 commit comments

Comments
 (0)