kylin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From liy...@apache.org
Subject [15/48] kylin git commit: KYLIN-2803 refine logic and error message
Date Fri, 15 Sep 2017 14:42:35 GMT
KYLIN-2803 refine logic and error message


Project: http://git-wip-us.apache.org/repos/asf/kylin/repo
Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/b1559c66
Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/b1559c66
Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/b1559c66

Branch: refs/heads/ranger
Commit: b1559c669c3594b33f9e4aeff415d780a0a170ca
Parents: 565fe9a
Author: Hongbin Ma <mahongbin@apache.org>
Authored: Wed Sep 6 13:09:10 2017 +0800
Committer: liyang-gmt8 <liyang@apache.org>
Committed: Wed Sep 6 13:26:23 2017 +0800

----------------------------------------------------------------------
 .../org/apache/kylin/query/KylinTestBase.java   |  2 +-
 .../apache/kylin/query/util/PushDownUtil.java   | 27 ++++++++++---
 .../org/apache/kylin/query/util/QueryUtil.java  | 17 ++++----
 .../apache/kylin/query/util/QueryUtilTest.java  | 33 +++++++++++++++-
 .../apache/kylin/rest/service/QueryService.java | 41 +++++++++++++-------
 5 files changed, 90 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kylin/blob/b1559c66/kylin-it/src/test/java/org/apache/kylin/query/KylinTestBase.java
