You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by xx...@apache.org on 2021/05/08 04:10:18 UTC

[kylin] 01/02: [KYLIN-4554] validate filter condition on model saving

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

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

commit 9589802b4450a1a23047c4e26abdfe4e6b761a71
Author: zheniantoushipashi <su...@gmail.com>
AuthorDate: Sun Apr 11 18:23:53 2021 +0800

    [KYLIN-4554] validate filter condition on model saving
    
    verify filter condition using the following login
    
    1 select * from tableName where {filterCondition} pass   calcite  parse
    
    2 all column in filter condition must be model table column
---
 .../java/org/apache/kylin/job/JoinedFormatter.java |  10 +-
 .../org/apache/kylin/metadata/util/ModelUtil.java  | 103 +++++++++++++++++++++
 .../apache/kylin/rest/service/ModelService.java    |  17 +++-
 .../kylin/rest/service/ModelServiceTest.java       |  25 ++++-
 4 files changed, 150 insertions(+), 5 deletions(-)

diff --git a/core-job/src/main/java/org/apache/kylin/job/JoinedFormatter.java b/core-job/src/main/java/org/apache/kylin/job/JoinedFormatter.java
index 3f19ce9..20e0f47 100644
--- a/core-job/src/main/java/org/apache/kylin/job/JoinedFormatter.java
+++ b/core-job/src/main/java/org/apache/kylin/job/JoinedFormatter.java
@@ -56,6 +56,14 @@ public class JoinedFormatter {
         setDateEnv(flatDesc);
     }
 
