From 7b6c07547190f056b0464098bb5a2247129d7aa2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Nov 2017 11:24:12 -0400 Subject: [PATCH] Teach planner to account for HAVING quals in aggregation plan nodes. For some reason, we have never accounted for either the evaluation cost or the selectivity of filter conditions attached to Agg and Group nodes (which, in practice, are always conditions from a HAVING clause). Applying our regular selectivity logic to post-grouping conditions is a bit bogus, but it's surely better than taking the selectivity as 1.0. Perhaps someday the extended-statistics mechanism can be taught to provide statistics that would help us in getting non-default estimates here. Per a gripe from Benjamin Coutu. This is surely a bug fix, but I'm hesitant to back-patch because of the prospect of destabilizing existing plan choices. Given that it took us this long to notice the bug, it's probably not hurting too many people in the field. Discussion: https://postgr.es/m/20968.1509486337@sss.pgh.pa.us --- src/backend/optimizer/path/costsize.c | 46 +++++++++++++++++++++++++- src/backend/optimizer/prep/prepunion.c | 2 ++ src/backend/optimizer/util/pathnode.c | 25 ++++++++++++++ src/include/optimizer/cost.h | 2 ++ 4 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index ce32b8a4b9..98fb16e85a 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -1874,6 +1874,7 @@ void cost_agg(Path *path, PlannerInfo *root, AggStrategy aggstrategy, const AggClauseCosts *aggcosts, int numGroupCols, double numGroups, + List *quals, Cost input_startup_cost, Cost input_total_cost, double input_tuples) { @@ -1955,6 +1956,26 @@ cost_agg(Path *path, PlannerInfo *root, output_tuples = numGroups; } + /* + * If there are quals (HAVING quals), account for their cost and + * selectivity. + */ + if (quals) + { + QualCost qual_cost; + + cost_qual_eval(&qual_cost, quals, root); + startup_cost += qual_cost.startup; + total_cost += qual_cost.startup + output_tuples * qual_cost.per_tuple; + + output_tuples = clamp_row_est(output_tuples * + clauselist_selectivity(root, + quals, + 0, + JOIN_INNER, + NULL)); + } + path->rows = output_tuples; path->startup_cost = startup_cost; path->total_cost = total_cost; @@ -2040,12 +2061,15 @@ cost_windowagg(Path *path, PlannerInfo *root, void cost_group(Path *path, PlannerInfo *root, int numGroupCols, double numGroups, + List *quals, Cost input_startup_cost, Cost input_total_cost, double input_tuples) { + double output_tuples; Cost startup_cost; Cost total_cost; + output_tuples = numGroups; startup_cost = input_startup_cost; total_cost = input_total_cost; @@ -2055,7 +2079,27 @@ cost_group(Path *path, PlannerInfo *root, */ total_cost += cpu_operator_cost * input_tuples * numGroupCols; - path->rows = numGroups; + /* + * If there are quals (HAVING quals), account for their cost and + * selectivity. + */ + if (quals) + { + QualCost qual_cost; + + cost_qual_eval(&qual_cost, quals, root); + startup_cost += qual_cost.startup; + total_cost += qual_cost.startup + output_tuples * qual_cost.per_tuple; + + output_tuples = clamp_row_est(output_tuples * + clauselist_selectivity(root, + quals, + 0, + JOIN_INNER, + NULL)); + } + + path->rows = output_tuples; path->startup_cost = startup_cost; path->total_cost = total_cost; } diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 1c84a2cb28..f620243ab4 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -977,6 +977,7 @@ choose_hashed_setop(PlannerInfo *root, List *groupClauses, */ cost_agg(&hashed_p, root, AGG_HASHED, NULL, numGroupCols, dNumGroups, + NIL, input_path->startup_cost, input_path->total_cost, input_path->rows); @@ -991,6 +992,7 @@ choose_hashed_setop(PlannerInfo *root, List *groupClauses, input_path->rows, input_path->pathtarget->width, 0.0, work_mem, -1.0); cost_group(&sorted_p, root, numGroupCols, dNumGroups, + NIL, sorted_p.startup_cost, sorted_p.total_cost, input_path->rows); diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index 2d491eb0ba..36ec025b05 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -1374,6 +1374,11 @@ create_result_path(PlannerInfo *root, RelOptInfo *rel, pathnode->path.startup_cost = target->cost.startup; pathnode->path.total_cost = target->cost.startup + cpu_tuple_cost + target->cost.per_tuple; + + /* + * Add cost of qual, if any --- but we ignore its selectivity, since our + * rowcount estimate should be 1 no matter what the qual is. + */ if (resconstantqual) { QualCost qual_cost; @@ -1596,6 +1601,7 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath, cost_agg(&agg_path, root, AGG_HASHED, NULL, numCols, pathnode->path.rows, + NIL, subpath->startup_cost, subpath->total_cost, rel->rows); @@ -2592,6 +2598,7 @@ create_group_path(PlannerInfo *root, cost_group(&pathnode->path, root, list_length(groupClause), numGroups, + qual, subpath->startup_cost, subpath->total_cost, subpath->rows); @@ -2709,6 +2716,7 @@ create_agg_path(PlannerInfo *root, cost_agg(&pathnode->path, root, aggstrategy, aggcosts, list_length(groupClause), numGroups, + qual, subpath->startup_cost, subpath->total_cost, subpath->rows); @@ -2817,6 +2825,7 @@ create_groupingsets_path(PlannerInfo *root, agg_costs, numGroupCols, rollup->numGroups, + having_qual, subpath->startup_cost, subpath->total_cost, subpath->rows); @@ -2840,6 +2849,7 @@ create_groupingsets_path(PlannerInfo *root, agg_costs, numGroupCols, rollup->numGroups, + having_qual, 0.0, 0.0, subpath->rows); if (!rollup->is_hashed) @@ -2863,6 +2873,7 @@ create_groupingsets_path(PlannerInfo *root, agg_costs, numGroupCols, rollup->numGroups, + having_qual, sort_path.startup_cost, sort_path.total_cost, sort_path.rows); @@ -2932,6 +2943,19 @@ create_minmaxagg_path(PlannerInfo *root, pathnode->path.total_cost = initplan_cost + target->cost.startup + target->cost.per_tuple + cpu_tuple_cost; + /* + * Add cost of qual, if any --- but we ignore its selectivity, since our + * rowcount estimate should be 1 no matter what the qual is. + */ + if (quals) + { + QualCost qual_cost; + + cost_qual_eval(&qual_cost, quals, root); + pathnode->path.startup_cost += qual_cost.startup; + pathnode->path.total_cost += qual_cost.startup + qual_cost.per_tuple; + } + return pathnode; } @@ -3781,6 +3805,7 @@ reparameterize_pathlist_by_child(PlannerInfo *root, { Path *path = reparameterize_path_by_child(root, lfirst(lc), child_rel); + if (path == NULL) { list_free(result); diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h index 306d923a22..6c2317df39 100644 --- a/src/include/optimizer/cost.h +++ b/src/include/optimizer/cost.h @@ -116,6 +116,7 @@ extern void cost_material(Path *path, extern void cost_agg(Path *path, PlannerInfo *root, AggStrategy aggstrategy, const AggClauseCosts *aggcosts, int numGroupCols, double numGroups, + List *quals, Cost input_startup_cost, Cost input_total_cost, double input_tuples); extern void cost_windowagg(Path *path, PlannerInfo *root, @@ -124,6 +125,7 @@ extern void cost_windowagg(Path *path, PlannerInfo *root, double input_tuples); extern void cost_group(Path *path, PlannerInfo *root, int numGroupCols, double numGroups, + List *quals, Cost input_startup_cost, Cost input_total_cost, double input_tuples); extern void initial_cost_nestloop(PlannerInfo *root, -- 2.40.0