----------------------------------------------------------------------
diff --git a/kylin-it/src/test/java/org/apache/kylin/query/KylinTestBase.java b/kylin-it/src/test/java/org/apache/kylin/query/KylinTestBase.java
index d3db995..0f11996 100644
--- a/kylin-it/src/test/java/org/apache/kylin/query/KylinTestBase.java
+++ b/kylin-it/src/test/java/org/apache/kylin/query/KylinTestBase.java
@@ -261,7 +261,7 @@ public class KylinTestBase {
             return output(resultSet, needDisplay);
         } catch (SQLException sqlException) {
             Pair<List<List<String>>, List<SelectedColumnMeta>> result
= PushDownUtil
-                    .tryPushDownQuery(ProjectInstance.DEFAULT_PROJECT_NAME, sql, "DEFAULT",
sqlException);
+                    .tryPushDownSelectQuery(ProjectInstance.DEFAULT_PROJECT_NAME, sql, "DEFAULT",
sqlException);
             if (result == null) {
                 throw sqlException;
             }

http://git-wip-us.apache.org/repos/asf/kylin/blob/b1559c66/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java
----------------------------------------------------------------------
diff --git a/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java b/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java
index 9262b20..50b2136 100644
--- a/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java
+++ b/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java
@@ -59,18 +59,35 @@ import com.google.common.collect.Lists;
 public class PushDownUtil {
     private static final Logger logger = LoggerFactory.getLogger(PushDownUtil.class);
 
-    public static Pair<List<List<String>>, List<SelectedColumnMeta>>
tryPushDownQuery(String project, String sql,
+    public static Pair<List<List<String>>, List<SelectedColumnMeta>>
tryPushDownSelectQuery(String project, String sql,
             String defaultSchema, SQLException sqlException) throws Exception {
+        return tryPushDownQuery(project, sql, defaultSchema, sqlException, true);
+    }
+
+    public static Pair<List<List<String>>, List<SelectedColumnMeta>>
tryPushDownNonSelectQuery(String project,
+            String sql, String defaultSchema) throws Exception {
+        return tryPushDownQuery(project, sql, defaultSchema, null, false);
+    }
+
+    private static Pair<List<List<String>>, List<SelectedColumnMeta>>
tryPushDownQuery(String project, String sql,
+            String defaultSchema, SQLException sqlException, boolean isSelect) throws Exception
{
 
         KylinConfig kylinConfig = KylinConfig.getInstanceFromEnv();
 
         if (!kylinConfig.isPushDownEnabled())
             return null;
 
-        if (!isExpectedCause(sqlException))
-            return null;
+        if (isSelect) {
+            logger.info("Query failed to utilize pre-calculation, routing to other engines",
sqlException);
+            if (!isExpectedCause(sqlException)) {
+                logger.info("quit doPushDownQuery because prior exception thrown is unexpected");
+                return null;
+            }
+        } else {
+            Preconditions.checkState(sqlException == null);
+            logger.info("Kylin cannot support non-select queries, routing to other engines");
+        }
 
-        logger.info("Query failed to utilize pre-calculation, routing to other engines",
sqlException);
         IPushDownRunner runner = (IPushDownRunner) ClassUtil.newInstance(kylinConfig.getPushDownRunnerClassName());
         runner.init(kylinConfig);
         logger.debug("Query Pushdown runner {}", runner);
@@ -103,7 +120,7 @@ public class PushDownUtil {
         List<List<String>> returnRows = Lists.newArrayList();
         List<SelectedColumnMeta> returnColumnMeta = Lists.newArrayList();
 
-        if (QueryUtil.isSelectStatement(sql)) {
+        if (isSelect) {
             runner.executeQuery(sql, returnRows, returnColumnMeta);
         } else {
             runner.executeUpdate(sql);

http://git-wip-us.apache.org/repos/asf/kylin/blob/b1559c66/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
----------------------------------------------------------------------
diff --git a/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java b/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
index 2dff07c..7a1222e 100644
--- a/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
+++ b/query/src/main/java/org/apache/kylin/query/util/QueryUtil.java
@@ -58,8 +58,8 @@ public class QueryUtil {
         }
 
         // https://issues.apache.org/jira/browse/KYLIN-2649
-        if (kylinConfig.getForceLimit() > 0 &&
-                !sql.toLowerCase().contains("limit") && sql.toLowerCase().contains("*"))
{
+        if (kylinConfig.getForceLimit() > 0 && !sql.toLowerCase().contains("limit")
+                && sql.toLowerCase().contains("*")) {
             sql += ("\nLIMIT " + kylinConfig.getForceLimit());
         }
 
@@ -198,15 +198,16 @@ public class QueryUtil {
     }
 
     public static boolean isSelectStatement(String sql) {
-        String sql1 = removeCommentInSql(sql);
-        return sql1.startsWith("select") || sql1.startsWith("with") && sql1.contains("select");
+        String sql1 = sql.toLowerCase();
+        sql1 = removeCommentInSql(sql1);
+        sql1 = sql1.trim();
+        return sql1.startsWith("select") || (sql1.startsWith("with") && sql1.contains("select"));
     }
 
-    public static String removeCommentInSql(String sql) {
-        String sql1 = sql.toLowerCase();
+    public static String removeCommentInSql(String sql1) {
         // match two patterns, one is "-- comment", the other is "/* comment */"
-        final String[] commentPatterns = new String[] {"--[^\r\n]*", "/\\*[^\\*/]*"};
-        final int[] endOffset = new int[] {0, 2};
+        final String[] commentPatterns = new String[] { "--[^\r\n]*", "/\\*[^\\*/]*" };
+        final int[] endOffset = new int[] { 0, 2 };
 
         for (int i = 0; i < commentPatterns.length; i++) {
             String commentPattern = commentPatterns[i];

http://git-wip-us.apache.org/repos/asf/kylin/blob/b1559c66/query/src/test/java/org/apache/kylin/query/util/QueryUtilTest.java
----------------------------------------------------------------------
diff --git a/query/src/test/java/org/apache/kylin/query/util/QueryUtilTest.java b/query/src/test/java/org/apache/kylin/query/util/QueryUtilTest.java
index 46f5df4..de044ac 100644
--- a/query/src/test/java/org/apache/kylin/query/util/QueryUtilTest.java
+++ b/query/src/test/java/org/apache/kylin/query/util/QueryUtilTest.java
@@ -53,6 +53,37 @@ public class QueryUtilTest extends LocalFileMetadataTestCase {
     }
 
     @Test
+    public void testIsSelect() {
+        {
+            String sql = "select ( date '2001-09-28' + interval floor(1.2) day) from test_kylin_fact";
+            boolean selectStatement = QueryUtil.isSelectStatement(sql);
+            Assert.assertEquals(true, selectStatement);
+        }
+        {
+            String sql = " Select ( date '2001-09-28' + interval floor(1.2) day) from test_kylin_fact";
+            boolean selectStatement = QueryUtil.isSelectStatement(sql);
+            Assert.assertEquals(true, selectStatement);
+        }
+        {
+            String sql = " \n" + "Select ( date '2001-09-28' + interval floor(1.2) day) from
test_kylin_fact";
+            boolean selectStatement = QueryUtil.isSelectStatement(sql);
+            Assert.assertEquals(true, selectStatement);
+        }
+        {
+            String sql = "--comment\n"
+                    + " /* comment */Select ( date '2001-09-28' + interval floor(1.2) day)
from test_kylin_fact";
+            boolean selectStatement = QueryUtil.isSelectStatement(sql);
+            Assert.assertEquals(true, selectStatement);
+        }
+        {
+            String sql = " UPDATE Customers\n" + "SET ContactName = 'Alfred Schmidt', City=
'Frankfurt'\n"
+                    + "WHERE CustomerID = 1;";
+            boolean selectStatement = QueryUtil.isSelectStatement(sql);
+            Assert.assertEquals(false, selectStatement);
+        }
+    }
+
+    @Test
     public void testKeywordDefaultDirtyHack() {
         {
             String sql = "select * from DEFAULT.TEST_KYLIN_FACT";
@@ -64,7 +95,7 @@ public class QueryUtilTest extends LocalFileMetadataTestCase {
     @Test
     public void testRemoveCommentInSql() {
 
-        String originSql =  "select count(*) from test_kylin_fact where price > 10.0";
+        String originSql = "select count(*) from test_kylin_fact where price > 10.0";
 
         {
             String sqlWithComment = "-- comment \n" + originSql;

http://git-wip-us.apache.org/repos/asf/kylin/blob/b1559c66/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
----------------------------------------------------------------------
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
index f57aeb1..e1d0712 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
@@ -183,20 +183,24 @@ public class QueryService extends BasicService {
         }
     }
 
-
     public SQLResponse update(SQLRequest sqlRequest) throws Exception {
         // non select operations, only supported when enable pushdown
-        logger.debug("Query pushdown enabled, redirect the query to alternative engine. ");
+        logger.debug("Query pushdown enabled, redirect the non-select query to pushdown engine.");
         Connection conn = null;
         try {
             conn = QueryConnection.getConnection(sqlRequest.getProject());
             Pair<List<List<String>>, List<SelectedColumnMeta>> r
= PushDownUtil
-                    .tryPushDownQuery(sqlRequest.getProject(), sqlRequest.getSql(), conn.getSchema(),
null);
-            return buildSqlResponse(true, r.getFirst(), r.getSecond());
-            
+                    .tryPushDownNonSelectQuery(sqlRequest.getProject(), sqlRequest.getSql(),
conn.getSchema());
+
+            List<SelectedColumnMeta> columnMetas = Lists.newArrayList();
+            columnMetas.add(new SelectedColumnMeta(false, false, false, false, 1, false,
Integer.MAX_VALUE, "c0", "c0",
+                    null, null, null, Integer.MAX_VALUE, 128, 1, "char", false, false, false));
+
+            return buildSqlResponse(true, r.getFirst(), columnMetas);
+
         } catch (Exception e) {
-            logger.error("failed to do pushdown, error is " + e.getMessage(), e);
-            throw new InternalErrorException(e);
+            logger.info("pushdown engine failed to finish current non-select query");
+            throw e;
         } finally {
             close(null, null, conn);
         }
@@ -402,9 +406,9 @@ public class QueryService extends BasicService {
 
             try {
                 if (null == sqlResponse) {
-                    if (isSelect == true) {
+                    if (isSelect) {
                         sqlResponse = query(sqlRequest);
-                    } else if (kylinConfig.isPushDownEnabled() == true) {
+                    } else if (kylinConfig.isPushDownEnabled()) {
                         sqlResponse = update(sqlRequest);
                     } else {
                         logger.debug(
@@ -447,7 +451,7 @@ public class QueryService extends BasicService {
                 checkQueryAuth(sqlResponse, project);
 
             } catch (Throwable e) { // calcite may throw AssertError
-                logger.error("Exception when execute sql", e);
+                logger.error("Exception while executing query", e);
                 String errMsg = QueryUtil.makeErrorMsgUserFriendly(e);
 
                 sqlResponse = new SQLResponse(null, null, 0, true, errMsg);
@@ -829,17 +833,24 @@ public class QueryService extends BasicService {
 
                 results.add(oneRow);
             }
-            
+
         } catch (SQLException sqlException) {
-            Pair<List<List<String>>, List<SelectedColumnMeta>> r
= PushDownUtil
-                    .tryPushDownQuery(sqlRequest.getProject(), correctedSql, conn.getSchema(),
sqlException);
+            Pair<List<List<String>>, List<SelectedColumnMeta>> r
= null;
+            try {
+                r = PushDownUtil.tryPushDownSelectQuery(sqlRequest.getProject(), correctedSql,
conn.getSchema(),
+                        sqlException);
+            } catch (Exception e2) {
+                //exception in pushdown engine will be printed, but we'll not re-throw it,
we'll 
+                logger.info("pushdown engine failed current query too", e2);
+            }
+
             if (r == null)
                 throw sqlException;
-            
+
             isPushDown = true;
             results = r.getFirst();
             columnMetas = r.getSecond();
-            
+
         } finally {
             close(resultSet, stat, null); //conn is passed in, not my duty to close
         }


Mime
View raw message