From 7b6c07547190f056b0464098bb5a2247129d7aa2 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
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,