[fix](Nereids) support aggregate function only in having statement (#34086)

SQL like

> SELECT 1 AS c1 FROM t HAVING count(1) > 0 OR c1 IS NOT NULL
This commit is contained in:
morrySnow
2024-04-25 16:42:35 +08:00
committed by yiguolei
parent a237f7ec6e
commit 75644392f4
3 changed files with 92 additions and 15 deletions

View File

@ -30,6 +30,7 @@ import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.Aggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalHaving;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
@ -158,20 +159,52 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
// Convert having to filter
RuleType.FILL_UP_HAVING_PROJECT.build(
logicalHaving(logicalProject()).then(having -> {
LogicalProject<Plan> project = having.child();
Set<Slot> projectOutputSet = project.getOutputSet();
Set<Slot> notExistedInProject = having.getExpressions().stream()
.map(Expression::getInputSlots)
.flatMap(Set::stream)
.filter(s -> !projectOutputSet.contains(s))
.collect(Collectors.toSet());
if (notExistedInProject.isEmpty()) {
return null;
if (having.getExpressions().stream().anyMatch(e -> e.containsType(AggregateFunction.class))) {
// This is very weird pattern.
// There are some aggregate functions in having, but its child is project.
// There are some slot from project in having too.
// Having should execute after project.
// But aggregate function should execute before project.
// Since no aggregate here, we should add an empty aggregate before project.
// We should push aggregate function into aggregate node first.
// Then put aggregate result slots and original project slots into new project.
// The final plan should be
// Having
// +-- Project
// +-- Aggregate
// Since aggregate node have no group by key.
// So project should not contain any slot from its original child.
// Or otherwise slot cannot find will be thrown.
LogicalProject<Plan> project = having.child();
// new an empty agg here
LogicalAggregate<Plan> agg = new LogicalAggregate<>(
ImmutableList.of(), ImmutableList.of(), project.child());
// avoid throw exception even if having have slot from its child.
// because we will add a project between having and project.
Resolver resolver = new Resolver(agg, false);
having.getConjuncts().forEach(resolver::resolve);
agg = agg.withAggOutput(resolver.getNewOutputSlots());
Set<Expression> newConjuncts = ExpressionUtils.replace(
having.getConjuncts(), resolver.getSubstitution());
ImmutableList.Builder<NamedExpression> projects = ImmutableList.builder();
projects.addAll(project.getOutputs()).addAll(agg.getOutput());
return new LogicalHaving<>(newConjuncts, new LogicalProject<>(projects.build(), agg));
} else {
LogicalProject<Plan> project = having.child();
Set<Slot> projectOutputSet = project.getOutputSet();
Set<Slot> notExistedInProject = having.getExpressions().stream()
.map(Expression::getInputSlots)
.flatMap(Set::stream)
.filter(s -> !projectOutputSet.contains(s))
.collect(Collectors.toSet());
if (notExistedInProject.isEmpty()) {
return null;
}
List<NamedExpression> projects = ImmutableList.<NamedExpression>builder()
.addAll(project.getProjects()).addAll(notExistedInProject).build();
return new LogicalProject<>(ImmutableList.copyOf(project.getOutput()),
having.withChildren(new LogicalProject<>(projects, project.child())));
}
List<NamedExpression> projects = ImmutableList.<NamedExpression>builder()
.addAll(project.getProjects()).addAll(notExistedInProject).build();
return new LogicalProject<>(ImmutableList.copyOf(project.getOutput()),
having.withChildren(new LogicalProject<>(projects, project.child())));
})
)
);
@ -184,13 +217,19 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
private final Map<Expression, Slot> substitution = Maps.newHashMap();
private final List<NamedExpression> newOutputSlots = Lists.newArrayList();
private final Map<Slot, Expression> outputSubstitutionMap;
private final boolean checkSlot;
Resolver(Aggregate aggregate) {
Resolver(Aggregate<?> aggregate, boolean checkSlot) {
outputExpressions = aggregate.getOutputExpressions();
groupByExpressions = aggregate.getGroupByExpressions();
outputSubstitutionMap = outputExpressions.stream().filter(Alias.class::isInstance)
.collect(Collectors.toMap(NamedExpression::toSlot, alias -> alias.child(0),
(k1, k2) -> k1));
this.checkSlot = checkSlot;
}
Resolver(Aggregate<?> aggregate) {
this(aggregate, true);
}
public void resolve(Expression expression) {
@ -218,7 +257,9 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
// We couldn't find the equivalent expression in output expressions and group-by expressions,
// so we should check whether the expression is valid.
if (expression instanceof SlotReference) {
throw new AnalysisException(expression.toSql() + " should be grouped by.");
if (checkSlot) {
throw new AnalysisException(expression.toSql() + " should be grouped by.");
}
} else if (expression instanceof AggregateFunction) {
if (checkWhetherNestedAggregateFunctionsExist((AggregateFunction) expression)) {
throw new AnalysisException("Aggregate functions in having clause can't be nested: "