Skip to content

Commit

Permalink
[KYLIN-4554] validate filter condition on model saving
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zheniantoushipashi authored and hit-lacus committed May 8, 2021
1 parent b577521 commit 9589802
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -83,7 +91,7 @@ public Object getValue(String key) {
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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;
Expand All @@ -40,12 +41,15 @@
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;
Expand All @@ -54,9 +58,6 @@
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
*/
Expand Down Expand Up @@ -184,6 +185,16 @@ public void checkModelCompatibility(DataModelDesc dataModalDesc, List<TableDesc>
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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -64,6 +64,29 @@ public void testSuccessModelUpdate() throws IOException, JobException {
Assert.assertTrue(dataModelDesc.getOwner().equals("somebody"));
}

@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);
Expand Down

0 comments on commit 9589802

Please sign in to comment.