diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 342d782c74b..2e2e7acf195 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -147,6 +147,7 @@ static double preprocess_limit(PlannerInfo *root, double tuple_fraction, int64 *offset_est, int64 *count_est); static List *preprocess_groupclause(PlannerInfo *root, List *force); +static bool has_empty_grouping_set(List *groupingSets); static List *extract_rollup_sets(List *groupingSets); static List *reorder_grouping_sets(List *groupingSets, List *sortclause); static void standard_qp_callback(PlannerInfo *root, void *extra); @@ -1131,11 +1132,12 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, * In some cases we may want to transfer a HAVING clause into WHERE. We * cannot do so if the HAVING clause contains aggregates (obviously) or * volatile functions (since a HAVING clause is supposed to be executed - * only once per group). We also can't do this if there are any nonempty - * grouping sets and the clause references any columns that are nullable - * by the grouping sets; moving such a clause into WHERE would potentially - * change the results. (If there are only empty grouping sets, then the - * HAVING clause must be degenerate as discussed below.) + * only once per group). We also can't do this if there are any grouping + * sets and the clause references any columns that are nullable by the + * grouping sets; the nulled values of those columns are not available + * before the grouping step. (The test on groupClause might seem wrong, + * but it's okay: it's just an optimization to avoid running pull_varnos + * when there cannot be any Vars in the HAVING clause.) * * Also, it may be that the clause is so expensive to execute that we're * better off doing it only once per group, despite the loss of @@ -1145,19 +1147,19 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, * clause into WHERE, in hopes of eliminating tuples before aggregation * instead of after. * - * If the query has explicit grouping then we can simply move such a + * If the query has no empty grouping set then we can simply move such a * clause into WHERE; any group that fails the clause will not be in the * output because none of its tuples will reach the grouping or - * aggregation stage. Otherwise we must have a degenerate (variable-free) - * HAVING clause, which we put in WHERE so that query_planner() can use it - * in a gating Result node, but also keep in HAVING to ensure that we - * don't emit a bogus aggregated row. (This could be done better, but it - * seems not worth optimizing.) + * aggregation stage. Otherwise we have to keep the clause in HAVING to + * ensure that we don't emit a bogus aggregated row. But then the HAVING + * clause must be degenerate (variable-free), so we can copy it into WHERE + * so that query_planner() can use it in a gating Result node. (This could + * be done better, but it seems not worth optimizing.) * * Note that a HAVING clause may contain expressions that are not fully * preprocessed. This can happen if these expressions are part of * grouping items. In such cases, they are replaced with GROUP Vars in - * the parser and then replaced back after we've done with expression + * the parser and then replaced back after we're done with expression * preprocessing on havingQual. This is not an issue if the clause * remains in HAVING, because these expressions will be matched to lower * target items in setrefs.c. However, if the clause is moved or copied @@ -1182,7 +1184,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, /* keep it in HAVING */ newHaving = lappend(newHaving, havingclause); } - else if (parse->groupClause) + else if (parse->groupClause && + !has_empty_grouping_set(parse->groupingSets)) { Node *whereclause; @@ -2910,6 +2913,39 @@ preprocess_groupclause(PlannerInfo *root, List *force) return new_groupclause; } +/* + * Check for empty grouping sets within a list of GroupingSets. + */ +static bool +has_empty_grouping_set(List *groupingSets) +{ + ListCell *lc; + + foreach(lc, groupingSets) + { + GroupingSet *gset = castNode(GroupingSet, lfirst(lc)); + + switch (gset->kind) + { + case GROUPING_SET_EMPTY: + return true; + case GROUPING_SET_SIMPLE: + /* keep looking */ + break; + case GROUPING_SET_ROLLUP: + case GROUPING_SET_CUBE: + /* these necessarily include an empty set */ + return true; + case GROUPING_SET_SETS: + /* recurse */ + if (has_empty_grouping_set(gset->content)) + return true; + break; + } + } + return false; +} + /* * Extract lists of grouping sets that can be implemented using a single * rollup-type aggregate pass each. Returns a list of lists of grouping sets. diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index 991121545c5..a480b4749a8 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -890,7 +890,8 @@ explain (costs off) -> Seq Scan on gstest2 (10 rows) --- test pushdown of HAVING clause that does not reference any columns that are nullable by grouping sets +-- test pushdown of non-degenerate HAVING clause that does not reference any +-- columns that are nullable by grouping sets explain (costs off) select a, b, count(*) from gstest2 group by grouping sets ((a, b), (a)) having a > 1 and b > 1; QUERY PLAN @@ -911,6 +912,44 @@ select a, b, count(*) from gstest2 group by grouping sets ((a, b), (a)) having a 2 | 2 | 1 (1 row) +-- test pushdown of degenerate HAVING clause +explain (costs off) +select count(*) from gstest2 group by grouping sets (()) having false; + QUERY PLAN +----------------------------------- + Aggregate + Group Key: () + Filter: false + -> Result + Replaces: Scan on gstest2 + One-Time Filter: false +(6 rows) + +select count(*) from gstest2 group by grouping sets (()) having false; + count +------- +(0 rows) + +explain (costs off) +select a, count(*) from gstest2 group by grouping sets ((a), ()) having false; + QUERY PLAN +----------------------------------------- + GroupAggregate + Group Key: a + Group Key: () + Filter: false + -> Sort + Sort Key: a + -> Result + Replaces: Scan on gstest2 + One-Time Filter: false +(9 rows) + +select a, count(*) from gstest2 group by grouping sets ((a), ()) having false; + a | count +---+------- +(0 rows) + -- HAVING with GROUPING queries select ten, grouping(ten) from onek group by grouping sets(ten) having grouping(ten) >= 0 diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index 38d3cdd0fd8..dbacc2ffdce 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -290,11 +290,21 @@ explain (costs off) select v.c, (select count(*) from gstest2 group by () having v.c) from (values (false),(true)) v(c) order by v.c; --- test pushdown of HAVING clause that does not reference any columns that are nullable by grouping sets +-- test pushdown of non-degenerate HAVING clause that does not reference any +-- columns that are nullable by grouping sets explain (costs off) select a, b, count(*) from gstest2 group by grouping sets ((a, b), (a)) having a > 1 and b > 1; select a, b, count(*) from gstest2 group by grouping sets ((a, b), (a)) having a > 1 and b > 1; +-- test pushdown of degenerate HAVING clause +explain (costs off) +select count(*) from gstest2 group by grouping sets (()) having false; +select count(*) from gstest2 group by grouping sets (()) having false; + +explain (costs off) +select a, count(*) from gstest2 group by grouping sets ((a), ()) having false; +select a, count(*) from gstest2 group by grouping sets ((a), ()) having false; + -- HAVING with GROUPING queries select ten, grouping(ten) from onek group by grouping sets(ten) having grouping(ten) >= 0