From 648a6c7bd815f98b35709bd56f9f1ca276b33ae6 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 15 Mar 2018 11:33:52 -0400 Subject: [PATCH] Pass additional arguments to a couple of grouping-related functions. get_number_of_groups() and make_partial_grouping_target() currently fish information directly out of the PlannerInfo; in the former case, the target list, and in the latter case, the HAVING qual. This works fine if there's only one grouping relation, but if the pending patch for partition-wise aggregate gets committed, we'll have multiple grouping relations and must therefore use appropriately translated versions of these values for each one. To make that simpler, pass the values to be used as arguments. Jeevan Chalke. The larger patch series of which this patch is a part was also reviewed and tested by Antonin Houska, Rajkumar Raghuwanshi, David Rowley, Dilip Kumar, Konstantin Knizhnik, Pascal Legrand, Rafia Sabih, and me. Discussion: http://postgr.es/m/CAM2+6=UqFnFUypOvLdm5TgC+2M=-E0Q7_LOh0VDFFzmk2BBPzQ@mail.gmail.com Discussion: http://postgr.es/m/CAM2+6=W+L=C4yBqMrgrfTfNtbtmr4T53-hZhwbA2kvbZ9VMrrw@mail.gmail.com --- src/backend/optimizer/plan/planner.c | 35 +++++++++++++++++----------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 182b01627e..f38bffdc15 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -130,7 +130,8 @@ static List *reorder_grouping_sets(List *groupingSets, List *sortclause); static void standard_qp_callback(PlannerInfo *root, void *extra); static double get_number_of_groups(PlannerInfo *root, double path_rows, - grouping_sets_data *gd); + grouping_sets_data *gd, + List *target_list); static Size estimate_hashagg_tablesize(Path *path, const AggClauseCosts *agg_costs, double dNumGroups); @@ -175,7 +176,8 @@ static RelOptInfo *create_ordered_paths(PlannerInfo *root, static PathTarget *make_group_input_target(PlannerInfo *root, PathTarget *final_target); static PathTarget *make_partial_grouping_target(PlannerInfo *root, - PathTarget *grouping_target); + PathTarget *grouping_target, + Node *havingQual); static List *postprocess_setop_tlist(List *new_tlist, List *orig_tlist); static List *select_active_windows(PlannerInfo *root, WindowFuncLists *wflists); static PathTarget *make_window_input_target(PlannerInfo *root, @@ -3505,6 +3507,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) * * path_rows: number of output rows from scan/join step * gd: grouping sets data including list of grouping sets and their clauses + * target_list: target list containing group clause references * * If doing grouping sets, we also annotate the gsets data with the estimates * for each set and each individual rollup list, with a view to later @@ -3513,7 +3516,8 @@ standard_qp_callback(PlannerInfo *root, void *extra) static double get_number_of_groups(PlannerInfo *root, double path_rows, - grouping_sets_data *gd) + grouping_sets_data *gd, + List *target_list) { Query *parse = root->parse; double dNumGroups; @@ -3538,7 +3542,7 @@ get_number_of_groups(PlannerInfo *root, ListCell *lc; groupExprs = get_sortgrouplist_exprs(rollup->groupClause, - parse->targetList); + target_list); rollup->numGroups = 0.0; @@ -3565,7 +3569,7 @@ get_number_of_groups(PlannerInfo *root, gd->dNumHashGroups = 0; groupExprs = get_sortgrouplist_exprs(parse->groupClause, - parse->targetList); + target_list); forboth(lc, gd->hash_sets_idx, lc2, gd->unsortable_sets) { @@ -3587,7 +3591,7 @@ get_number_of_groups(PlannerInfo *root, { /* Plain GROUP BY */ groupExprs = get_sortgrouplist_exprs(parse->groupClause, - parse->targetList); + target_list); dNumGroups = estimate_num_groups(root, groupExprs, path_rows, NULL); @@ -3797,7 +3801,8 @@ create_grouping_paths(PlannerInfo *root, */ dNumGroups = get_number_of_groups(root, cheapest_path->rows, - gd); + gd, + parse->targetList); /* * Determine whether it's possible to perform sort-based implementations @@ -3859,7 +3864,8 @@ create_grouping_paths(PlannerInfo *root, * appear in the result tlist, and (2) the Aggrefs must be set in * partial mode. */ - partial_grouping_target = make_partial_grouping_target(root, target); + partial_grouping_target = make_partial_grouping_target(root, target, + (Node *) parse->havingQual); partially_grouped_rel->reltarget = partial_grouping_target; /* @@ -4912,10 +4918,12 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target) * these would be Vars that are grouped by or used in grouping expressions.) * * grouping_target is the tlist to be emitted by the topmost aggregation step. - * We get the HAVING clause out of *root. + * havingQual represents the HAVING clause. */ static PathTarget * -make_partial_grouping_target(PlannerInfo *root, PathTarget *grouping_target) +make_partial_grouping_target(PlannerInfo *root, + PathTarget *grouping_target, + Node *havingQual) { Query *parse = root->parse; PathTarget *partial_target; @@ -4957,8 +4965,8 @@ make_partial_grouping_target(PlannerInfo *root, PathTarget *grouping_target) /* * If there's a HAVING clause, we'll need the Vars/Aggrefs it uses, too. */ - if (parse->havingQual) - non_group_cols = lappend(non_group_cols, parse->havingQual); + if (havingQual) + non_group_cols = lappend(non_group_cols, havingQual); /* * Pull out all the Vars, PlaceHolderVars, and Aggrefs mentioned in @@ -6283,7 +6291,8 @@ add_paths_to_partial_grouping_rel(PlannerInfo *root, /* Estimate number of partial groups. */ dNumPartialGroups = get_number_of_groups(root, cheapest_partial_path->rows, - gd); + gd, + parse->targetList); if (can_sort) {