Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix](nereids)group by expr may be bound twice in bind agg slot #28771

Merged
merged 2 commits into from
Dec 25, 2023
Merged

[fix](nereids)group by expr may be bound twice in bind agg slot #28771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
171c4dd
[fix](nereids)group by expr may be bound twice in bind agg slot
starocean999 Dec 21, 2023
a2f3342
fix failed cast
starocean999 Dec 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,18 +292,32 @@ protected boolean condition(Rule rule, Plan plan) {
if (alias.child().anyMatch(expr -> expr instanceof AggregateFunction)) {
continue;
}
// NOTICE: must use unbound expressions, because we will bind them in binding group by expr.
childOutputsToExpr.putIfAbsent(alias.getName(), agg.getOutputExpressions().get(i).child(0));
/*
Alias(x) has been bound by binding agg's output
we add x to childOutputsToExpr, so when binding group by exprs later, we can use x directly
and won't bind it again
select
p_cycle_time / (select max(p_cycle_time) from log_event_8) as 'x',
count(distinct case_id) as 'y'
from
log_event_8
group by
x
*/
childOutputsToExpr.putIfAbsent(alias.getName(), output.get(i).child(0));
}

Set<Expression> boundedGroupByExpressions = Sets.newHashSet();
List<Expression> replacedGroupBy = agg.getGroupByExpressions().stream()
.map(groupBy -> {
if (groupBy instanceof UnboundSlot) {
UnboundSlot unboundSlot = (UnboundSlot) groupBy;
if (unboundSlot.getNameParts().size() == 1) {
String name = unboundSlot.getNameParts().get(0);
if (childOutputsToExpr.containsKey(name)) {
return childOutputsToExpr.get(name);
Expression expression = childOutputsToExpr.get(name);
boundedGroupByExpressions.add(expression);
return expression;
}
}
}
Expand Down Expand Up @@ -336,16 +350,24 @@ protected boolean condition(Rule rule, Plan plan) {

List<Expression> groupBy = replacedGroupBy.stream()
.map(expression -> {
Expression e = binder.bind(expression);
if (e instanceof UnboundSlot) {
return childBinder.bind(e);
if (boundedGroupByExpressions.contains(expression)) {
// expr has been bound by binding agg's output
return expression;
} else {
// bind slot for unbound exprs
Expression e = binder.bind(expression);
if (e instanceof UnboundSlot) {
return childBinder.bind(e);
}
return e;
}
return e;
})
.collect(Collectors.toList());
groupBy.forEach(expression -> checkBoundExceptLambda(expression, ctx.root));
groupBy = groupBy.stream()
.map(expr -> bindFunction(expr, ctx.root, ctx.cascadesContext))
// bind function for unbound exprs or return old expr if it's bound by binding agg's output
.map(expr -> boundedGroupByExpressions.contains(expr) ? expr
: bindFunction(expr, ctx.root, ctx.cascadesContext))
.collect(ImmutableList.toImmutableList());
checkIfOutputAliasNameDuplicatedForGroupBy(groupBy, output);
return agg.withGroupByAndOutput(groupBy, output);
Expand Down
11 changes: 11 additions & 0 deletions regression-test/suites/nereids_syntax_p0/analyze_agg.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,15 @@ suite("analyze_agg") {

// should not bind g /g in group by again, otherwise will throw exception
sql "select g / g as nu, sum(c) from t2 group by nu"
sql """
select
1,
id / (select max(id) from t2) as 'x',
count(distinct c) as 'y'
from
t2
group by
1,
x
"""
}
Loading