[drill] branch master updated: DRILL-7160: e.q.max_rows QUERY-level option shown even if not set

[so...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sorabh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git


The following commit(s) were added to refs/heads/master by this push:
     new bf07127  DRILL-7160: e.q.max_rows QUERY-level option shown even if not set
bf07127 is described below

commit bf0712755884db1627e8dba3df4fec6b53b70d80
Author: Kunal Khatua <kk...@maprtech.com>
AuthorDate: Wed Apr 10 12:33:02 2019 -0700

    DRILL-7160: e.q.max_rows QUERY-level option shown even if not set
    
    The fix is to force setting to zero IFF autoLimit was intended to be set originally but is inapplicable; such as 'SHOW DATABASES'. If autolimit was not intended to be applied, setting the value to zero is not required.
    WebUI is also patched to not show the zero value set in such scenarios.
---
 .../drill/exec/planner/sql/DrillSqlWorker.java     | 24 +++++++++++++---------
 .../drill/exec/server/rest/QueryWrapper.java       | 10 ++++++++-
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
index dc331d9..09fbbdc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
@@ -216,27 +216,31 @@ public class DrillSqlWorker {
     return handler.getPlan(sqlNode);
   }
 
-  private static boolean isAutoLimitShouldBeApplied(QueryContext context, SqlNode sqlNode) {
-    return (context.getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue() > 0) && sqlNode.getKind().belongsTo(SqlKind.QUERY)
-        && (sqlNode.getKind() != SqlKind.ORDER_BY || isAutoLimitLessThanOrderByFetch((SqlOrderBy) sqlNode, context));
+  private static boolean isAutoLimitShouldBeApplied(SqlNode sqlNode, int queryMaxRows) {
+    return (queryMaxRows > 0) && sqlNode.getKind().belongsTo(SqlKind.QUERY)
+        && (sqlNode.getKind() != SqlKind.ORDER_BY || isAutoLimitLessThanOrderByFetch((SqlOrderBy) sqlNode, queryMaxRows));
   }
 
   private static SqlNode checkAndApplyAutoLimit(SqlConverter parser, QueryContext context, String sql) {
     SqlNode sqlNode = parser.parse(sql);
-    if (isAutoLimitShouldBeApplied(context, sqlNode)) {
-      sqlNode = wrapWithAutoLimit(sqlNode, context);
+    int queryMaxRows = context.getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue();
+    if (isAutoLimitShouldBeApplied(sqlNode, queryMaxRows)) {
+      sqlNode = wrapWithAutoLimit(sqlNode, queryMaxRows);
     } else {
-      context.getOptions().setLocalOption(ExecConstants.QUERY_MAX_ROWS, 0);
+      //Force setting to zero IFF autoLimit was intended to be set originally but is inapplicable
+      if (queryMaxRows > 0) {
+        context.getOptions().setLocalOption(ExecConstants.QUERY_MAX_ROWS, 0);
+      }
     }
     return sqlNode;
   }
 
-  private static boolean isAutoLimitLessThanOrderByFetch(SqlOrderBy orderBy, QueryContext context) {
-    return orderBy.fetch == null || Integer.parseInt(orderBy.fetch.toString()) > context.getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue();
+  private static boolean isAutoLimitLessThanOrderByFetch(SqlOrderBy orderBy, int queryMaxRows) {
+    return orderBy.fetch == null || Integer.parseInt(orderBy.fetch.toString()) > queryMaxRows;
   }
 
-  private static SqlNode wrapWithAutoLimit(SqlNode sqlNode, QueryContext context) {
-    SqlNumericLiteral autoLimitLiteral = SqlLiteral.createExactNumeric(String.valueOf(context.getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue()), SqlParserPos.ZERO);
+  private static SqlNode wrapWithAutoLimit(SqlNode sqlNode, int queryMaxRows) {
+    SqlNumericLiteral autoLimitLiteral = SqlLiteral.createExactNumeric(String.valueOf(queryMaxRows), SqlParserPos.ZERO);
     if (sqlNode.getKind() == SqlKind.ORDER_BY) {
       SqlOrderBy orderBy = (SqlOrderBy) sqlNode;
       return new SqlOrderBy(orderBy.getParserPosition(), orderBy.query, orderBy.orderList, orderBy.offset, autoLimitLiteral);
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
index 89315ce..43dd737 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
 
 import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.exceptions.UserRemoteException;
+import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.proto.UserBitShared.QueryId;
 import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
 import org.apache.drill.exec.proto.UserBitShared.QueryType;
@@ -77,7 +78,14 @@ public class QueryWrapper {
         .setAutolimitRowcount(autoLimitRowCount)
         .build();
 
-    webUserConnection.setAutoLimitRowCount(autoLimitRowCount);
+    int defaultMaxRows = webUserConnection.getSession().getOptions().getOption(ExecConstants.QUERY_MAX_ROWS).num_val.intValue();
+    int maxRows;
+    if (autoLimitRowCount > 0 && defaultMaxRows > 0) {
+      maxRows = Math.min(autoLimitRowCount, defaultMaxRows);
+    } else {
+      maxRows = Math.max(autoLimitRowCount, defaultMaxRows);
+    }
+    webUserConnection.setAutoLimitRowCount(maxRows);
 
     // Submit user query to Drillbit work queue.
     final QueryId queryId = workManager.getUserWorker().submitWork(webUserConnection, runQuery);