From 8dc7cc42d0cfd47c10b2f1a406911b468608c699 Mon Sep 17 00:00:00 2001 From: morningman Date: Thu, 13 Aug 2020 10:45:39 +0800 Subject: [PATCH 1/6] first --- .../doris/analysis/ExpressionFunctions.java | 18 ++++++++++++++---- .../apache/doris/analysis/SysVariableDesc.java | 18 +++++++++++++++++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExpressionFunctions.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExpressionFunctions.java index ee9a6f5499c74c..88420cd7c58543 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExpressionFunctions.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExpressionFunctions.java @@ -22,6 +22,8 @@ import org.apache.doris.catalog.ScalarType; import org.apache.doris.catalog.Type; import org.apache.doris.common.AnalysisException; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.VariableMgr; import org.apache.doris.rewrite.FEFunction; import org.apache.doris.rewrite.FEFunctions; @@ -56,18 +58,18 @@ private ExpressionFunctions() { public Expr evalExpr(Expr constExpr) { // Function's arg are all LiteralExpr. for (Expr child : constExpr.getChildren()) { - if (!(child instanceof LiteralExpr)) { + if (!(child instanceof LiteralExpr) && !(child instanceof SysVariableDesc)) { return constExpr; } } if (constExpr instanceof ArithmeticExpr - || constExpr instanceof FunctionCallExpr + || constExpr instanceof FunctionCallExpr || constExpr instanceof TimestampArithmeticExpr) { Function fn = constExpr.getFn(); - + Preconditions.checkNotNull(fn, "Expr's fn can't be null."); - + // return NullLiteral directly iff: // 1. Not UDF // 2. Not in NonNullResultWithNullParamFunctions @@ -96,6 +98,14 @@ public Expr evalExpr(Expr constExpr) { return constExpr; } } + } else if (constExpr instanceof SysVariableDesc) { + try { + VariableMgr.fillValue(ConnectContext.get().getSessionVariable(), (SysVariableDesc) constExpr); + return ((SysVariableDesc) constExpr).getLiteralExpr(); + } catch (AnalysisException e) { + LOG.warn("failed to get session variable value: " + ((SysVariableDesc) constExpr).getName()); + return constExpr; + } } return constExpr; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java index 3d7da4074e53f0..07ad6bb6bb90d9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java @@ -17,7 +17,6 @@ package org.apache.doris.analysis; -import com.google.common.base.Strings; import org.apache.doris.catalog.Type; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.DdlException; @@ -32,6 +31,8 @@ import org.apache.doris.thrift.TIntLiteral; import org.apache.doris.thrift.TStringLiteral; +import com.google.common.base.Strings; + // System variable // Converted to StringLiteral in analyze, if this variable is not exist, throw AnalysisException. public class SysVariableDesc extends Expr { @@ -42,6 +43,8 @@ public class SysVariableDesc extends Expr { private double floatValue; private String strValue; + private LiteralExpr literalExpr; + public SysVariableDesc(String name) { this(name, SetType.SESSION); } @@ -89,18 +92,31 @@ public SetType getSetType() { public void setBoolValue(boolean value) { this.boolValue = value; + this.literalExpr = new BoolLiteral(value); } public void setIntValue(long value) { this.intValue = value; + this.literalExpr = new IntLiteral(value); } public void setFloatValue(double value) { this.floatValue = value; + this.literalExpr = new FloatLiteral(value); } public void setStringValue(String value) { this.strValue = value; + this.literalExpr = new StringLiteral(value); + } + + public Expr getLiteralExpr() { + return this.literalExpr; + } + + @Override + protected boolean isConstantImpl() { + return true; } @Override From a7c23245521d383854e50dbc3e66a7b780300d27 Mon Sep 17 00:00:00 2001 From: morningman Date: Thu, 13 Aug 2020 17:44:53 +0800 Subject: [PATCH 2/6] rewrite --- .../main/java/org/apache/doris/analysis/SetVar.java | 5 +++++ .../java/org/apache/doris/planner/QueryPlanTest.java | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java index 4d3b0d61cd3b29..bb58a32cafe82a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java @@ -28,6 +28,7 @@ import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.GlobalVariable; import org.apache.doris.qe.SessionVariable; +import org.apache.doris.rewrite.ExprRewriter; import org.apache.doris.system.HeartbeatFlags; // change one variable. @@ -106,6 +107,10 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { throw new AnalysisException("Set statement does't support non-constant expr."); } + ExprRewriter exprRewriter = analyzer.getExprRewriter(); + exprRewriter.reset(); + value = exprRewriter.rewrite(value, analyzer); + final Expr literalExpr = value.getResultValue(); if (!(literalExpr instanceof LiteralExpr)) { throw new AnalysisException("Set statement does't support computing expr:" + literalExpr.toSql()); diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java index beb7b506de239f..b97649279ab97c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -17,7 +17,6 @@ package org.apache.doris.planner; -import com.google.common.collect.Lists; import org.apache.doris.analysis.CreateDbStmt; import org.apache.doris.analysis.CreateTableStmt; import org.apache.doris.analysis.DropDbStmt; @@ -41,6 +40,8 @@ import org.apache.doris.qe.QueryState.MysqlStateType; import org.apache.doris.utframe.UtFrameUtils; +import com.google.common.collect.Lists; + import org.apache.commons.lang3.StringUtils; import org.junit.AfterClass; import org.junit.Assert; @@ -1005,4 +1006,12 @@ public void testEmptyNode() throws Exception { Assert.assertFalse(explainString.contains(denseRank)); } } + + @Test + public void testConst() throws Exception { + connectContext.setDatabase("default_cluster:test"); + String queryStr = "set sql_mode = concat(@@exec_mem_limit, \"abc\")"; + // default set PreferBroadcastJoin true + String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, queryStr); + } } From 376488163a226de682431711e9879f5937af6ad0 Mon Sep 17 00:00:00 2001 From: morningman Date: Thu, 13 Aug 2020 19:00:03 +0800 Subject: [PATCH 3/6] rewrite 2 --- .../main/java/org/apache/doris/analysis/SetVar.java | 10 ++++++++-- .../org/apache/doris/analysis/SysVariableDesc.java | 13 +++++++++++++ .../org/apache/doris/planner/QueryPlanTest.java | 4 +++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java index bb58a32cafe82a..9a7007e536edf3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java @@ -17,7 +17,6 @@ package org.apache.doris.analysis; -import com.google.common.base.Strings; import org.apache.doris.catalog.Catalog; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.ErrorCode; @@ -28,9 +27,10 @@ import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.GlobalVariable; import org.apache.doris.qe.SessionVariable; -import org.apache.doris.rewrite.ExprRewriter; import org.apache.doris.system.HeartbeatFlags; +import com.google.common.base.Strings; + // change one variable. public class SetVar { @@ -107,9 +107,15 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { throw new AnalysisException("Set statement does't support non-constant expr."); } + /* ExprRewriter exprRewriter = analyzer.getExprRewriter(); exprRewriter.reset(); value = exprRewriter.rewrite(value, analyzer); + if (exprRewriter.changed()) { + Analyzer newAnalyzer = new Analyzer(analyzer.getCatalog(), analyzer.getContext()); + value.analyze(newAnalyzer); + } + */ final Expr literalExpr = value.getResultValue(); if (!(literalExpr instanceof LiteralExpr)) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java index 07ad6bb6bb90d9..23d901334c5dfc 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java @@ -114,6 +114,19 @@ public Expr getLiteralExpr() { return this.literalExpr; } + @Override + public Expr getResultValue() throws AnalysisException { + Expr expr = super.getResultValue(); + if (!Strings.isNullOrEmpty(name) && name.equalsIgnoreCase(SessionVariable.SQL_MODE)) { + try { + return new StringLiteral(SqlModeHelper.decode(intValue)); + } catch (DdlException e) { + throw new AnalysisException(e.getMessage()); + } + } + return expr; + } + @Override protected boolean isConstantImpl() { return true; diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java index b97649279ab97c..1cdb4b271ebffb 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -1010,7 +1010,9 @@ public void testEmptyNode() throws Exception { @Test public void testConst() throws Exception { connectContext.setDatabase("default_cluster:test"); - String queryStr = "set sql_mode = concat(@@exec_mem_limit, \"abc\")"; + // String queryStr = "set sql_mode = concat(@@sql_mode, \"STRICT_TRANS_TABLES\")"; + // String queryStr = "select concat(@@sql_mode, \"STRICT_TRANS_TABLES\")"; + String queryStr = "select concat(@@exec_mem_limit, \"STRICT_TRANS_TABLES\")"; // default set PreferBroadcastJoin true String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, queryStr); } From bbaab0d88c66cc891840522aa4a2af080beaa4a4 Mon Sep 17 00:00:00 2001 From: morningman Date: Sun, 16 Aug 2020 17:48:22 +0800 Subject: [PATCH 4/6] add comment --- .../main/java/org/apache/doris/analysis/SetVar.java | 12 ------------ .../org/apache/doris/analysis/SysVariableDesc.java | 4 ++++ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java index 9a7007e536edf3..3d6ece7fe16b83 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java @@ -29,8 +29,6 @@ import org.apache.doris.qe.SessionVariable; import org.apache.doris.system.HeartbeatFlags; -import com.google.common.base.Strings; - // change one variable. public class SetVar { @@ -107,16 +105,6 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { throw new AnalysisException("Set statement does't support non-constant expr."); } - /* - ExprRewriter exprRewriter = analyzer.getExprRewriter(); - exprRewriter.reset(); - value = exprRewriter.rewrite(value, analyzer); - if (exprRewriter.changed()) { - Analyzer newAnalyzer = new Analyzer(analyzer.getCatalog(), analyzer.getContext()); - value.analyze(newAnalyzer); - } - */ - final Expr literalExpr = value.getResultValue(); if (!(literalExpr instanceof LiteralExpr)) { throw new AnalysisException("Set statement does't support computing expr:" + literalExpr.toSql()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java index 23d901334c5dfc..f8906c393e8275 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SysVariableDesc.java @@ -118,6 +118,10 @@ public Expr getLiteralExpr() { public Expr getResultValue() throws AnalysisException { Expr expr = super.getResultValue(); if (!Strings.isNullOrEmpty(name) && name.equalsIgnoreCase(SessionVariable.SQL_MODE)) { + // SQL_MODE is a special variable. Its type is int, but it is usually set using a string. + // Such as `set sql_mode = concat(@@sql_mode, "STRICT_TRANS_TABLES");` + // So we return the string type here so that it can correctly match the subsequent function signature. + // We will convert the string to int in VariableMgr. try { return new StringLiteral(SqlModeHelper.decode(intValue)); } catch (DdlException e) { From 413ff10214b968f9bb9c945f41089090ad84da03 Mon Sep 17 00:00:00 2001 From: morningman Date: Sun, 16 Aug 2020 17:57:09 +0800 Subject: [PATCH 5/6] add ut --- .../org/apache/doris/analysis/SetVar.java | 2 + .../doris/analysis/SetVariableTest.java | 52 +++++++++++++++++++ .../apache/doris/planner/QueryPlanTest.java | 14 +---- 3 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java index 3d6ece7fe16b83..a62f36bda505b9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetVar.java @@ -29,6 +29,8 @@ import org.apache.doris.qe.SessionVariable; import org.apache.doris.system.HeartbeatFlags; +import com.google.common.base.Strings; + // change one variable. public class SetVar { diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java new file mode 100644 index 00000000000000..ed5a2dd42dca75 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java @@ -0,0 +1,52 @@ +// 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.analysis; + +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.UserException; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.utframe.UtFrameUtils; + +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.UUID; + +public class SetVariableTest { + // use a unique dir so that it won't be conflict with other unit test which + // may also start a Mocked Frontend + private static String runningDir = "fe/mocked/QueryPlanTest/" + UUID.randomUUID().toString() + "/"; + + private static ConnectContext connectContext; + + @BeforeClass + public static void beforeClass() throws Exception { + UtFrameUtils.createMinDorisCluster(runningDir); + // create connect context + connectContext = UtFrameUtils.createDefaultCtx(); + } + + @Test + public void testNormal() throws UserException, AnalysisException { + String setStr = "set sql_mode = concat(@@sql_mode, 'STRICT_TRANS_TABLES');"; + connectContext.getState().reset(); + StmtExecutor stmtExecutor = new StmtExecutor(connectContext, setStr); + System.out.println(connectContext.getSessionVariable().getSqlMode()); + } +} \ No newline at end of file diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java index 1cdb4b271ebffb..492bcd57dd758d 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -40,14 +40,14 @@ import org.apache.doris.qe.QueryState.MysqlStateType; import org.apache.doris.utframe.UtFrameUtils; -import com.google.common.collect.Lists; - import org.apache.commons.lang3.StringUtils; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import com.google.common.collect.Lists; + import java.io.File; import java.util.List; import java.util.UUID; @@ -1006,14 +1006,4 @@ public void testEmptyNode() throws Exception { Assert.assertFalse(explainString.contains(denseRank)); } } - - @Test - public void testConst() throws Exception { - connectContext.setDatabase("default_cluster:test"); - // String queryStr = "set sql_mode = concat(@@sql_mode, \"STRICT_TRANS_TABLES\")"; - // String queryStr = "select concat(@@sql_mode, \"STRICT_TRANS_TABLES\")"; - String queryStr = "select concat(@@exec_mem_limit, \"STRICT_TRANS_TABLES\")"; - // default set PreferBroadcastJoin true - String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, queryStr); - } } From 0c2350f15b0dd5abcfecabff10a3c9e7db5cf587 Mon Sep 17 00:00:00 2001 From: morningman Date: Sun, 16 Aug 2020 18:26:22 +0800 Subject: [PATCH 6/6] fix bug --- .../doris/analysis/SetVariableTest.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java index ed5a2dd42dca75..7cf115531338bb 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/SetVariableTest.java @@ -17,12 +17,12 @@ package org.apache.doris.analysis; -import org.apache.doris.common.AnalysisException; -import org.apache.doris.common.UserException; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SqlModeHelper; import org.apache.doris.qe.StmtExecutor; import org.apache.doris.utframe.UtFrameUtils; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; @@ -43,10 +43,21 @@ public static void beforeClass() throws Exception { } @Test - public void testNormal() throws UserException, AnalysisException { + public void testSqlMode() throws Exception { String setStr = "set sql_mode = concat(@@sql_mode, 'STRICT_TRANS_TABLES');"; connectContext.getState().reset(); StmtExecutor stmtExecutor = new StmtExecutor(connectContext, setStr); - System.out.println(connectContext.getSessionVariable().getSqlMode()); + stmtExecutor.execute(); + Assert.assertEquals("STRICT_TRANS_TABLES", + SqlModeHelper.decode(connectContext.getSessionVariable().getSqlMode())); + } + + @Test + public void testExecMemLimit() throws Exception { + String setStr = "set exec_mem_limit = @@exec_mem_limit * 10"; + connectContext.getState().reset(); + StmtExecutor stmtExecutor = new StmtExecutor(connectContext, setStr); + stmtExecutor.execute(); + Assert.assertEquals(21474836480L, connectContext.getSessionVariable().getMaxExecMemByte()); } } \ No newline at end of file