This is an automated email from the ASF dual-hosted git repository.
huajianlan pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 8409f24062c [fix](Nereids) fix fold constant by be return type
mismatched (#39723)(#41164)(#41331)(#41546) (#41838)
8409f24062c is described below
commit 8409f24062c868280f5fc4e9be4a73b5a24ff48a
Author: LiBinfeng <[email protected]>
AuthorDate: Fri Oct 18 20:34:03 2024 +0800
[fix](Nereids) fix fold constant by be return type mismatched
(#39723)(#41164)(#41331)(#41546) (#41838)
cherry-pick: #39723 #41164 #41331 #41546 because later problem is intro by
prev one, so put them together
when using fold constant by be,
the return type of substring('123456',1, 3) would changed to be text, which
we want it to be 3 remove windowframe in window expression to avoid folding
constant on be
---
.../doris/nereids/parser/LogicalPlanBuilder.java | 4 +-
.../doris/nereids/properties/SelectHintSetVar.java | 41 ++++++++++
.../rules/analysis/EliminateLogicalSelectHint.java | 40 +---------
.../expression/rules/FoldConstantRuleOnBE.java | 18 +++--
.../trees/expressions/WindowExpression.java | 4 +-
.../glue/translator/ExpressionTranslatorTest.java | 3 +-
.../doris/nereids/preprocess/SelectHintTest.java | 88 ----------------------
.../org/apache/doris/qe/SessionVariablesTest.java | 8 ++
.../fold_constant/fold_constant_by_be.groovy | 8 +-
.../suites/nereids_syntax_p0/system_var.groovy | 2 +-
10 files changed, 76 insertions(+), 140 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index 05e8feabd7f..3a11d5307ea 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -3140,7 +3140,9 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
parameters.put(parameterName, value);
}
}
- hints.add(new SelectHintSetVar(hintName, parameters));
+ SelectHintSetVar setVar = new
SelectHintSetVar(hintName, parameters);
+
setVar.setVarOnceInSql(ConnectContext.get().getStatementContext());
+ hints.add(setVar);
break;
case "leading":
List<String> leadingParameters = new
ArrayList<String>();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/SelectHintSetVar.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/SelectHintSetVar.java
index 8c08dcf378f..231e2fdbb92 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/SelectHintSetVar.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/SelectHintSetVar.java
@@ -17,6 +17,13 @@
package org.apache.doris.nereids.properties;
+import org.apache.doris.analysis.SetVar;
+import org.apache.doris.analysis.StringLiteral;
+import org.apache.doris.nereids.StatementContext;
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.qe.SessionVariable;
+import org.apache.doris.qe.VariableMgr;
+
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
@@ -38,6 +45,40 @@ public class SelectHintSetVar extends SelectHint {
return parameters;
}
+ /**
+ * set session variable in sql level
+ * @param context statement context
+ */
+ public void setVarOnceInSql(StatementContext context) {
+ SessionVariable sessionVariable =
context.getConnectContext().getSessionVariable();
+ // set temporary session value, and then revert value in the 'finally
block' of StmtExecutor#execute
+ sessionVariable.setIsSingleSetVar(true);
+ for (Map.Entry<String, Optional<String>> kv :
getParameters().entrySet()) {
+ String key = kv.getKey();
+ Optional<String> value = kv.getValue();
+ if (value.isPresent()) {
+ try {
+ VariableMgr.setVar(sessionVariable, new SetVar(key, new
StringLiteral(value.get())));
+ context.invalidCache(key);
+ } catch (Throwable t) {
+ throw new AnalysisException("Can not set session variable
'"
+ + key + "' = '" + value.get() + "'", t);
+ }
+ }
+ }
+ // if sv set enable_nereids_planner=true and hint set
enable_nereids_planner=false, we should set
+ // enable_fallback_to_original_planner=true and revert it after
executing.
+ // throw exception to fall back to original planner
+ if (!sessionVariable.isEnableNereidsPlanner()) {
+ try {
+ sessionVariable.enableFallbackToOriginalPlannerOnce();
+ } catch (Throwable t) {
+ throw new AnalysisException("failed to set fallback to
original planner to true", t);
+ }
+ throw new AnalysisException("The nereids is disabled in this sql,
fallback to original planner");
+ }
+ }
+
@Override
public String toString() {
String kvString = parameters
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/EliminateLogicalSelectHint.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/EliminateLogicalSelectHint.java
index 13eee57637b..35081a9b610 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/EliminateLogicalSelectHint.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/EliminateLogicalSelectHint.java
@@ -17,12 +17,9 @@
package org.apache.doris.nereids.rules.analysis;
-import org.apache.doris.analysis.SetVar;
-import org.apache.doris.analysis.StringLiteral;
import org.apache.doris.common.DdlException;
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.StatementContext;
-import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.hint.Hint;
import org.apache.doris.nereids.hint.LeadingHint;
import org.apache.doris.nereids.hint.OrderedHint;
@@ -39,15 +36,10 @@ import
org.apache.doris.nereids.rules.rewrite.OneRewriteRuleFactory;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalSelectHint;
import org.apache.doris.qe.ConnectContext;
-import org.apache.doris.qe.SessionVariable;
-import org.apache.doris.qe.VariableMgr;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.Map.Entry;
-import java.util.Optional;
-
/**
* eliminate logical select hint and set them to cascade context
*/
@@ -61,7 +53,7 @@ public class EliminateLogicalSelectHint extends
OneRewriteRuleFactory {
for (SelectHint hint : selectHintPlan.getHints()) {
String hintName = hint.getHintName();
if (hintName.equalsIgnoreCase("SET_VAR")) {
- setVar((SelectHintSetVar) hint, ctx.statementContext);
+ ((SelectHintSetVar)
hint).setVarOnceInSql(ctx.statementContext);
} else if (hintName.equalsIgnoreCase("ORDERED")) {
try {
ctx.cascadesContext.getConnectContext().getSessionVariable()
@@ -90,36 +82,6 @@ public class EliminateLogicalSelectHint extends
OneRewriteRuleFactory {
}).toRule(RuleType.ELIMINATE_LOGICAL_SELECT_HINT);
}
- private void setVar(SelectHintSetVar selectHint, StatementContext context)
{
- SessionVariable sessionVariable =
context.getConnectContext().getSessionVariable();
- // set temporary session value, and then revert value in the 'finally
block' of StmtExecutor#execute
- sessionVariable.setIsSingleSetVar(true);
- for (Entry<String, Optional<String>> kv :
selectHint.getParameters().entrySet()) {
- String key = kv.getKey();
- Optional<String> value = kv.getValue();
- if (value.isPresent()) {
- try {
- VariableMgr.setVar(sessionVariable, new SetVar(key, new
StringLiteral(value.get())));
- context.invalidCache(key);
- } catch (Throwable t) {
- throw new AnalysisException("Can not set session variable
'"
- + key + "' = '" + value.get() + "'", t);
- }
- }
- }
- // if sv set enable_nereids_planner=true and hint set
enable_nereids_planner=false, we should set
- // enable_fallback_to_original_planner=true and revert it after
executing.
- // throw exception to fall back to original planner
- if (!sessionVariable.isEnableNereidsPlanner()) {
- try {
- sessionVariable.enableFallbackToOriginalPlannerOnce();
- } catch (Throwable t) {
- throw new AnalysisException("failed to set fallback to
original planner to true", t);
- }
- throw new AnalysisException("The nereids is disabled in this sql,
fallback to original planner");
- }
- }
-
private void extractLeading(SelectHintLeading selectHint, CascadesContext
context,
StatementContext statementContext,
LogicalSelectHint<Plan> selectHintPlan) {
LeadingHint hint = new LeadingHint("Leading",
selectHint.getParameters(), selectHint.toString());
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
index 2852c1c4415..c614b50c80e 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java
@@ -175,14 +175,23 @@ public class FoldConstantRuleOnBE implements
ExpressionPatternRuleFactory {
if (newChild != child) {
hasNewChildren = true;
}
- newChildren.add(newChild);
+ if (!newChild.getDataType().equals(child.getDataType())) {
+ try {
+ newChildren.add(newChild.castTo(child.getDataType()));
+ } catch (Exception e) {
+ LOG.warn("expression of type {} cast to {} failed. ",
newChild.getDataType(), child.getDataType());
+ newChildren.add(newChild);
+ }
+ } else {
+ newChildren.add(newChild);
+ }
}
return hasNewChildren ? root.withChildren(newChildren) : root;
}
private static void collectConst(Expression expr, Map<String, Expression>
constMap,
Map<String, TExpr> tExprMap, IdGenerator<ExprId> idGenerator) {
- if (expr.isConstant() && !shouldSkipFold(expr)) {
+ if (expr.isConstant() && !expr.isLiteral() && !expr.anyMatch(e ->
shouldSkipFold((Expression) e))) {
String id = idGenerator.getNextId().toString();
constMap.put(id, expr);
Expr staleExpr;
@@ -208,11 +217,6 @@ public class FoldConstantRuleOnBE implements
ExpressionPatternRuleFactory {
// Some expressions should not do constant folding
private static boolean shouldSkipFold(Expression expr) {
- // Skip literal expr
- if (expr.isLiteral()) {
- return true;
- }
-
// Frontend can not represent those types
if (expr.getDataType().isAggStateType() ||
expr.getDataType().isObjectType()
|| expr.getDataType().isVariantType() ||
expr.getDataType().isTimeLikeType()
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/WindowExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/WindowExpression.java
index 457c6d3a645..5bea07fff00 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/WindowExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/WindowExpression.java
@@ -66,7 +66,6 @@ public class WindowExpression extends Expression {
.add(function)
.addAll(partitionKeys)
.addAll(orderKeys)
- .add(windowFrame)
.build());
this.function = function;
this.partitionKeys = ImmutableList.copyOf(partitionKeys);
@@ -153,6 +152,9 @@ public class WindowExpression extends Expression {
if (index < children.size()) {
return new WindowExpression(func, partitionKeys, orderKeys,
(WindowFrame) children.get(index));
}
+ if (windowFrame.isPresent()) {
+ return new WindowExpression(func, partitionKeys, orderKeys,
windowFrame.get());
+ }
return new WindowExpression(func, partitionKeys, orderKeys);
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/ExpressionTranslatorTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/ExpressionTranslatorTest.java
index cdea9eea5d3..22add37d8eb 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/ExpressionTranslatorTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/ExpressionTranslatorTest.java
@@ -23,7 +23,6 @@ import org.apache.doris.analysis.Expr;
import org.apache.doris.analysis.IntLiteral;
import org.apache.doris.catalog.Function.NullableMode;
import org.apache.doris.catalog.Type;
-import org.apache.doris.common.AnalysisException;
import org.apache.doris.nereids.trees.expressions.BitNot;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
@@ -33,7 +32,7 @@ import org.junit.jupiter.api.Test;
public class ExpressionTranslatorTest {
@Test
- public void testUnaryArithmetic() throws AnalysisException {
+ public void testUnaryArithmetic() throws Exception {
BitNot bitNot = new BitNot(new IntegerLiteral(1));
ExpressionTranslator translator = ExpressionTranslator.INSTANCE;
Expr actual = translator.visitUnaryArithmetic(bitNot, null);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/preprocess/SelectHintTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/preprocess/SelectHintTest.java
deleted file mode 100644
index 62965ac27b3..00000000000
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/preprocess/SelectHintTest.java
+++ /dev/null
@@ -1,88 +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.preprocess;
-
-import org.apache.doris.catalog.Env;
-import org.apache.doris.nereids.NereidsPlanner;
-import org.apache.doris.nereids.StatementContext;
-import org.apache.doris.nereids.exceptions.AnalysisException;
-import org.apache.doris.nereids.parser.NereidsParser;
-import org.apache.doris.nereids.properties.PhysicalProperties;
-import org.apache.doris.qe.ConnectContext;
-import org.apache.doris.qe.OriginStatement;
-import org.apache.doris.qe.SessionVariable;
-import org.apache.doris.qe.StmtExecutor;
-import org.apache.doris.thrift.TUniqueId;
-
-import mockit.Expectations;
-import mockit.Mock;
-import mockit.MockUp;
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
-
-public class SelectHintTest {
-
- @BeforeAll
- public static void init() {
- ConnectContext ctx = new ConnectContext();
- new MockUp<ConnectContext>() {
- @Mock
- public ConnectContext get() {
- return ctx;
- }
- };
- new MockUp<Env>() {
- @Mock
- public boolean isMaster() {
- return true;
- }
- };
- }
-
- @Test
- public void testFallbackToOriginalPlanner() throws Exception {
- String sql = " SELECT /*+ SET_VAR(enable_nereids_planner=\"false\") */
1";
-
- ConnectContext ctx = new ConnectContext();
- ctx.setEnv(Env.getCurrentEnv());
- StatementContext statementContext = new StatementContext(ctx, new
OriginStatement(sql, 0));
- SessionVariable sv = ctx.getSessionVariable();
- Assertions.assertNotNull(sv);
- sv.setEnableNereidsPlanner(true);
- sv.enableFallbackToOriginalPlanner = false;
- Assertions.assertThrows(AnalysisException.class, () -> new
NereidsPlanner(statementContext)
- .planWithLock(new NereidsParser().parseSingle(sql),
PhysicalProperties.ANY));
-
- // manually recover sv
- sv.setEnableNereidsPlanner(true);
- sv.enableFallbackToOriginalPlanner = false;
- StmtExecutor stmtExecutor = new StmtExecutor(ctx, sql);
-
- new Expectations(stmtExecutor) {
- {
- stmtExecutor.executeByLegacy((TUniqueId) any);
- }
- };
-
- stmtExecutor.execute();
-
- Assertions.assertTrue(sv.isEnableNereidsPlanner());
- Assertions.assertFalse(sv.enableFallbackToOriginalPlanner);
- }
-}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java
index bad7842e01e..541b474c74b 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java
@@ -27,6 +27,7 @@ import org.apache.doris.common.PatternMatcher;
import org.apache.doris.common.PatternMatcherWrapper;
import org.apache.doris.common.VariableAnnotation;
import org.apache.doris.common.util.ProfileManager;
+import org.apache.doris.nereids.parser.NereidsParser;
import org.apache.doris.thrift.TQueryOptions;
import org.apache.doris.utframe.TestWithFeService;
@@ -168,4 +169,11 @@ public class SessionVariablesTest extends
TestWithFeService {
Assertions.assertEquals("test",
sessionVariableClone.getSessionOriginValue().get(txIsolationSessionVariableField));
}
+
+ @Test
+ public void testSetVarInHint() {
+ String sql = "insert into test_t1 select /*+
set_var(enable_nereids_dml_with_pipeline=false)*/ * from test_t1 where
enable_nereids_dml_with_pipeline=true";
+ new NereidsParser().parseSQL(sql);
+ Assertions.assertEquals(false,
connectContext.getSessionVariable().enableNereidsDmlWithPipeline);
+ }
}
diff --git
a/regression-test/suites/nereids_p0/expression/fold_constant/fold_constant_by_be.groovy
b/regression-test/suites/nereids_p0/expression/fold_constant/fold_constant_by_be.groovy
index 9f306860dfb..882b84b7769 100644
---
a/regression-test/suites/nereids_p0/expression/fold_constant/fold_constant_by_be.groovy
+++
b/regression-test/suites/nereids_p0/expression/fold_constant/fold_constant_by_be.groovy
@@ -48,5 +48,11 @@ suite("fold_constant_by_be") {
qt_sql "explain select sleep(sign(1)*100);"
sql 'set query_timeout=12;'
- qt_sql "select sleep(sign(1)*5);"
+ qt_sql "select sleep(sign(1)*10);"
+
+ explain {
+ sql("verbose select substring('123456', 1, 3)")
+ contains "varchar(3)"
+ }
+
}
diff --git a/regression-test/suites/nereids_syntax_p0/system_var.groovy
b/regression-test/suites/nereids_syntax_p0/system_var.groovy
index 242df6ec94e..8f1270702b8 100644
--- a/regression-test/suites/nereids_syntax_p0/system_var.groovy
+++ b/regression-test/suites/nereids_syntax_p0/system_var.groovy
@@ -38,7 +38,7 @@ suite("nereids_sys_var") {
// set an invalid parameter, and throw an exception
test {
sql "select /*+SET_VAR(runtime_filter_type=10000)*/ * from supplier
limit 10"
- exception "Can not set session variable"
+ exception "errCode"
}
sql "select @@session.time_zone"
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
