Skip to content

Commit 7c9f29b

Browse files
authored
planner: fix having clause item that ref-ed to grouping set expression shouldn't be pushed down through Expand OP (#45651)
close #45647
1 parent 4220dfa commit 7c9f29b

File tree

4 files changed

+44
-5
lines changed

4 files changed

+44
-5
lines changed

planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_in.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,9 @@
209209
"explain format = 'brief' select grouping(a,b) from t group by a, b with rollup; -- 5. grouping function contains grouping set column a,c",
210210
"explain format = 'brief' select a, grouping(b,a) from t group by a,b with rollup; -- 6. resolve normal column a to grouping set column a'",
211211
"explain format = 'brief' select a+1, grouping(b) from t group by a+1, b with rollup; -- 7. resolve field list a+1 to grouping set column a+1",
212-
"explain format = 'brief' SELECT SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2",
213-
"explain format = 'brief' SELECT year+2, SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2"
212+
"explain format = 'brief' SELECT SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2; -- 8. order by item year+2 resolve to gby grouping expression",
213+
"explain format = 'brief' SELECT year+2, SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2; -- 9. order by item year+2 resolve to select field",
214+
"explain format = 'brief' SELECT year+2 as y, SUM(profit) as profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP having y > 2002 order by year+2, profit; -- 10. having (year+2) shouldn't be pushed down"
214215
]
215216
}
216217
]

planner/core/casetest/enforcempp/testdata/enforce_mpp_suite_out.json

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2049,7 +2049,7 @@
20492049
"Warn": null
20502050
},
20512051
{
2052-
"SQL": "explain format = 'brief' SELECT SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2",
2052+
"SQL": "explain format = 'brief' SELECT SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2; -- 8. order by item year+2 resolve to gby grouping expression",
20532053
"Plan": [
20542054
"Projection 8000.00 root Column#9",
20552055
"└─Sort 8000.00 root Column#6",
@@ -2067,7 +2067,7 @@
20672067
"Warn": null
20682068
},
20692069
{
2070-
"SQL": "explain format = 'brief' SELECT year+2, SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2",
2070+
"SQL": "explain format = 'brief' SELECT year+2, SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP order by year+2; -- 9. order by item year+2 resolve to select field",
20712071
"Plan": [
20722072
"Projection 8000.00 root Column#6->Column#10, Column#9",
20732073
"└─Sort 8000.00 root Column#6",
@@ -2083,6 +2083,25 @@
20832083
" └─TableFullScan 10000.00 mpp[tiflash] table:sales keep order:false, stats:pseudo"
20842084
],
20852085
"Warn": null
2086+
},
2087+
{
2088+
"SQL": "explain format = 'brief' SELECT year+2 as y, SUM(profit) as profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP having y > 2002 order by year+2, profit; -- 10. having (year+2) shouldn't be pushed down",
2089+
"Plan": [
2090+
"Projection 6400.00 root Column#6, Column#9",
2091+
"└─Sort 6400.00 root Column#6, Column#9",
2092+
" └─TableReader 6400.00 root MppVersion: 2, data:ExchangeSender",
2093+
" └─ExchangeSender 6400.00 mpp[tiflash] ExchangeType: PassThrough",
2094+
" └─Projection 6400.00 mpp[tiflash] Column#9, Column#6",
2095+
" └─HashAgg 6400.00 mpp[tiflash] group by:Column#20, Column#21, Column#22, funcs:sum(Column#18)->Column#9, funcs:firstrow(Column#19)->Column#6",
2096+
" └─Projection 8000.00 mpp[tiflash] cast(test.sales.profit, decimal(10,0) BINARY)->Column#18, Column#6->Column#19, Column#6->Column#20, Column#7->Column#21, gid->Column#22",
2097+
" └─ExchangeReceiver 8000.00 mpp[tiflash] ",
2098+
" └─ExchangeSender 8000.00 mpp[tiflash] ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: Column#6, collate: binary], [name: Column#7, collate: binary], [name: gid, collate: binary]",
2099+
" └─Selection 8000.00 mpp[tiflash] gt(Column#6, 2002)",
2100+
" └─Expand 10000.00 mpp[tiflash] level-projection:[test.sales.profit, <nil>->Column#6, <nil>->Column#7, 0->gid],[test.sales.profit, Column#6, <nil>->Column#7, 1->gid],[test.sales.profit, Column#6, Column#7, 3->gid]; schema: [test.sales.profit,Column#6,Column#7,gid]",
2101+
" └─Projection 10000.00 mpp[tiflash] test.sales.profit, plus(test.sales.year, 2)->Column#6, plus(test.sales.year, test.sales.profit)->Column#7",
2102+
" └─TableFullScan 10000.00 mpp[tiflash] table:sales keep order:false, stats:pseudo"
2103+
],
2104+
"Warn": null
20862105
}
20872106
]
20882107
}

