This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch 2.1-tmp
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 9bc7902e5a0a4d8f10be1cb1e7bedbbc68b9fda6
Author: 924060929 <924060...@qq.com>
AuthorDate: Mon Apr 1 21:28:53 2024 +0800
[fix](Nereids) fix bind group by int literal (#33117)
This sql will failed because
2 in the group by will bind to 1 as col2 in BindExpression
ResolveOrdinalInOrderByAndGroupBy will replace 1 to MIN (LENGTH
(cast(age as varchar)))
CheckAnalysis will throw an exception because group by can not contains
aggregate function
select MIN (LENGTH (cast(age as varchar))), 1 AS col2
from test_bind_groupby_slots
group by 2
we should move ResolveOrdinalInOrderByAndGroupBy into BindExpression
(cherry picked from commit 3fab4496c3fefe95b4db01f300bf747080bfc3d8)
---
.../doris/nereids/jobs/executor/Analyzer.java | 2 -
.../nereids/rules/analysis/BindExpression.java | 33 ++++++-
.../ResolveOrdinalInOrderByAndGroupBy.java | 102 ---------------------
.../data/nereids_syntax_p0/bind_priority.out | 6 ++
.../suites/nereids_syntax_p0/bind_priority.groovy | 28 ++++++
5 files changed, 63 insertions(+), 108 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
index 9ad10a30aa2..a0431e066be 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
@@ -43,7 +43,6 @@ import
org.apache.doris.nereids.rules.analysis.OneRowRelationExtractAggregate;
import org.apache.doris.nereids.rules.analysis.ProjectToGlobalAggregate;
import org.apache.doris.nereids.rules.analysis.ProjectWithDistinctToAggregate;
import org.apache.doris.nereids.rules.analysis.ReplaceExpressionByChildOutput;
-import
org.apache.doris.nereids.rules.analysis.ResolveOrdinalInOrderByAndGroupBy;
import org.apache.doris.nereids.rules.analysis.SubqueryToApply;
import org.apache.doris.nereids.rules.rewrite.MergeProjects;
import org.apache.doris.nereids.rules.rewrite.SemiJoinCommute;
@@ -147,7 +146,6 @@ public class Analyzer extends AbstractBatchJobExecutor {
// please see rule BindSlotReference or BindFunction for
example
new EliminateDistinctConstant(),
new ProjectWithDistinctToAggregate(),
- new ResolveOrdinalInOrderByAndGroupBy(),
new ReplaceExpressionByChildOutput(),
new OneRowRelationExtractAggregate()
),
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
index 6211f493eaf..43f89d5b010 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
@@ -56,6 +56,7 @@ import
org.apache.doris.nereids.trees.expressions.functions.scalar.GroupingScala
import org.apache.doris.nereids.trees.expressions.functions.scalar.Lambda;
import
org.apache.doris.nereids.trees.expressions.functions.scalar.StructElement;
import
org.apache.doris.nereids.trees.expressions.functions.table.TableValuedFunction;
+import org.apache.doris.nereids.trees.expressions.literal.IntegerLikeLiteral;
import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
import org.apache.doris.nereids.trees.plans.AbstractPlan;
import org.apache.doris.nereids.trees.plans.JoinType;
@@ -486,11 +487,12 @@ public class BindExpression implements
AnalysisRuleFactory {
LogicalSort<LogicalSetOperation> sort = ctx.root;
CascadesContext cascadesContext = ctx.cascadesContext;
+ List<Slot> childOutput = sort.child().getOutput();
SimpleExprAnalyzer analyzer = buildSimpleExprAnalyzer(
sort, cascadesContext, sort.children(), true, true);
Builder<OrderKey> boundKeys =
ImmutableList.builderWithExpectedSize(sort.getOrderKeys().size());
for (OrderKey orderKey : sort.getOrderKeys()) {
- Expression boundKey = analyzer.analyze(orderKey.getExpr());
+ Expression boundKey = bindWithOrdinal(orderKey.getExpr(),
analyzer, childOutput);
boundKeys.add(orderKey.withExpression(boundKey));
}
return new LogicalSort<>(boundKeys.build(), sort.child());
@@ -699,7 +701,11 @@ public class BindExpression implements AnalysisRuleFactory
{
return useOutputExpr.build();
});
- List<Expression> boundGroupBy = analyzer.analyzeToList(groupBy);
+ ImmutableList.Builder<Expression> boundGroupByBuilder =
ImmutableList.builderWithExpectedSize(groupBy.size());
+ for (Expression key : groupBy) {
+ boundGroupByBuilder.add(bindWithOrdinal(key, analyzer,
boundAggOutput));
+ }
+ List<Expression> boundGroupBy = boundGroupByBuilder.build();
checkIfOutputAliasNameDuplicatedForGroupBy(boundGroupBy,
boundAggOutput);
return boundGroupBy;
}
@@ -723,6 +729,9 @@ public class BindExpression implements AnalysisRuleFactory {
private Plan
bindSortWithoutSetOperation(MatchingContext<LogicalSort<Plan>> ctx) {
LogicalSort<Plan> sort = ctx.root;
Plan input = sort.child();
+
+ List<Slot> childOutput = input.getOutput();
+
// we should skip LogicalHaving to bind slot in LogicalSort;
if (input instanceof LogicalHaving) {
input = input.child(0);
@@ -744,7 +753,8 @@ public class BindExpression implements AnalysisRuleFactory {
// group by col1
// order by col1; # order by order_col1
// bind order_col1 with alias_col1, then, bind it with inner_col1
- Scope inputScope = toScope(cascadesContext, input.getOutput());
+ List<Slot> inputSlots = input.getOutput();
+ Scope inputScope = toScope(cascadesContext, inputSlots);
final Plan finalInput = input;
Supplier<Scope> inputChildrenScope = Suppliers.memoize(
@@ -766,7 +776,7 @@ public class BindExpression implements AnalysisRuleFactory {
Builder<OrderKey> boundOrderKeys =
ImmutableList.builderWithExpectedSize(sort.getOrderKeys().size());
for (OrderKey orderKey : sort.getOrderKeys()) {
- Expression boundKey = analyzer.analyze(orderKey.getExpr());
+ Expression boundKey = bindWithOrdinal(orderKey.getExpr(),
analyzer, childOutput);
boundOrderKeys.add(orderKey.withExpression(boundKey));
}
return new LogicalSort<>(boundOrderKeys.build(), sort.child());
@@ -858,6 +868,21 @@ public class BindExpression implements AnalysisRuleFactory
{
unboundFunction.getDbName(), unboundFunction.getName());
}
+ private Expression bindWithOrdinal(
+ Expression unbound, SimpleExprAnalyzer analyzer, List<? extends
Expression> boundSelectOutput) {
+ if (unbound instanceof IntegerLikeLiteral) {
+ int ordinal = ((IntegerLikeLiteral) unbound).getIntValue();
+ if (ordinal >= 1 && ordinal <= boundSelectOutput.size()) {
+ Expression boundSelectItem = boundSelectOutput.get(ordinal -
1);
+ return boundSelectItem instanceof Alias ?
boundSelectItem.child(0) : boundSelectItem;
+ } else {
+ return unbound; // bound literal
+ }
+ } else {
+ return analyzer.analyze(unbound);
+ }
+ }
+
private <E extends Expression> E checkBoundExceptLambda(E expression, Plan
plan) {
if (expression instanceof Lambda) {
return expression;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ResolveOrdinalInOrderByAndGroupBy.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ResolveOrdinalInOrderByAndGroupBy.java
deleted file mode 100644
index 1cefd203ff7..00000000000
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ResolveOrdinalInOrderByAndGroupBy.java
+++ /dev/null
@@ -1,102 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package org.apache.doris.nereids.rules.analysis;
-
-import org.apache.doris.nereids.properties.OrderKey;
-import org.apache.doris.nereids.rules.Rule;
-import org.apache.doris.nereids.rules.RuleType;
-import org.apache.doris.nereids.trees.expressions.Alias;
-import org.apache.doris.nereids.trees.expressions.Expression;
-import org.apache.doris.nereids.trees.expressions.NamedExpression;
-import org.apache.doris.nereids.trees.expressions.Slot;
-import org.apache.doris.nereids.trees.expressions.literal.IntegerLikeLiteral;
-import org.apache.doris.nereids.trees.plans.Plan;
-import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
-import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
-
-import com.google.common.collect.ImmutableList;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * SELECT col1, col2 FROM t1 ORDER BY 1 -> SELECT col1, col2 FROM t1 ORDER BY
col1
- * SELECT col1, SUM(col2) FROM t1 GROUP BY 1 -> SELECT col1, SUM(col2) FROM t1
GROUP BY col1
- */
-public class ResolveOrdinalInOrderByAndGroupBy implements AnalysisRuleFactory {
-
- @Override
- public List<Rule> buildRules() {
- return ImmutableList.<Rule>builder()
- .add(RuleType.RESOLVE_ORDINAL_IN_ORDER_BY.build(
- logicalSort().thenApply(ctx -> {
- LogicalSort<Plan> sort = ctx.root;
- List<Slot> childOutput = sort.child().getOutput();
- List<OrderKey> orderKeys = sort.getOrderKeys();
- List<OrderKey> orderKeysWithoutOrd = new
ArrayList<>();
- for (OrderKey k : orderKeys) {
- Expression expression = k.getExpr();
- if (expression instanceof IntegerLikeLiteral) {
- IntegerLikeLiteral i =
(IntegerLikeLiteral) expression;
- int ord = i.getIntValue();
- checkOrd(ord, childOutput.size());
- orderKeysWithoutOrd
- .add(new
OrderKey(childOutput.get(ord - 1), k.isAsc(), k.isNullFirst()));
- } else {
- orderKeysWithoutOrd.add(k);
- }
- }
- return sort.withOrderKeys(orderKeysWithoutOrd);
- })
- ))
- .add(RuleType.RESOLVE_ORDINAL_IN_GROUP_BY.build(
-
logicalAggregate().whenNot(LogicalAggregate::isOrdinalIsResolved).thenApply(ctx
-> {
- LogicalAggregate<Plan> agg = ctx.root;
- List<NamedExpression> aggOutput =
agg.getOutputExpressions();
- List<Expression> groupByWithoutOrd = new
ArrayList<>();
- boolean ordExists = false;
- for (Expression groupByExpr :
agg.getGroupByExpressions()) {
- if (groupByExpr instanceof IntegerLikeLiteral)
{
- IntegerLikeLiteral i =
(IntegerLikeLiteral) groupByExpr;
- int ord = i.getIntValue();
- checkOrd(ord, aggOutput.size());
- Expression aggExpr = aggOutput.get(ord -
1);
- if (aggExpr instanceof Alias) {
- aggExpr = ((Alias) aggExpr).child();
- }
- groupByWithoutOrd.add(aggExpr);
- ordExists = true;
- } else {
- groupByWithoutOrd.add(groupByExpr);
- }
- }
- if (ordExists) {
- return new
LogicalAggregate<>(groupByWithoutOrd, agg.getOutputExpressions(),
- true, agg.child());
- } else {
- return agg;
- }
- }))).build();
- }
-
- private void checkOrd(int ord, int childOutputSize) {
- if (ord < 1 || ord > childOutputSize) {
- throw new IllegalStateException(String.format("ordinal exceeds
number of items in select list: %s", ord));
- }
- }
-}
diff --git a/regression-test/data/nereids_syntax_p0/bind_priority.out
b/regression-test/data/nereids_syntax_p0/bind_priority.out
index 53432880c24..56706546bab 100644
--- a/regression-test/data/nereids_syntax_p0/bind_priority.out
+++ b/regression-test/data/nereids_syntax_p0/bind_priority.out
@@ -85,3 +85,9 @@ all 2
-- !having_bind_group_by --
2 1
+-- !sql --
+2 1
+
+-- !sql --
+2 1
+
diff --git a/regression-test/suites/nereids_syntax_p0/bind_priority.groovy
b/regression-test/suites/nereids_syntax_p0/bind_priority.groovy
index 84bab14eba0..769f1771982 100644
--- a/regression-test/suites/nereids_syntax_p0/bind_priority.groovy
+++ b/regression-test/suites/nereids_syntax_p0/bind_priority.groovy
@@ -309,4 +309,32 @@ suite("bind_priority") {
having pk = 2;
"""
}()
+
+ def bindGroupBy = {
+ sql "drop table if exists test_bind_groupby_slots"
+
+ sql """create table test_bind_groupby_slots
+ (id int, age int)
+ distributed by hash(id)
+ properties('replication_num'='1');
+ """
+ sql "insert into test_bind_groupby_slots values(1, 10), (2, 20), (3,
30);"
+
+ order_qt_sql "select MIN (LENGTH (cast(age as varchar))), 1 AS col2
from test_bind_groupby_slots group by 2"
+ }()
+
+
+
+ def bindOrderBy = {
+ sql "drop table if exists test_bind_orderby_slots"
+
+ sql """create table test_bind_orderby_slots
+ (id int, age int)
+ distributed by hash(id)
+ properties('replication_num'='1');
+ """
+ sql "insert into test_bind_orderby_slots values(1, 10), (2, 20), (3,
30);"
+
+ order_qt_sql "select MIN (LENGTH (cast(age as varchar))), 1 AS col2
from test_bind_orderby_slots order by 2"
+ }()
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
