From 98a9a452e48fd9b7a04167315db6805988eb7f4d Mon Sep 17 00:00:00 2001 From: LiBinfeng <46676950+LiBinfeng-01@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:59:37 +0800 Subject: [PATCH 1/3] [feat](Nereids) support set var in hint when parse sql (#41331) set var hint need to be enable to use before analyze, so it need to be set when parsing sql now it would set twice when parse and begin of analyze --- .../nereids/parser/LogicalPlanBuilder.java | 4 ++- .../nereids/properties/SelectHintSetVar.java | 30 ++++++++++++++++ .../analysis/EliminateLogicalSelectHint.java | 36 +------------------ .../apache/doris/qe/SessionVariablesTest.java | 9 +++++ 4 files changed, 43 insertions(+), 36 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 07d6f77f6d0f8c..4a80e98a506f42 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 @@ -1834,7 +1834,9 @@ private LogicalPlan withSelectHint(LogicalPlan logicalPlan, List leadingParameters = new ArrayList(); 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 8c08dcf378fabd..68ac1d699f2472 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,29 @@ public Map> getParameters() { 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> kv : getParameters().entrySet()) { + String key = kv.getKey(); + Optional 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); + } + } + } + } + @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 06d85e660b090a..3de7e20c5a765e 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; @@ -35,8 +32,6 @@ 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; @@ -58,7 +53,7 @@ public Rule build() { for (Entry hint : selectHintPlan.getHints().entrySet()) { String hintName = hint.getKey(); if (hintName.equalsIgnoreCase("SET_VAR")) { - setVar((SelectHintSetVar) hint.getValue(), ctx.statementContext); + ((SelectHintSetVar) hint.getValue()).setVarOnceInSql(ctx.statementContext); } else if (hintName.equalsIgnoreCase("ORDERED")) { try { ctx.cascadesContext.getConnectContext().getSessionVariable() @@ -81,35 +76,6 @@ public Rule build() { }).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> kv : selectHint.getParameters().entrySet()) { - String key = kv.getKey(); - Optional value = kv.getValue(); - if (value.isPresent()) { - try { - VariableMgr.setVar(sessionVariable, new SetVar(key, new StringLiteral(value.get()))); - } 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, Map hints) { LeadingHint hint = new LeadingHint("Leading", selectHint.getParameters(), selectHint.toString()); 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 2ba2291bc4d8ff..7082995d88dad3 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,9 +27,11 @@ import org.apache.doris.common.FeConstants; 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.common.util.RuntimeProfile; import org.apache.doris.load.ExportJob; +import org.apache.doris.nereids.parser.NereidsParser; import org.apache.doris.task.ExportExportingTask; import org.apache.doris.thrift.TQueryOptions; import org.apache.doris.utframe.TestWithFeService; @@ -229,4 +231,11 @@ public void testDisableProfile() { } } + + @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); + } } From d28d6c58067ff31c34dfa40ec591c48bc3abfdf1 Mon Sep 17 00:00:00 2001 From: LiBinfeng <1204975323@qq.com> Date: Wed, 23 Oct 2024 18:03:43 +0800 Subject: [PATCH 2/3] fix checkstyle --- .../doris/nereids/properties/SelectHintSetVar.java | 12 +++++++++++- .../rules/analysis/EliminateLogicalSelectHint.java | 1 - .../org/apache/doris/qe/SessionVariablesTest.java | 1 - 3 files changed, 11 insertions(+), 3 deletions(-) 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 68ac1d699f2472..b32d18fc89b297 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 @@ -59,13 +59,23 @@ public void setVarOnceInSql(StatementContext context) { 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 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 3de7e20c5a765e..295a5ebb0a066b 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 @@ -38,7 +38,6 @@ import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; /** * eliminate logical select hint and set them to cascade context 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 7082995d88dad3..238c2b52ede901 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,7 +27,6 @@ import org.apache.doris.common.FeConstants; 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.common.util.RuntimeProfile; import org.apache.doris.load.ExportJob; From 6941e36cb6a19e1823876887ff43d0419ef60f46 Mon Sep 17 00:00:00 2001 From: LiBinfeng <1204975323@qq.com> Date: Thu, 24 Oct 2024 14:35:37 +0800 Subject: [PATCH 3/3] fix p0 --- .../org/apache/doris/nereids/properties/SelectHintSetVar.java | 3 +++ regression-test/suites/nereids_syntax_p0/system_var.groovy | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) 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 b32d18fc89b297..0d54cb75b1e6cc 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 @@ -50,6 +50,9 @@ public Map> getParameters() { * @param context statement context */ public void setVarOnceInSql(StatementContext context) { + if (context == null) { + return; + } SessionVariable sessionVariable = context.getConnectContext().getSessionVariable(); // set temporary session value, and then revert value in the 'finally block' of StmtExecutor#execute sessionVariable.setIsSingleSetVar(true); diff --git a/regression-test/suites/nereids_syntax_p0/system_var.groovy b/regression-test/suites/nereids_syntax_p0/system_var.groovy index 3a3714e8c8161c..42f19ef530aca3 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 "Unexpected exception: Can not set session variable" + exception "errCode" } sql "select @@session.time_zone"