Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d3701bc

Browse filesBrowse files
tglsfdctglsfdc
committed
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
1 parent 190fb57 commit d3701bc
Copy full SHA for d3701bc

File tree

Expand file treeCollapse file tree

3 files changed

+63
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+63
-0
lines changed
Open diff view settings
Collapse file

‎src/backend/optimizer/path/allpaths.c‎

Copy file name to clipboardExpand all lines: src/backend/optimizer/path/allpaths.c
+15Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,6 +2278,17 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
22782278
* thereby changing the partition contents and thus the window functions'
22792279
* results for rows that remain.
22802280
*
2281+
* 5. If the subquery has nonempty grouping sets, we cannot push down any
2282+
* quals. The concern here is that a qual referencing a "constant" grouping
2283+
* column could get constant-folded, which would be improper because the value
2284+
* is potentially nullable by grouping-set expansion. This restriction could
2285+
* be removed if we had a parsetree representation that shows that such
2286+
* grouping columns are not really constant. (There are other ideas that
2287+
* could be used to relax this restriction, but that's the approach most
2288+
* likely to get taken in the future. Note that there's not much to be gained
2289+
* so long as subquery_planner can't move HAVING clauses to WHERE within such
2290+
* a subquery.)
2291+
*
22812292
* In addition, we make several checks on the subquery's output columns to see
22822293
* if it is safe to reference them in pushed-down quals. If output column k
22832294
* is found to be unsafe to reference, we set safetyInfo->unsafeColumns[k]
@@ -2322,6 +2333,10 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
23222333
if (subquery->limitOffset != NULL || subquery->limitCount != NULL)
23232334
return false;
23242335

2336+
/* Check point 5 */
2337+
if (subquery->groupClause && subquery->groupingSets)
2338+
return false;
2339+
23252340
/* Check points 3 and 4 */
23262341
if (subquery->distinctClause || subquery->hasWindowFuncs)
23272342
safetyInfo->unsafeVolatile = true;
Collapse file

‎src/test/regress/expected/groupingsets.out‎

Copy file name to clipboardExpand all lines: src/test/regress/expected/groupingsets.out
+32Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,38 @@ select x, not x as not_x, q2 from
426426
| | 4567890123456789
427427
(5 rows)
428428

429+
-- check qual push-down rules for a subquery with grouping sets
430+
explain (verbose, costs off)
431+
select * from (
432+
select 1 as x, q1, sum(q2)
433+
from int8_tbl i1
434+
group by grouping sets(1, 2)
435+
) ss
436+
where x = 1 and q1 = 123;
437+
QUERY PLAN
438+
--------------------------------------------
439+
Subquery Scan on ss
440+
Output: ss.x, ss.q1, ss.sum
441+
Filter: ((ss.x = 1) AND (ss.q1 = 123))
442+
-> GroupAggregate
443+
Output: (1), i1.q1, sum(i1.q2)
444+
Group Key: 1
445+
Sort Key: i1.q1
446+
Group Key: i1.q1
447+
-> Seq Scan on public.int8_tbl i1
448+
Output: 1, i1.q1, i1.q2
449+
(10 rows)
450+
451+
select * from (
452+
select 1 as x, q1, sum(q2)
453+
from int8_tbl i1
454+
group by grouping sets(1, 2)
455+
) ss
456+
where x = 1 and q1 = 123;
457+
x | q1 | sum
458+
---+----+-----
459+
(0 rows)
460+
429461
-- simple rescan tests
430462
select a, b, sum(v.x)
431463
from (values (1),(2)) v(x), gstest_data(v.x)
Collapse file

‎src/test/regress/sql/groupingsets.sql‎

Copy file name to clipboardExpand all lines: src/test/regress/sql/groupingsets.sql
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,22 @@ select x, not x as not_x, q2 from
161161
group by grouping sets(x, q2)
162162
order by x, q2;
163163

164+
-- check qual push-down rules for a subquery with grouping sets
165+
explain (verbose, costs off)
166+
select * from (
167+
select 1 as x, q1, sum(q2)
168+
from int8_tbl i1
169+
group by grouping sets(1, 2)
170+
) ss
171+
where x = 1 and q1 = 123;
172+
173+
select * from (
174+
select 1 as x, q1, sum(q2)
175+
from int8_tbl i1
176+
group by grouping sets(1, 2)
177+
) ss
178+
where x = 1 and q1 = 123;
179+
164180
-- simple rescan tests
165181

166182
select a, b, sum(v.x)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.