KYLIN-3119 Fix bugs in the function 'massageSql' of 'QueryUtil.java'

apache · Mar 13, 2018 · a74cb8a · a74cb8a
1 parent ac05738
commit a74cb8a
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 11 deletions.
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
@@ -50,32 +50,39 @@ public static String massageSql(String sql, String project, int limit, int offse
         ProjectManager projectManager = ProjectManager.getInstance(KylinConfig.getInstanceFromEnv());
         ProjectInstance projectInstance = projectManager.getProject(project);
         KylinConfig kylinConfig = projectInstance.getConfig();
-
+        sql = removeCommentInSql(sql);
         while (sql.endsWith(";"))
             sql = sql.substring(0, sql.length() - 1);
 
-        if (limit > 0 && !sql.toLowerCase().contains("limit")) {
-            sql += ("\nLIMIT " + limit);
-        }
+        String sql1=sql;
+        final String suffixPattern = "^.+?\\s(limit\\s\\d+)?\\s(offset\\s\\d+)?\\s*$";
+        sql = sql.replaceAll("\\s+", " ");
+        Pattern pattern = Pattern.compile(suffixPattern);
+        Matcher matcher = pattern.matcher(sql.toLowerCase() + "  ");
 
-        if (offset > 0 && !sql.toLowerCase().contains("offset")) {
-            sql += ("\nOFFSET " + offset);
+        if (matcher.find()) {
+            if (limit > 0 && matcher.group(1) == null) {
+                sql1 += ("\nLIMIT " + limit);
+            }
+            if (offset > 0 && matcher.group(2) == null) {
+                sql1 += ("\nOFFSET " + offset);
+            }
         }
 
         // https://issues.apache.org/jira/browse/KYLIN-2649
-        if (kylinConfig.getForceLimit() > 0 && !sql.toLowerCase().contains("limit")
-                && sql.toLowerCase().matches("^select\\s+\\*\\p{all}*")) {
-            sql += ("\nLIMIT " + kylinConfig.getForceLimit());
+        if (kylinConfig.getForceLimit() > 0 && limit <= 0 && matcher.group(1) == null
+                && sql1.toLowerCase().matches("^select\\s+\\*\\p{all}*")) {
+            sql1 += ("\nLIMIT " + kylinConfig.getForceLimit());
         }
 
         // customizable SQL transformation
         if (queryTransformers == null) {
             initQueryTransformers();
         }
         for (IQueryTransformer t : queryTransformers) {
-            sql = t.transform(sql, project, defaultSchema);
+            sql1 = t.transform(sql1, project, defaultSchema);
         }
-        return sql;
+        return sql1;
     }
 
     private static void initQueryTransformers() {

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
@@ -51,6 +51,58 @@ public void testMassageSql() {
                     "select ( date '2001-09-28' + interval '2' month) from test_kylin_fact group by ( date '2001-09-28' + interval '2' month)",
                     s);
         }
+        {
+            String sql = "select count(*) test_limit from test_kylin_fact where price > 10.0";
+            String s = QueryUtil.massageSql(sql, "default", 50000, 0, "DEFAULT");
+            Assert.assertEquals(
+                    "select count(*) test_limit from test_kylin_fact where price > 10.0\n" +
+                            "LIMIT 50000",
+                    s);
+        }
+        {
+            String sql = "select count(*) test_offset from test_kylin_fact where price > 10.0";
+            String s = QueryUtil.massageSql(sql, "default", 0, 50, "DEFAULT");
+            Assert.assertEquals(
+                    "select count(*) test_offset from test_kylin_fact where price > 10.0\n" +
+                            "OFFSET 50",
+                    s);
+        }
+        {
+            String sql = "select count(*) test_limit_and_offset from test_kylin_fact where price > 10.0";
+            String s = QueryUtil.massageSql(sql, "default", 50000, 50, "DEFAULT");
+            Assert.assertEquals(
+                    "select count(*) test_limit_and_offset from test_kylin_fact where price > 10.0\n" +
+                            "LIMIT 50000\nOFFSET 50",
+                    s);
+        }
+
+        {
+            String newLine = System.getProperty("line.separator");
+            String sql = "select count(*)     test_limit from " + newLine + "test_kylin_fact where price > 10.0";
+            newLine = newLine.replace("\r", " ").replace("\n", newLine);
+            String s = QueryUtil.massageSql(sql, "default", 50000, 0, "DEFAULT");
+            Assert.assertEquals(
+                    "select count(*)     test_limit from " + newLine + "test_kylin_fact where price > 10.0\nLIMIT 50000",
+                    s);
+        }
+        {
+            String newLine = System.getProperty("line.separator");
+            String sql = "select count(*)     test_offset from " + newLine + "test_kylin_fact where price > 10.0";
+            newLine = newLine.replace("\r", " ").replace("\n", newLine);
+            String s = QueryUtil.massageSql(sql, "default", 50000, 0, "DEFAULT");
+            Assert.assertEquals(
+                    "select count(*)     test_offset from " + newLine + "test_kylin_fact where price > 10.0\nLIMIT 50000",
+                    s);
+        }
+        {
+            String newLine = System.getProperty("line.separator");
+            String sql = "select count(*)     test_limit_and_offset from " + newLine + "test_kylin_fact where price > 10.0";
+            newLine = newLine.replace("\r", " ").replace("\n", newLine);
+            String s = QueryUtil.massageSql(sql, "default", 50000, 0, "DEFAULT");
+            Assert.assertEquals(
+                    "select count(*)     test_limit_and_offset from " + newLine + "test_kylin_fact where price > 10.0\nLIMIT 50000",
+                    s);
+        }
     }
 
     @Test