shaofengshi closed pull request #268: KYLIN-3594 Obey JDBC standard
URL: https://github.com/apache/kylin/pull/268
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/kylin-it/src/test/resources/query/sql_snowflake/query07.sql 
b/kylin-it/src/test/resources/query/sql_snowflake/query07.sql
new file mode 100644
index 0000000000..2705f783d7
--- /dev/null
+++ b/kylin-it/src/test/resources/query/sql_snowflake/query07.sql
@@ -0,0 +1,23 @@
+SELECT
+
+count(*) as cnt
+
+FROM defaultCatalog.TEST_KYLIN_FACT as TEST_KYLIN_FACT
+INNER JOIN defaultCatalog.TEST_ORDER as TEST_ORDER
+ON TEST_KYLIN_FACT.ORDER_ID = TEST_ORDER.ORDER_ID
+INNER JOIN defaultCatalog.TEST_ACCOUNT as BUYER_ACCOUNT
+ON TEST_ORDER.BUYER_ID = BUYER_ACCOUNT.ACCOUNT_ID
+INNER JOIN defaultCatalog.TEST_ACCOUNT as SELLER_ACCOUNT
+ON TEST_KYLIN_FACT.SELLER_ID = SELLER_ACCOUNT.ACCOUNT_ID
+INNER JOIN defaultCatalog.EDW.TEST_CAL_DT as TEST_CAL_DT
+ON TEST_KYLIN_FACT.CAL_DT = TEST_CAL_DT.CAL_DT
+INNER JOIN defaultCatalog.TEST_CATEGORY_GROUPINGS as TEST_CATEGORY_GROUPINGS
+ON TEST_KYLIN_FACT.LEAF_CATEG_ID = TEST_CATEGORY_GROUPINGS.LEAF_CATEG_ID AND 
TEST_KYLIN_FACT.LSTG_SITE_ID = TEST_CATEGORY_GROUPINGS.SITE_ID
+INNER JOIN defaultCatalog.EDW.TEST_SITES as TEST_SITES
+ON TEST_KYLIN_FACT.LSTG_SITE_ID = TEST_SITES.SITE_ID
+INNER JOIN defaultCatalog.EDW.TEST_SELLER_TYPE_DIM as TEST_SELLER_TYPE_DIM
+ON TEST_KYLIN_FACT.SLR_SEGMENT_CD = TEST_SELLER_TYPE_DIM.SELLER_TYPE_CD
+INNER JOIN defaultCatalog.TEST_COUNTRY as BUYER_COUNTRY
+ON BUYER_ACCOUNT.ACCOUNT_COUNTRY = BUYER_COUNTRY.COUNTRY
+INNER JOIN defaultCatalog.TEST_COUNTRY as SELLER_COUNTRY
+ON SELLER_ACCOUNT.ACCOUNT_COUNTRY = SELLER_COUNTRY.COUNTRY
diff --git a/kylin-it/src/test/resources/query/sql_snowflake/query08.sql 
b/kylin-it/src/test/resources/query/sql_snowflake/query08.sql
new file mode 100644
index 0000000000..454779c0af
--- /dev/null
+++ b/kylin-it/src/test/resources/query/sql_snowflake/query08.sql
@@ -0,0 +1,14 @@
+SELECT
+
+count(*) as cnt, sum(price) as sum_price, SELLER_COUNTRY.NAME
+
+FROM TEST_KYLIN_FACT as TEST_KYLIN_FACT 
+INNER JOIN defaultCatalog.TEST_ACCOUNT as SELLER_ACCOUNT
+ON TEST_KYLIN_FACT.SELLER_ID = SELLER_ACCOUNT.ACCOUNT_ID
+INNER JOIN defaultCatalog.TEST_CATEGORY_GROUPINGS as TEST_CATEGORY_GROUPINGS
+ON TEST_KYLIN_FACT.LEAF_CATEG_ID = TEST_CATEGORY_GROUPINGS.LEAF_CATEG_ID AND 
TEST_KYLIN_FACT.LSTG_SITE_ID = TEST_CATEGORY_GROUPINGS.SITE_ID
+INNER JOIN defaultCatalog.TEST_COUNTRY as SELLER_COUNTRY
+ON SELLER_ACCOUNT.ACCOUNT_COUNTRY = SELLER_COUNTRY.COUNTRY
+
+where SELLER_ACCOUNT.ACCOUNT_SELLER_LEVEL=1
+group by SELLER_COUNTRY.NAME
\ No newline at end of file
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 4378221a3c..1e7498b511 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
@@ -38,12 +38,20 @@
 
     protected static final Logger logger = 
