[BugFix] Validate query_timeout at analyze stage (#13653)

StarRocks · Nov 21, 2022 · da05d82 · da05d82
1 parent bfe760b
commit da05d82
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 19 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/MasterOpExecutor.java b/fe/fe-core/src/main/java/com/starrocks/qe/MasterOpExecutor.java
@@ -68,6 +68,9 @@ public MasterOpExecutor(StatementBase parsedStmt, OriginStatement originStmt,
         // set thriftTimeoutMs to query_timeout + thrift_rpc_timeout_ms
         // so that we can return an execution timeout instead of a network timeout
         this.thriftTimeoutMs = ctx.getSessionVariable().getQueryTimeoutS() * 1000 + Config.thrift_rpc_timeout_ms;
+        if (this.thriftTimeoutMs < 0) {
+            this.thriftTimeoutMs = ctx.getSessionVariable().getQueryTimeoutS() * 1000;
+        }
         this.parsedStmt = parsedStmt;
     }
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java b/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java
@@ -106,8 +106,6 @@ public class SessionVariable implements Serializable, Writable, Cloneable {
     public static final String SQL_SAFE_UPDATES = "sql_safe_updates";
     public static final String NET_BUFFER_LENGTH = "net_buffer_length";
     public static final String CODEGEN_LEVEL = "codegen_level";
-    // mem limit can't smaller than bufferpool's default page size
-    public static final int MIN_EXEC_MEM_LIMIT = 2097152;
     public static final String BATCH_SIZE = "batch_size";
     public static final String CHUNK_SIZE = "chunk_size";
     public static final String DISABLE_STREAMING_PREAGGREGATIONS = "disable_streaming_preaggregations";
@@ -232,6 +230,12 @@ public class SessionVariable implements Serializable, Writable, Cloneable {
 
     public static final String STATISTIC_COLLECT_PARALLEL = "statistic_collect_parallel";
 
+    // Limitations
+    // mem limit can't smaller than bufferpool's default page size
+    public static final int MIN_EXEC_MEM_LIMIT = 2097152;
+    // query timeout cannot greater than one month
+    public static final int MAX_QUERY_TIMEOUT = 259200;
+
     @VariableMgr.VarAttr(name = ENABLE_PIPELINE, alias = ENABLE_PIPELINE_ENGINE, show = ENABLE_PIPELINE_ENGINE)
     private boolean enablePipelineEngine = true;
 
@@ -657,16 +661,10 @@ public String getCollationServer() {
     }
 
     public long getSqlSelectLimit() {
-        if (sqlSelectLimit < 0) {
-            return DEFAULT_SELECT_LIMIT;
-        }
         return sqlSelectLimit;
     }
 
     public void setSqlSelectLimit(long limit) {
-        if (limit < 0) {
-            return;
-        }
         this.sqlSelectLimit = limit;
     }
 
@@ -679,11 +677,7 @@ public void setTimeZone(String timeZone) {
     }
 
     public void setMaxExecMemByte(long maxExecMemByte) {
-        if (maxExecMemByte < MIN_EXEC_MEM_LIMIT) {
-            this.maxExecMemByte = MIN_EXEC_MEM_LIMIT;
-        } else {
-            this.maxExecMemByte = maxExecMemByte;
-        }
+        this.maxExecMemByte = maxExecMemByte;
     }
 
     public void setLoadMemLimit(long loadMemLimit) {

diff --git a/fe/fe-core/src/test/java/com/starrocks/qe/VariableMgrTest.java b/fe/fe-core/src/test/java/com/starrocks/qe/VariableMgrTest.java
@@ -108,9 +108,9 @@ public void testNormal() throws IllegalAccessException, NoSuchFieldException, Us
         SetVar setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(1234L));
         setVar.analyze(null);
         VariableMgr.setVar(var, setVar, false);
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
         var = VariableMgr.newSessionVariable();
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
 
         SetVar setVar2 = new SetVar(SetType.GLOBAL, "parallel_fragment_exec_instance_num", new IntLiteral(5L));
         setVar2.analyze(null);
@@ -137,15 +137,15 @@ public void testNormal() throws IllegalAccessException, NoSuchFieldException, Us
         setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(1234L));
         setVar.analyze(null);
         VariableMgr.setVar(var, setVar, false);
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
 
         // onlySessionVar
         setVar = new SetVar(SetType.GLOBAL, "exec_mem_limit", new IntLiteral(4321L));
         setVar.analyze(null);
         VariableMgr.setVar(var, setVar, true);
-        Assert.assertEquals(4321L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999935L, var.getMaxExecMemByte());
         var = VariableMgr.newSessionVariable();
-        Assert.assertEquals(1234L, var.getMaxExecMemByte());
+        Assert.assertEquals(12999934L, var.getMaxExecMemByte());
 
         setVar3 = new SetVar(SetType.SESSION, "time_zone", new StringLiteral("Asia/Jakarta"));
         setVar3.analyze(null);

diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/LimitTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/LimitTest.java
@@ -416,7 +416,7 @@ public void testSqlSelectLimitSession() throws Exception {
         Assert.assertTrue(plan.contains("limit: 8888"));
         connectContext.getSessionVariable().setSqlSelectLimit(SessionVariable.DEFAULT_SELECT_LIMIT);
 
-        connectContext.getSessionVariable().setSqlSelectLimit(-100);
+        connectContext.getSessionVariable().setSqlSelectLimit(0);
         sql = "select * from test_all_type";
         plan = getFragmentPlan(sql);
         Assert.assertFalse(plan.contains("limit"));