You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kylin.apache.org by GitBox <gi...@apache.org> on 2018/02/27 06:47:43 UTC

[GitHub] rogercloud commented on a change in pull request #107: kylin-3119

rogercloud commented on a change in pull request #107: kylin-3119
URL: https://github.com/apache/kylin/pull/107#discussion_r170828728
 
 

 ##########
 File path: 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");
 
 Review comment:
   I guess you want to test newline character on different platform. It's a good point. I suggest to hard code "\r\n" for Windows and "\n" for Linux instead of reading "line.separator". Maybe I misunderstand, please figure it out.
   
   As there are more code blocks here, some comments to explain the test purpose is a good practice. :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services