LoggerFactory.getLogger(QueryUtil.class);
 
+    private QueryUtil() {
+        throw new IllegalStateException("Class QueryUtil is an utility class 
!");
+    }
+
     private static List<IQueryTransformer> queryTransformers;
 
     public interface IQueryTransformer {
         String transform(String sql, String project, String defaultSchema);
     }
 
+    /**
+     * @deprecated Deprecated because of KYLIN-3594
+     */
+    @Deprecated
     public static String massageSql(String sql, String project, int limit, int 
offset, String defaultSchema) {
         sql = sql.trim();
         sql = sql.replace("\r", " ").replace("\n", 
System.getProperty("line.separator"));
@@ -55,7 +63,7 @@ public static String massageSql(String sql, String project, 
int limit, int offse
         while (sql.endsWith(";"))
             sql = sql.substring(0, sql.length() - 1);
 
-        String sql1=sql;
+        String sql1 = sql;
         final String suffixPattern = 
"^.+?\\s(limit\\s\\d+)?\\s(offset\\s\\d+)?\\s*$";
         sql = sql.replaceAll("\\s+", " ");
         Pattern pattern = Pattern.compile(suffixPattern);
@@ -86,6 +94,29 @@ public static String massageSql(String sql, String project, 
int limit, int offse
         return sql1;
     }
 
+    /**
+     * add remove catalog step at final
+     */
+    public static String massageSql(String sql, String project, int limit, int 
offset, String defaultSchema, String catalog) {
+        String correctedSql = massageSql(sql, project, limit, offset, 
defaultSchema);
+        correctedSql = removeCatalog(correctedSql, catalog);
+        return correctedSql;
+    }
+
+    /**
+     * Although SQL standard define CATALOG concept, ISV has right not to 
implement it.
+     * We remove it in before send it to SQL parser.
+     *
+     * @param sql query which maybe has such pattern: 
[[catalogName.]schemaName.]tableName
+     * @return replace [[catalogName.]schemaName.]tableName with 
[schemaName.]tableName
+     */
+    static String removeCatalog(String sql, String catalog) {
+        if (catalog == null)
+            return sql;
+        else
+            return sql.replace(catalog + ".", "");
+    }
+
     private static void initQueryTransformers() {
         List<IQueryTransformer> transformers = Lists.newArrayList();
 
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 30f4934f38..bf6f8363be 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
@@ -27,6 +27,8 @@
 
 public class QueryUtilTest extends LocalFileMetadataTestCase {
 
+    static final String catalog = "CATALOG";
+
     @Before
     public void setUp() throws Exception {
         this.createTestMetadata();
@@ -231,4 +233,34 @@ public void testUnknownErrorResponseMessage() {
         String msg = QueryUtil.makeErrorMsgUserFriendly(new 
NullPointerException());
         Assert.assertEquals("Unknown error.", msg);
     }
+
+    @Test
+    public void testRemoveCatalog() {
+
+        String[] beforeRemoveSql = new String[] {
+                "select name, count(*) as cnt from schema1.user where bb.dd >2 
group by name",
+                "select name, count(*) as cnt from .default2.user where dd >2 
group by name",
+                "select name, count(*) as cnt from %s.default2.user where dd 
>2 group by name",
+                "select name, count(*) as cnt from %s.user.a.cu where dd >2 
group by name",
+                "select name, count(*) as cnt from %s.default2.user where dd 
>2 group by name",
+                "select name, count() as cnt from %s.test.kylin_sales inner 
join " + "%s.test.kylin_account "
+                        + "ON kylin_sales.BUYER_ID=kylin_account.ACCOUNT_ID 
group by name",
+                "select schema1.table1.col1 from %s.schema1.table1" };
+        String[] afterRemoveSql = new String[] {
+                "select name, count(*) as cnt from schema1.user where bb.dd >2 
group by name",
+                "select name, count(*) as cnt from .default2.user where dd >2 
group by name",
+                "select name, count(*) as cnt from default2.user where dd >2 
group by name",
+                "select name, count(*) as cnt from user.a.cu where dd >2 group 
by name",
+                "select name, count(*) as cnt from default2.user where dd >2 
group by name",
+                "select name, count() as cnt from test.kylin_sales inner join 
" + "test.kylin_account "
+                        + "ON kylin_sales.BUYER_ID=kylin_account.ACCOUNT_ID 
group by name",
+                "select schema1.table1.col1 from schema1.table1" };
+        Assert.assertEquals(afterRemoveSql.length, beforeRemoveSql.length);
+        for (int i = 0; i < beforeRemoveSql.length; i++) {
+            String before = beforeRemoveSql[i];
+            before = before.replace("%s", catalog);
+            String after = afterRemoveSql[i];
+            Assert.assertEquals(after, QueryUtil.removeCatalog(before, 
catalog));
+        }
+    }
 }
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 65054c425d..9e6b4afe49 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
@@ -558,9 +558,8 @@ private SQLResponse queryWithSqlMassage(SQLRequest 
sqlRequest) throws Exception
                 logger.debug("Return fake response, is exception? " + 
fakeResponse.getIsException());
                 return fakeResponse;
             }
-
             String correctedSql = QueryUtil.massageSql(sqlRequest.getSql(), 
sqlRequest.getProject(),
-                    sqlRequest.getLimit(), sqlRequest.getOffset(), 
conn.getSchema());
+                    sqlRequest.getLimit(), sqlRequest.getOffset(), 
conn.getSchema(), Constant.FakeCatalogName);
             if (!correctedSql.equals(sqlRequest.getSql())) {
                 logger.info("The corrected query: " + correctedSql);
 
@@ -1087,23 +1086,23 @@ private void setParam(PreparedStatement preparedState, 
int index, PrepareSqlRequ
             break;
         case PRIMITIVE_INT:
         case INTEGER:
-            preparedState.setInt(index, isNull ? 0 : 
Integer.valueOf(param.getValue()));
+            preparedState.setInt(index, isNull ? 0 : 
Integer.parseInt(param.getValue()));
             break;
         case PRIMITIVE_SHORT:
         case SHORT:
-            preparedState.setShort(index, isNull ? 0 : 
Short.valueOf(param.getValue()));
+            preparedState.setShort(index, isNull ? 0 : 
Short.parseShort(param.getValue()));
             break;
         case PRIMITIVE_LONG:
         case LONG:
-            preparedState.setLong(index, isNull ? 0 : 
Long.valueOf(param.getValue()));
+            preparedState.setLong(index, isNull ? 0 : 
Long.parseLong(param.getValue()));
             break;
         case PRIMITIVE_FLOAT:
         case FLOAT:
-            preparedState.setFloat(index, isNull ? 0 : 
Float.valueOf(param.getValue()));
+            preparedState.setFloat(index, isNull ? 0 : 
Float.parseFloat(param.getValue()));
             break;
         case PRIMITIVE_DOUBLE:
         case DOUBLE:
-            preparedState.setDouble(index, isNull ? 0 : 
Double.valueOf(param.getValue()));
+            preparedState.setDouble(index, isNull ? 0 : 
Double.parseDouble(param.getValue()));
             break;
         case PRIMITIVE_BOOLEAN:
         case BOOLEAN:
@@ -1111,7 +1110,7 @@ private void setParam(PreparedStatement preparedState, 
int index, PrepareSqlRequ
             break;
         case PRIMITIVE_BYTE:
         case BYTE:
-            preparedState.setByte(index, isNull ? 0 : 
Byte.valueOf(param.getValue()));
+            preparedState.setByte(index, isNull ? 0 : 
Byte.parseByte(param.getValue()));
             break;
         case JAVA_UTIL_DATE:
         case JAVA_SQL_DATE:


 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to