Skip to content

Commit 4ab5c1a

Browse files
feiniaofeiafeiweixingyu12
authored and
weixingyu12
committed
[Fix](nereids) fix NormalizeAgg, change the upper project projections rewrite logic (apache#36623)
cherry-pick apache#36161 to branch-2.0 NormalizeAggregate rewrite logic has a bug, for sql like this: SELECT CASE 1 WHEN CAST( NULL AS SIGNED ) THEN NULL WHEN COUNT( DISTINCT CAST( NULL AS SIGNED ) ) THEN NULL ELSE null END ; This is the plan after NormalizeAggregate, the LogicalAggregate only output `count(DISTINCT cast(NULL as SIGNED))`apache#3, do not output cast(NULL as SIGNED)apache#2, but the upper project use cast(NULL as SIGNED)apache#2, so Doris report error "cast(NULL as SIGNED) not in aggregate's output". LogicalResultSink[29] ( outputExprs=[__case_when_0#1] ) +--LogicalProject[26] ( distinct=false, projects=[CASE WHEN (1 = cast(NULL as SIGNED)apache#2) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))apache#3) THEN NULL ELSE NULL END AS `CASE WHEN (1 = cast(NULL as SIGNED)) THEN NULL WHEN (1 = count(DISTINCT cast(NULL as SIGNED))) THEN NULL ELSE NULL END`apache#1], excepts=[] ) +--LogicalAggregate[25] ( groupByExpr=[], outputExpr=[count(DISTINCT cast(NULL as SIGNED)apache#2) AS `count(DISTINCT cast(NULL as SIGNED))`apache#3], hasRepeat=false ) +--LogicalProject[24] ( distinct=false, projects=[cast(NULL as SIGNED) AS `cast(NULL as SIGNED)`apache#2], excepts=[] ) +--LogicalOneRowRelation ( projects=[0 AS `0`#0] ) The problem is that the cast(NULL as SIGNED)apache#2 should not outputted by LogicalAggregate, cast(NULL as SIGNED) should be computed in LogicalProject. This pr change the upper project projections rewrite logic: aggregateOutputs is rewritten and become the upper-level LogicalProject projections. During the rewriting process, the expressions inside the agg function can be rewritten with expressions in aggregate function arguments and group by expressions, but the ones outside the agg function can only be rewritten with group by expressions. --------- Co-authored-by: moailing <moailing@selectdb.com>
1 parent a408597 commit 4ab5c1a

File tree

4 files changed

+91
-11
lines changed

4 files changed

+91
-11
lines changed

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java

+50-11
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,16 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan> aggregate, LogicalHaving
193193

194194
// push down 3 kinds of exprs, these pushed exprs will be used to normalize agg output later
195195
// 1. group by exprs
196-
// 2. trivalAgg children
197-
// 3. trivalAgg input slots
198-
Set<Expression> allPushDownExprs =
199-
Sets.union(groupingByExprs, Sets.union(needPushSelf, needPushInputSlots));
200-
NormalizeToSlotContext bottomSlotContext =
201-
NormalizeToSlotContext.buildContext(existsAlias, allPushDownExprs);
196+
// 2. trivialAgg children
197+
// 3. trivialAgg input slots
198+
// We need to distinguish between expressions in aggregate function arguments and group by expressions.
199+
NormalizeToSlotContext groupByExprContext = NormalizeToSlotContext.buildContext(existsAlias, groupingByExprs);
200+
Set<Alias> existsAliasAndGroupByAlias = getExistsAlias(existsAlias, groupByExprContext.getNormalizeToSlotMap());
201+
Set<Expression> argsOfAggFuncNeedPushDown = Sets.union(needPushSelf, needPushInputSlots);
202+
NormalizeToSlotContext argsOfAggFuncNeedPushDownContext = NormalizeToSlotContext
203+
.buildContext(existsAliasAndGroupByAlias, argsOfAggFuncNeedPushDown);
204+
NormalizeToSlotContext bottomSlotContext = argsOfAggFuncNeedPushDownContext.mergeContext(groupByExprContext);
205+
202206
Set<NamedExpression> pushedGroupByExprs =
203207
bottomSlotContext.pushDownToNamedExpression(groupingByExprs);
204208
Set<NamedExpression> pushedTrivalAggChildren =
@@ -258,8 +262,12 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan> aggregate, LogicalHaving
258262
aggregate.withNormalized(normalizedGroupExprs, normalizedAggOutput, bottomPlan);
259263

260264
// create upper projects by normalize all output exprs in old LogicalAggregate
265+
// In aggregateOutput, the expressions inside the agg function can be rewritten
266+
// with expressions in aggregate function arguments and group by expressions,
267+
// but the ones outside the agg function can only be rewritten with group by expressions.
268+
// After the above two rewrites are completed, use aggregate output agg functions to rewrite.
261269
List<NamedExpression> upperProjects = normalizeOutput(aggregateOutput,
262-
bottomSlotContext, normalizedAggFuncsToSlotContext);
270+
groupByExprContext, argsOfAggFuncNeedPushDownContext, normalizedAggFuncsToSlotContext);
263271

