From 67fe6d448323fe3b1d7c85dc14393b912b628127 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 6 Mar 2018 14:13:00 -0500 Subject: [PATCH 3/3] Refactor code to add Gather/Gather Merge. --- src/backend/optimizer/plan/planner.c | 74 +++++++++++------------------ src/test/regress/expected/partition_agg.out | 12 ++--- 2 files changed, 34 insertions(+), 52 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 934c9d322c..b87cba1f62 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -205,6 +205,7 @@ static void add_paths_to_partial_grouping_rel(PlannerInfo *root, bool can_hash, bool use_partial_pathlist, bool need_partial_agg); +static void gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel); static bool can_parallel_agg(PlannerInfo *root, RelOptInfo *input_rel, RelOptInfo *grouped_rel, const AggClauseCosts *agg_costs); static void apply_scanjoin_target_to_paths(PlannerInfo *root, RelOptInfo *rel, @@ -6245,39 +6246,13 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel, } /* - * If there are any fully aggregated partial paths present, may be because - * of parallel Append over partitionwise aggregates, we must stick a - * Gather or Gather Merge path atop the cheapest partial path. + * When partitionwise aggregate is used, we might have fully aggregated + * paths in the partial pathlist, because add_paths_to_append_rel() will + * consider a path for grouped_rel consisting of a Parallel Append of + * non-partial paths from each child. */ - if (grouped_rel->partial_pathlist) - { - Path *apath; - double total_groups; - - apath = (Path *) linitial(grouped_rel->partial_pathlist); - Assert(apath->parallel_workers > 0); - total_groups = apath->rows * apath->parallel_workers; - - add_path(grouped_rel, (Path *) - create_gather_path(root, grouped_rel, apath, - apath->pathtarget, NULL, &total_groups)); - - /* - * Sorting the cheapest path to match the group keys and then applying - * a Gather Merge node to the result might be a winning strategy. - */ - if (root->group_pathkeys) - { - apath = (Path *) create_sort_path(root, grouped_rel, apath, - root->group_pathkeys, -1.0); - - add_path(grouped_rel, (Path *) - create_gather_merge_path(root, grouped_rel, apath, - apath->pathtarget, - root->group_pathkeys, - NULL, &total_groups)); - } - } + if (grouped_rel->partial_pathlist != NIL) + gather_grouping_paths(root, grouped_rel); } /* @@ -6444,14 +6419,24 @@ add_paths_to_partial_grouping_rel(PlannerInfo *root, if (need_partial_agg) return; - /* - * Try adding Gather or Gather Merge to partial paths to produce - * non-partial paths. - */ - generate_gather_paths(root, partially_grouped_rel, true); + gather_grouping_paths(root, partially_grouped_rel); + + set_cheapest(partially_grouped_rel); +} + +/* + * Try adding Gather or Gather Merge to partial paths to produce non-partial + * paths. + */ +static void +gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) +{ + Path *cheapest_path; + + generate_gather_paths(root, rel, true); - /* Get cheapest partial path from partially_grouped_rel */ - cheapest_path = linitial(partially_grouped_rel->partial_pathlist); + /* Get cheapest partial path from rel */ + cheapest_path = linitial(rel->partial_pathlist); /* * generate_gather_paths won't consider sorting the cheapest path to match @@ -6464,23 +6449,20 @@ add_paths_to_partial_grouping_rel(PlannerInfo *root, double total_groups; total_groups = cheapest_path->rows * cheapest_path->parallel_workers; - path = (Path *) create_sort_path(root, partially_grouped_rel, + path = (Path *) create_sort_path(root, rel, cheapest_path, root->group_pathkeys, -1.0); path = (Path *) create_gather_merge_path(root, - partially_grouped_rel, + rel, path, - partially_grouped_rel->reltarget, + rel->reltarget, root->group_pathkeys, NULL, &total_groups); - add_path(partially_grouped_rel, path); + add_path(rel, path); } - - /* Now choose the best path(s) */ - set_cheapest(partially_grouped_rel); } /* diff --git a/src/test/regress/expected/partition_agg.out b/src/test/regress/expected/partition_agg.out index a7fd6c1a64..612de4aded 100644 --- a/src/test/regress/expected/partition_agg.out +++ b/src/test/regress/expected/partition_agg.out @@ -934,12 +934,12 @@ ANALYZE pagg_tab; -- is not partial agg safe. EXPLAIN (COSTS OFF) SELECT a, sum(b), array_agg(distinct c), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 3 ORDER BY 1, 2, 3; - QUERY PLAN ------------------------------------------------------------------------------------------------ - Sort - Sort Key: pagg_tab_p2_s1.a, (sum(pagg_tab_p2_s1.b)), (array_agg(DISTINCT pagg_tab_p2_s1.c)) - -> Gather - Workers Planned: 2 + QUERY PLAN +----------------------------------------------------------------------------------------------------- + Gather Merge + Workers Planned: 2 + -> Sort + Sort Key: pagg_tab_p2_s1.a, (sum(pagg_tab_p2_s1.b)), (array_agg(DISTINCT pagg_tab_p2_s1.c)) -> Parallel Append -> GroupAggregate Group Key: pagg_tab_p2_s1.a -- 2.14.3 (Apple Git-98)