From f40a9fd834f23b916cf24b6582c8931186386146 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 13 Mar 2018 13:42:56 -0400 Subject: [PATCH 5/7] Remove explicit path construction logic in create_ordered_paths. Instead of having create_ordered_paths build a path for an explicit Sort and Gather Merge of the cheapest partial path, add an explicitly-sorted path to tlist_rel. If this path looks advantageous from a cost point of view, the call to generate_gather_paths() for tlist_rel will take care of building a Gather Merge path for it. This improves the accuracy of costing for such paths because it avoids using apply_projection_to_path, which has the disadvantage of modifying a path in place after the cost has already been determined. Along the way, this fixes a mistake in gather_grouping_paths introduced by commit e2f1eb0ee30d144628ab523432320f174a2c8966: gather_grouping_paths should try to sort by the group_pathkeys when operating on a partially grouped rel, but not when operating on a fully grouped rel. In that case, it needs to sort by whatever the next ordering requirement will be. Patch by me, reviewed by Amit Kapila. --- src/backend/optimizer/plan/planner.c | 104 +++++++++++++++-------------------- 1 file changed, 44 insertions(+), 60 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index c48e98643a..90eddf73fd 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -217,7 +217,8 @@ static RelOptInfo *create_partial_grouping_paths(PlannerInfo *root, grouping_sets_data *gd, GroupPathExtraData *extra, bool force_rel_creation); -static void gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel); +static void gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel, + bool partial); static bool can_partial_agg(PlannerInfo *root, const AggClauseCosts *agg_costs); static RelOptInfo *create_tlist_paths(PlannerInfo *root, @@ -3999,7 +4000,7 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, /* Gather any partially grouped partial paths. */ if (partially_grouped_rel && partially_grouped_rel->partial_pathlist) { - gather_grouping_paths(root, partially_grouped_rel); + gather_grouping_paths(root, partially_grouped_rel, true); set_cheapest(partially_grouped_rel); } @@ -4856,57 +4857,6 @@ create_ordered_paths(PlannerInfo *root, } } - /* - * generate_gather_paths() will have already generated a simple Gather - * path for the best parallel path, if any, and the loop above will have - * considered sorting it. Similarly, generate_gather_paths() will also - * have generated order-preserving Gather Merge plans which can be used - * without sorting if they happen to match the sort_pathkeys, and the loop - * above will have handled those as well. However, there's one more - * possibility: it may make sense to sort the cheapest partial path - * according to the required output order and then use Gather Merge. - */ - if (ordered_rel->consider_parallel && root->sort_pathkeys != NIL && - input_rel->partial_pathlist != NIL) - { - Path *cheapest_partial_path; - - cheapest_partial_path = linitial(input_rel->partial_pathlist); - - /* - * If cheapest partial path doesn't need a sort, this is redundant - * with what's already been tried. - */ - if (!pathkeys_contained_in(root->sort_pathkeys, - cheapest_partial_path->pathkeys)) - { - Path *path; - double total_groups; - - path = (Path *) create_sort_path(root, - ordered_rel, - cheapest_partial_path, - root->sort_pathkeys, - limit_tuples); - - total_groups = cheapest_partial_path->rows * - cheapest_partial_path->parallel_workers; - path = (Path *) - create_gather_merge_path(root, ordered_rel, - path, - path->pathtarget, - root->sort_pathkeys, NULL, - &total_groups); - - /* Add projection step if needed */ - if (path->pathtarget != target) - path = apply_projection_to_path(root, ordered_rel, - path, target); - - add_path(ordered_rel, path); - } - } - /* * If there is an FDW that's responsible for all baserels of the query, * let it consider adding ForeignPaths. @@ -6398,7 +6348,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel, * non-partial paths from each child. */ if (grouped_rel->partial_pathlist != NIL) - gather_grouping_paths(root, grouped_rel); + gather_grouping_paths(root, grouped_rel, false); } /* @@ -6720,17 +6670,29 @@ create_partial_grouping_paths(PlannerInfo *root, * generate_gather_paths(). */ static void -gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) +gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel, bool partial) { Path *cheapest_partial_path; + List *pathkeys; /* Try Gather for unordered paths and Gather Merge for ordered ones. */ generate_gather_paths(root, rel, true); + if (partial) + pathkeys = root->group_pathkeys; + else if (root->window_pathkeys) + pathkeys = root->window_pathkeys; + else if (list_length(root->distinct_pathkeys) > + list_length(root->sort_pathkeys)) + pathkeys = root->distinct_pathkeys; + else if (root->sort_pathkeys) + pathkeys = root->sort_pathkeys; + else + pathkeys = NIL; + /* Try cheapest partial path + explicit Sort + Gather Merge. */ cheapest_partial_path = linitial(rel->partial_pathlist); - if (!pathkeys_contained_in(root->group_pathkeys, - cheapest_partial_path->pathkeys)) + if (!pathkeys_contained_in(pathkeys, cheapest_partial_path->pathkeys)) { Path *path; double total_groups; @@ -6738,14 +6700,14 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) total_groups = cheapest_partial_path->rows * cheapest_partial_path->parallel_workers; path = (Path *) create_sort_path(root, rel, cheapest_partial_path, - root->group_pathkeys, + pathkeys, -1.0); path = (Path *) create_gather_merge_path(root, rel, path, rel->reltarget, - root->group_pathkeys, + pathkeys, NULL, &total_groups); @@ -6934,8 +6896,10 @@ create_tlist_paths(PlannerInfo *root, * paths for the tlist_rel; these may be useful to upper planning * stages. */ - if (tlist_rel->consider_parallel) + if (tlist_rel->consider_parallel && input_rel->partial_pathlist != NIL) { + Path *cheapest_partial_path; + /* Apply the scan/join target to each partial path */ foreach(lc, input_rel->partial_pathlist) { @@ -6951,6 +6915,26 @@ create_tlist_paths(PlannerInfo *root, scanjoin_target); add_partial_path(tlist_rel, newpath); } + + /* Also try explicitly sorting the cheapest path. */ + cheapest_partial_path = linitial(input_rel->partial_pathlist); + if (!pathkeys_contained_in(root->query_pathkeys, + cheapest_partial_path->pathkeys) + && !is_other_rel) + { + Path *path; + + path = (Path *) create_projection_path(root, + tlist_rel, + cheapest_partial_path, + scanjoin_target); + path = (Path *) create_sort_path(root, + tlist_rel, + path, + root->query_pathkeys, + -1); + add_partial_path(tlist_rel, path); + } } /* Now fix things up if scan/join target contains SRFs */ -- 2.14.3 (Apple Git-98)