+    public JoinedFormatter(Boolean validateModel) {
+        // for validate model filter condition
+        String start = "20190710";
+        String end = "20190711";
+        setKeyValue(START_DATE, start);
+        setKeyValue(END_DATE, end);
+    }
+
     private void setDateEnv(IJoinedFlatTableDesc flatDesc) {
         DataModelDesc model = flatDesc.getDataModel();
         PartitionDesc partDesc = model.getPartitionDesc();
@@ -83,7 +91,7 @@ public class JoinedFormatter {
         return value == null ? "" : value;
     }
 
-    String formatSentence(String sentence) {
+    public String formatSentence(String sentence) {
         String[] cArray = REG_PATTERN.split(sentence);
         StringBuilder sbr = new StringBuilder();
         List<String> keys = getKeys(sentence);
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/util/ModelUtil.java b/core-metadata/src/main/java/org/apache/kylin/metadata/util/ModelUtil.java
new file mode 100644
index 0000000..a3dbf8d
--- /dev/null
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/util/ModelUtil.java
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kylin.metadata.util;
+
+import java.util.List;
+import java.util.Locale;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlSelect;
+import org.apache.calcite.sql.parser.SqlParser;
+import org.apache.calcite.sql.util.SqlBasicVisitor;
+import org.apache.kylin.metadata.model.TableDesc;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.Lists;
+
+public class ModelUtil {
+
+    private static final Logger logger = LoggerFactory.getLogger(ModelUtil.class);
+
+    public static void verifyFilterCondition(String factTableName, String filterCondition, TableDesc tableDesc)
+            throws Exception {
+        StringBuilder checkSql = new StringBuilder();
+        checkSql.append("select * from ").append(factTableName).append(" where ").append(filterCondition);
+
+        SqlCall inputToNode = (SqlCall) parse(doubleQuoteKeywordDefault(checkSql.toString()));
+        SqlVerify sqlVerify = new SqlVerify(tableDesc);
+        sqlVerify.visit(inputToNode);
+
+    }
+
+    public static SqlNode parse(String sql) throws Exception {
+        SqlParser.ConfigBuilder parserBuilder = SqlParser.configBuilder().setIdentifierMaxLength(300);
+        SqlParser sqlParser = SqlParser.create(sql, parserBuilder.build());
+        return sqlParser.parseQuery();
+    }
+
+    private static class SqlVerify extends SqlBasicVisitor {
+
+        private TableDesc tableDesc;
+
+        SqlVerify(TableDesc tableDesc) {
+            this.tableDesc = tableDesc;
+        }
+
+        @Override
+        public Object visit(SqlCall call) {
+            SqlSelect select = (SqlSelect) call;
+            WhereColumnVerify.verify(select.getWhere(), tableDesc);
+            return null;
+        }
+    }
+
+    private static class WhereColumnVerify extends SqlBasicVisitor {
+
+        private List<String> allSqlIdentifier = Lists.newArrayList();
+
+        static void verify(SqlNode whereNode, TableDesc tableDesc) {
+            WhereColumnVerify whereColumnVerify = new WhereColumnVerify();
+            whereNode.accept(whereColumnVerify);
+            whereColumnVerify.allSqlIdentifier.stream().forEach(col -> {
+                if (tableDesc.findColumnByName(col) == null) {
+                    String verifyError = String.format(Locale.ROOT,
+                            "filter condition col: %s is not a column in the table ", col);
+                    logger.error(verifyError);
+                    throw new IllegalArgumentException(verifyError);
+                }
+            });
+        }
+
+        public Object visit(SqlIdentifier id) {
+            allSqlIdentifier.add(id.names.get(0));
+            return null;
+        }
+    }
+
+    public static String doubleQuoteKeywordDefault(String sql) {
+        sql = sql.replaceAll("(?i)default\\.", "\"DEFAULT\".");
+        sql = sql.replace("defaultCatalog.", "");
+        sql = sql.replace("\"defaultCatalog\".", "");
+        return sql;
+    }
+
+}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
index 7ff3919..63cccfc 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java
@@ -31,6 +31,7 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.kylin.common.persistence.RootPersistentEntity;
 import org.apache.kylin.cube.CubeInstance;
 import org.apache.kylin.cube.model.CubeDesc;
+import org.apache.kylin.job.JoinedFormatter;
 import org.apache.kylin.metadata.ModifiedOrder;
 import org.apache.kylin.metadata.draft.Draft;
 import org.apache.kylin.metadata.model.DataModelDesc;
@@ -40,12 +41,15 @@ import org.apache.kylin.metadata.model.ModelDimensionDesc;
 import org.apache.kylin.metadata.model.TableDesc;
 import org.apache.kylin.metadata.model.TblColRef;
 import org.apache.kylin.metadata.project.ProjectInstance;
+import org.apache.kylin.metadata.util.ModelUtil;
 import org.apache.kylin.rest.exception.BadRequestException;
 import org.apache.kylin.rest.exception.ForbiddenException;
 import org.apache.kylin.rest.msg.Message;
 import org.apache.kylin.rest.msg.MsgPicker;
 import org.apache.kylin.rest.util.AclEvaluate;
 import org.apache.kylin.rest.util.ValidateUtil;
+import org.apache.kylin.shaded.com.google.common.collect.Maps;
+import org.apache.kylin.shaded.com.google.common.collect.Sets;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -54,9 +58,6 @@ import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Component;
 
-import org.apache.kylin.shaded.com.google.common.collect.Maps;
-import org.apache.kylin.shaded.com.google.common.collect.Sets;
-
 /**
  * @author jiazhong
  */
@@ -184,6 +185,16 @@ public class ModelService extends BasicService {
     public void validateModel(String project, DataModelDesc desc) throws IllegalArgumentException {
         String factTableName = desc.getRootFactTableName();
         TableDesc tableDesc = getTableManager().getTableDesc(factTableName, project);
+
+        if (!StringUtils.isEmpty(desc.getFilterCondition())) {
+            try {
+                JoinedFormatter formatter = new JoinedFormatter(true);
+                ModelUtil.verifyFilterCondition(factTableName, formatter.formatSentence(desc.getFilterCondition()),
+                        tableDesc);
+            } catch (Exception e) {
+                throw new BadRequestException(e.toString());
+            }
+        }
         if ((tableDesc.getSourceType() == ISourceAware.ID_STREAMING || tableDesc.isStreamingTable())
                 && (desc.getPartitionDesc() == null || desc.getPartitionDesc().getPartitionDateColumn() == null)) {
             throw new IllegalArgumentException("Must define a partition column.");
diff --git a/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java b/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
index aeb6d79..b6ff06d 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
@@ -50,7 +50,7 @@ public class ModelServiceTest extends ServiceTestBase {
     @Test
     public void testSuccessModelUpdate() throws IOException, JobException {
         Serializer<DataModelDesc> serializer = modelService.getDataModelManager().getDataModelSerializer();
-        
+
         List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_inner_join_model", "default", true);
         Assert.assertTrue(dataModelDescs.size() == 1);
 
@@ -65,6 +65,29 @@ public class ModelServiceTest extends ServiceTestBase {
     }
 
     @Test
+    public void testVerifyFilterCondition() throws IOException {
+        Serializer<DataModelDesc> serializer = modelService.getDataModelManager()
+            .getDataModelSerializer();
+        List<DataModelDesc> dataModelDescs = modelService
+            .listAllModels("ci_inner_join_model", "default", true);
+        Assert.assertTrue(dataModelDescs.size() == 1);
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        serializer.serialize(dataModelDescs.get(0), new DataOutputStream(baos));
+        ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+        DataModelDesc deserialize = serializer.deserialize(new DataInputStream(bais));
+        deserialize.setOwner("somebody");
+        deserialize.setFilterCondition("TRANS_ID = 1");
+        modelService.validateModel("default", deserialize);
+        try {
+            deserialize.setFilterCondition("TRANS_IDD = 1");
+            modelService.validateModel("default", deserialize);
+            Assert.fail("should throw an exception");
+        } catch (Exception e){
+            Assert.assertTrue(e.getMessage().equals("java.lang.IllegalArgumentException: filter condition col: TRANS_IDD is not a column in the table "));
+        }
+    }
+
+    @Test
     public void testRevisableModelInCaseOfDeleteMeasure() throws IOException {
         List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true);
         Assert.assertTrue(dataModelDescs.size() == 1);