From b439adcabbf987e03a6e2ae92743953e269269be Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 22 Aug 2020 14:46:40 -0400 Subject: [PATCH] Avoid pushing quals down into sub-queries that have grouping sets. The trouble with doing this is that an apparently-constant subquery output column isn't really constant if it is a grouping column that appears in only some of the grouping sets. A qual using such a column would be subject to incorrect const-folding after push-down, as seen in bug #16585 from Paul Sivash. To fix, just disable qual pushdown altogether if the sub-query has nonempty groupingSets. While we could imagine far less restrictive solutions, there is not much point in working harder right now, because subquery_planner() won't move HAVING clauses to WHERE within such a subquery. If the qual stays in HAVING it's not going to be a lot more useful than if we'd kept it at the outer level. Having said that, this restriction could be removed if we used a parsetree representation that distinguished such outputs from actual constants, which is something I hope to do in future. Hence, make the patch a minimal addition rather than integrating it more tightly (e.g. by renumbering the existing items in subquery_is_pushdown_safe's comment). Back-patch to 9.5 where grouping sets were introduced. Discussion: https://postgr.es/m/16585-9d8c340d23ade8c1@postgresql.org --- src/backend/optimizer/path/allpaths.c | 15 ++++++++++ src/test/regress/expected/groupingsets.out | 32 ++++++++++++++++++++++ src/test/regress/sql/groupingsets.sql | 16 +++++++++++ 3 files changed, 63 insertions(+) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 0c6a348f0e..975d8db658 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -2828,6 +2828,17 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels) * volatile qual could succeed for some SRF output rows and fail for others, * a behavior that cannot occur if it's evaluated before SRF expansion. * + * 6. If the subquery has nonempty grouping sets, we cannot push down any + * quals. The concern here is that a qual referencing a "constant" grouping + * column could get constant-folded, which would be improper because the value + * is potentially nullable by grouping-set expansion. This restriction could + * be removed if we had a parsetree representation that shows that such + * grouping columns are not really constant. (There are other ideas that + * could be used to relax this restriction, but that's the approach most + * likely to get taken in the future. Note that there's not much to be gained + * so long as subquery_planner can't move HAVING clauses to WHERE within such + * a subquery.) + * * In addition, we make several checks on the subquery's output columns to see * if it is safe to reference them in pushed-down quals. If output column k * is found to be unsafe to reference, we set safetyInfo->unsafeColumns[k] @@ -2872,6 +2883,10 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery, if (subquery->limitOffset != NULL || subquery->limitCount != NULL) return false; + /* Check point 6 */ + if (subquery->groupClause && subquery->groupingSets) + return false; + /* Check points 3, 4, and 5 */ if (subquery->distinctClause || subquery->hasWindowFuncs || diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index 5d92b08d20..f972dd81be 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -434,6 +434,38 @@ select x, not x as not_x, q2 from | | 4567890123456789 (5 rows) +-- check qual push-down rules for a subquery with grouping sets +explain (verbose, costs off) +select * from ( + select 1 as x, q1, sum(q2) + from int8_tbl i1 + group by grouping sets(1, 2) +) ss +where x = 1 and q1 = 123; + QUERY PLAN +-------------------------------------------- + Subquery Scan on ss + Output: ss.x, ss.q1, ss.sum + Filter: ((ss.x = 1) AND (ss.q1 = 123)) + -> GroupAggregate + Output: (1), i1.q1, sum(i1.q2) + Group Key: 1 + Sort Key: i1.q1 + Group Key: i1.q1 + -> Seq Scan on public.int8_tbl i1 + Output: 1, i1.q1, i1.q2 +(10 rows) + +select * from ( + select 1 as x, q1, sum(q2) + from int8_tbl i1 + group by grouping sets(1, 2) +) ss +where x = 1 and q1 = 123; + x | q1 | sum +---+----+----- +(0 rows) + -- simple rescan tests select a, b, sum(v.x) from (values (1),(2)) v(x), gstest_data(v.x) diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index d8f78fcc00..16925b7914 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -172,6 +172,22 @@ select x, not x as not_x, q2 from group by grouping sets(x, q2) order by x, q2; +-- check qual push-down rules for a subquery with grouping sets +explain (verbose, costs off) +select * from ( + select 1 as x, q1, sum(q2) + from int8_tbl i1 + group by grouping sets(1, 2) +) ss +where x = 1 and q1 = 123; + +select * from ( + select 1 as x, q1, sum(q2) + from int8_tbl i1 + group by grouping sets(1, 2) +) ss +where x = 1 and q1 = 123; + -- simple rescan tests select a, b, sum(v.x)