postgres.git / summary / log / commit / refs
commit 7edd36eae1fd0b256a55698a16a922c4ebfac88c
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat Aug 22 18:46:40 2020 +0000
First released in: 14.0, 9.5.24, 9.6.20, 10.15, 11.10, 12.5, 13.0
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 321820061c8..f5034c582de 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1876,6 +1876,17 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
* thereby changing the partition contents and thus the window functions'
* results for rows that remain.
*
+ * 5. 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]
@@ -1920,6 +1931,10 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
if (subquery->limitOffset != NULL || subquery->limitCount != NULL)
return false;
+ /* Check point 5 */
+ if (subquery->groupClause && subquery->groupingSets)
+ return false;
+
/* Check points 3 and 4 */
if (subquery->distinctClause || subquery->hasWindowFuncs)
safetyInfo->unsafeVolatile = true;
diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 1843d2d9578..2361a6d4476 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -397,6 +397,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 1a588f415c9..e2597310557 100644
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -150,6 +150,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)
[parent: e5c674927fff]