This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch kylin-on-parquet-v2
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 6d87071158cb9256552c880c0c29bc995eb2cb54
Author: zheniantoushipashi <sunbiaobia...@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 2bb803a..c488959 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;
@@ -39,12 +40,15 @@ import org.apache.kylin.metadata.model.JoinsTree;
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.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;
@@ -53,9 +57,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
*/
@@ -154,6 +155,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);