264272
// create a parent project node
265273
LogicalProject<Plan> project = new LogicalProject(upperProjects, newAggregate);
@@ -304,11 +312,18 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan> aggregate, LogicalHaving
304312
}
305313

306314
private List<NamedExpression> normalizeOutput(List<NamedExpression> aggregateOutput,
307-
NormalizeToSlotContext groupByToSlotContext, NormalizeToSlotContext normalizedAggFuncsToSlotContext) {
315+
NormalizeToSlotContext groupByToSlotContext, NormalizeToSlotContext argsOfAggFuncNeedPushDownContext,
316+
NormalizeToSlotContext normalizedAggFuncsToSlotContext) {
308317
// build upper project, use two context to do pop up, because agg output maybe contain two part:
309-
// group by keys and agg expressions
310-
List<NamedExpression> upperProjects = groupByToSlotContext
311-
.normalizeToUseSlotRefWithoutWindowFunction(aggregateOutput);
318+
// group by keys and agg expressions
319+
List<NamedExpression> upperProjects = new ArrayList<>();
320+
for (Expression expr : aggregateOutput) {
321+
Expression rewrittenExpr = expr.rewriteDownShortCircuit(
322+
e -> normalizeAggFuncChildren(
323+
argsOfAggFuncNeedPushDownContext, e));
324+
upperProjects.add((NamedExpression) rewrittenExpr);
325+
}
326+
upperProjects = groupByToSlotContext.normalizeToUseSlotRefWithoutWindowFunction(upperProjects);
312327
upperProjects = normalizedAggFuncsToSlotContext.normalizeToUseSlotRefWithoutWindowFunction(upperProjects);
313328

314329
Builder<NamedExpression> builder = new ImmutableList.Builder<>();
@@ -340,4 +355,28 @@ private List<Slot> collectAllUsedSlots(List<NamedExpression> expressions) {
340355
slots.addAll(ExpressionUtils.getInputSlotSet(expressions));
341356
return slots;
342357
}
358+
359+
private Set<Alias> getExistsAlias(Set<Alias> originAliases,
360+
Map<Expression, NormalizeToSlotTriplet> groupingExprMap) {
361+
Set<Alias> existsAlias = Sets.newHashSet();
362+
existsAlias.addAll(originAliases);
363+
for (NormalizeToSlotTriplet triplet : groupingExprMap.values()) {
364+
if (triplet.pushedExpr instanceof Alias) {
365+
Alias alias = (Alias) triplet.pushedExpr;
366+
existsAlias.add(alias);
367+
}
368+
}
369+
return existsAlias;
370+
}
371+
372+
private Expression normalizeAggFuncChildren(NormalizeToSlotContext context, Expression expr) {
373+
if (expr instanceof AggregateFunction) {
374+
AggregateFunction function = (AggregateFunction) expr;
375+
List<Expression> normalizedRealExpressions = context.normalizeToUseSlotRef(function.getArguments());
376+
function = function.withChildren(normalizedRealExpressions);
377+
return function;
378+
} else {
379+
return expr;
380+
}
381+
}
343382
}

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NormalizeToSlot.java

+11
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ public NormalizeToSlotContext(Map<Expression, NormalizeToSlotTriplet> normalizeT
4949
this.normalizeToSlotMap = normalizeToSlotMap;
5050
}
5151

52+
public Map<Expression, NormalizeToSlotTriplet> getNormalizeToSlotMap() {
53+
return normalizeToSlotMap;
54+
}
55+
56+
public NormalizeToSlotContext mergeContext(NormalizeToSlotContext context) {
57+
Map<Expression, NormalizeToSlotTriplet> newMap = Maps.newHashMap();
58+
newMap.putAll(this.normalizeToSlotMap);
59+
newMap.putAll(context.getNormalizeToSlotMap());
60+
return new NormalizeToSlotContext(newMap);
61+
}
62+
5263
/**
5364
* build normalization context by follow step.
5465
* 1. collect all exists alias by input parameters existsAliases build a reverted map: expr -> alias
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-- This file is automatically generated. You should know what you did if you want to edit this
2+
-- !test_upper_project_projections_rewrite2 --
3+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
suite("normalize_aggregate") {
18+
sql "SET enable_nereids_planner=true"
19+
sql "SET enable_fallback_to_original_planner=false"
20+
21+
sql "drop table if exists normalize_aggregate_tab"
22+
sql """CREATE TABLE normalize_aggregate_tab(col0 INTEGER, col1 INTEGER, col2 INTEGER) distributed by hash(col0) buckets 10
23+
properties('replication_num' = '1'); """
24+
qt_test_upper_project_projections_rewrite2 """
25+
SELECT - + AVG ( DISTINCT - col0 ) * - col0 FROM
26+
normalize_aggregate_tab WHERE + - col0 IS NULL GROUP BY col0 HAVING NULL IS NULL;"""
27+
}

0 commit comments

Comments
 (0)