Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit de627ad

Browse files
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 47de6ac commit de627ad

File tree

3 files changed

+63
-0
lines changed

3 files changed

+63
-0
lines changed

src/backend/optimizer/path/allpaths.c

+15
Original file line numberDiff line numberDiff line change
@@ -3182,6 +3182,17 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
31823182
* volatile qual could succeed for some SRF output rows and fail for others,
31833183
* a behavior that cannot occur if it's evaluated before SRF expansion.
31843184
*
3185+
* 6. If the subquery has nonempty grouping sets, we cannot push down any
3186+
* quals. The concern here is that a qual referencing a "constant" grouping
3187+
* column could get constant-folded, which would be improper because the value
3188+
* is potentially nullable by grouping-set expansion. This restriction could
3189+
* be removed if we had a parsetree representation that shows that such
3190+
* grouping columns are not really constant. (There are other ideas that
3191+
* could be used to relax this restriction, but that's the approach most
3192+
* likely to get taken in the future. Note that there's not much to be gained
3193+
* so long as subquery_planner can't move HAVING clauses to WHERE within such
3194+
* a subquery.)
3195+
*
31853196
* In addition, we make several checks on the subquery's output columns to see
31863197
* if it is safe to reference them in pushed-down quals. If output column k
31873198
* is found to be unsafe to reference, we set safetyInfo->unsafeColumns[k]
@@ -3226,6 +3237,10 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
32263237
if (subquery->limitOffset != NULL || subquery->limitCount != NULL)
32273238
return false;
32283239

3240+
/* Check point 6 */
3241+
if (subquery->groupClause && subquery->groupingSets)
3242+
return false;
3243+
32293244
/* Check points 3, 4, and 5 */
32303245
if (subquery->distinctClause ||
32313246
subquery->hasWindowFuncs ||

src/test/regress/expected/groupingsets.out

+32
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,38 @@ select x, not x as not_x, q2 from
434434
| | 4567890123456789
435435
(5 rows)
436436

437+
-- check qual push-down rules for a subquery with grouping sets
438+
explain (verbose, costs off)
439+
select * from (
440+
select 1 as x, q1, sum(q2)
441+
from int8_tbl i1
442+
group by grouping sets(1, 2)
443+
) ss
444+
where x = 1 and q1 = 123;
445+
QUERY PLAN
446+
--------------------------------------------
447+
Subquery Scan on ss
448+
Output: ss.x, ss.q1, ss.sum
449+
Filter: ((ss.x = 1) AND (ss.q1 = 123))
450+
-> GroupAggregate
451+
Output: (1), i1.q1, sum(i1.q2)
452+
Group Key: 1
453+
Sort Key: i1.q1
454+
Group Key: i1.q1
455+
-> Seq Scan on public.int8_tbl i1
456+
Output: 1, i1.q1, i1.q2
457+
(10 rows)
458+
459+
select * from (
460+
select 1 as x, q1, sum(q2)
461+
from int8_tbl i1
462+
group by grouping sets(1, 2)
463+
) ss
464+
where x = 1 and q1 = 123;
465+
x | q1 | sum
466+
---+----+-----
467+
(0 rows)
468+
437469
-- simple rescan tests
438470
select a, b, sum(v.x)
439471
from (values (1),(2)) v(x), gstest_data(v.x)

src/test/regress/sql/groupingsets.sql

+16
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,22 @@ select x, not x as not_x, q2 from
172172
group by grouping sets(x, q2)
173173
order by x, q2;
174174

175+
-- check qual push-down rules for a subquery with grouping sets
176+
explain (verbose, costs off)
177+
select * from (
178+
select 1 as x, q1, sum(q2)
179+
from int8_tbl i1
180+
group by grouping sets(1, 2)
181+
) ss
182+
where x = 1 and q1 = 123;
183+
184+
select * from (
185+
select 1 as x, q1, sum(q2)
186+
from int8_tbl i1
187+
group by grouping sets(1, 2)
188+
) ss
189+
where x = 1 and q1 = 123;
190+
175191
-- simple rescan tests
176192

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

0 commit comments

Comments
 (0)