planner/core/logical_plan_builder.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1325,6 +1325,13 @@ func (b *PlanBuilder) buildSelection(ctx context.Context, p LogicalPlan, where a
13251325
if err != nil {
13261326
return nil, err
13271327
}
1328+
// for case: explain SELECT year+2 as y, SUM(profit) AS profit FROM sales GROUP BY year+2, year+profit WITH ROLLUP having y > 2002;
1329+
// currently, we succeed to resolve y to (year+2), but fail to resolve (year+2) to grouping col, and to base column function: plus(year, 2) instead.
1330+
// which will cause this selection being pushed down through Expand OP itself.
1331+
//
1332+
// In expand, we will additionally project (year+2) out as a new column, let's say grouping_col here, and we wanna it can substitute any upper layer's (year+2)
1333+
expr = b.replaceGroupingFunc(expr)
1334+
13281335
p = np
13291336
if expr == nil {
13301337
continue
@@ -2805,11 +2812,11 @@ func (b *PlanBuilder) resolveHavingAndOrderBy(ctx context.Context, sel *ast.Sele
28052812
}
28062813
havingAggMapper := extractor.aggMapper
28072814
extractor.aggMapper = make(map[*ast.AggregateFuncExpr]int)
2808-
extractor.inExpr = false
28092815
// Extract agg funcs from order by clause.
28102816
if sel.OrderBy != nil {
28112817
extractor.curClause = orderByClause
28122818
for _, item := range sel.OrderBy.Items {
2819+
extractor.inExpr = false
28132820
if ast.HasWindowFlag(item.Expr) {
28142821
continue
28152822
}

planner/core/rule_predicate_push_down.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,18 @@ func isNullRejected(ctx sessionctx.Context, schema *expression.Schema, expr expr
453453
return false
454454
}
455455

456+
// PredicatePushDown implements LogicalPlan PredicatePushDown interface.
457+
func (p *LogicalExpand) PredicatePushDown(predicates []expression.Expression, opt *logicalOptimizeOp) (ret []expression.Expression, retPlan LogicalPlan) {
458+
// Note that, grouping column related predicates can't be pushed down, since grouping column has nullability change after Expand OP itself.
459+
// condition related with grouping column shouldn't be pushed down through it.
460+
// currently, since expand is adjacent to aggregate, any filter above aggregate wanted to be push down through expand only have two cases:
461+
// 1. agg function related filters. (these condition is always above aggregate)
462+
// 2. group-by item related filters. (there condition is always related with grouping sets columns, which can't be pushed down)
463+
// As a whole, we banned all the predicates pushing-down logic here that remained in Expand OP, and constructing a new selection above it if any.
464+
remained, child := p.baseLogicalPlan.PredicatePushDown(nil, opt)
465+
return append(remained, predicates...), child
466+
}
467+
456468
// PredicatePushDown implements LogicalPlan PredicatePushDown interface.
457469
func (p *LogicalProjection) PredicatePushDown(predicates []expression.Expression, opt *logicalOptimizeOp) (ret []expression.Expression, retPlan LogicalPlan) {
458470
canBePushed := make([]expression.Expression, 0, len(predicates))

0 commit comments

Comments
 (0)