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:19 UTC
[kylin] 02/02: [KYLIN-4554] validate filter condition on model
saving,
fix bug added all the columns of all tables to the validate collection
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 b77c92a49d6a5cef116b15f78255fd811eccc3be
Author: zheniantoushipashi <su...@gmail.com>
AuthorDate: Wed Apr 14 11:06:49 2021 +0800
[KYLIN-4554] validate filter condition on model saving, fix bug added all the columns of all tables to the validate collection
---
.../org/apache/kylin/metadata/util/ModelUtil.java | 47 ++++++++++++++++------
.../apache/kylin/rest/service/ModelService.java | 4 +-
.../kylin/rest/service/ModelServiceTest.java | 2 +-
3 files changed, 37 insertions(+), 16 deletions(-)
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
index a3dbf8d..2c09c48 100644
--- 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
@@ -18,8 +18,10 @@
package org.apache.kylin.metadata.util;
+import java.util.Arrays;
import java.util.List;
import java.util.Locale;
+import java.util.stream.Collectors;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlIdentifier;
@@ -27,7 +29,9 @@ 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.apache.kylin.metadata.TableMetadataManager;
+import org.apache.kylin.metadata.model.ColumnDesc;
+import org.apache.kylin.metadata.model.DataModelDesc;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -37,15 +41,15 @@ public class ModelUtil {
private static final Logger logger = LoggerFactory.getLogger(ModelUtil.class);
- public static void verifyFilterCondition(String factTableName, String filterCondition, TableDesc tableDesc)
- throws Exception {
+ public static void verifyFilterCondition(String project, TableMetadataManager tableManager, DataModelDesc modelDesc,
+ String filterCondition) throws Exception {
StringBuilder checkSql = new StringBuilder();
- checkSql.append("select * from ").append(factTableName).append(" where ").append(filterCondition);
+ checkSql.append("select * from ").append(modelDesc.getRootFactTableName()).append(" where ")
+ .append(filterCondition);
SqlCall inputToNode = (SqlCall) parse(doubleQuoteKeywordDefault(checkSql.toString()));
- SqlVerify sqlVerify = new SqlVerify(tableDesc);
+ SqlVerify sqlVerify = new SqlVerify(project, tableManager, modelDesc);
sqlVerify.visit(inputToNode);
-
}
public static SqlNode parse(String sql) throws Exception {
@@ -56,16 +60,20 @@ public class ModelUtil {
private static class SqlVerify extends SqlBasicVisitor {
- private TableDesc tableDesc;
+ private DataModelDesc modelDesc;
+ private TableMetadataManager tableManager;
+ private String project;
- SqlVerify(TableDesc tableDesc) {
- this.tableDesc = tableDesc;
+ SqlVerify(String project, TableMetadataManager tableManager, DataModelDesc modelDesc) {
+ this.modelDesc = modelDesc;
+ this.tableManager = tableManager;
+ this.project = project;
}
@Override
public Object visit(SqlCall call) {
SqlSelect select = (SqlSelect) call;
- WhereColumnVerify.verify(select.getWhere(), tableDesc);
+ WhereColumnVerify.verify(project, select.getWhere(), modelDesc, tableManager);
return null;
}
}
@@ -74,11 +82,20 @@ public class ModelUtil {
private List<String> allSqlIdentifier = Lists.newArrayList();
- static void verify(SqlNode whereNode, TableDesc tableDesc) {
+ static void verify(String project, SqlNode whereNode, DataModelDesc modelDesc,
+ TableMetadataManager tableManager) {
WhereColumnVerify whereColumnVerify = new WhereColumnVerify();
whereNode.accept(whereColumnVerify);
+
+ List<ColumnDesc> columnDesc = Arrays.stream(modelDesc.getJoinTables()).flatMap(table -> {
+ return Arrays.stream(tableManager.getTableDesc(table.getTable(), project).getColumns());
+ }).collect(Collectors.toList());
+ columnDesc.addAll(
+ Arrays.asList(tableManager.getTableDesc(modelDesc.getRootFactTableName(), project).getColumns()));
+ List<String> allColumn = columnDesc.stream().map(cd -> cd.getName().toLowerCase(Locale.ROOT))
+ .collect(Collectors.toList());
whereColumnVerify.allSqlIdentifier.stream().forEach(col -> {
- if (tableDesc.findColumnByName(col) == null) {
+ if (!allColumn.contains(col.toLowerCase(Locale.ROOT))) {
String verifyError = String.format(Locale.ROOT,
"filter condition col: %s is not a column in the table ", col);
logger.error(verifyError);
@@ -88,7 +105,11 @@ public class ModelUtil {
}
public Object visit(SqlIdentifier id) {
- allSqlIdentifier.add(id.names.get(0));
+ if (id.names.size() == 1) {
+ allSqlIdentifier.add(id.names.get(0));
+ } else if (id.names.size() == 2) {
+ allSqlIdentifier.add(id.names.get(1));
+ }
return null;
}
}
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 63cccfc..4876208 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
@@ -189,8 +189,8 @@ public class ModelService extends BasicService {
if (!StringUtils.isEmpty(desc.getFilterCondition())) {
try {
JoinedFormatter formatter = new JoinedFormatter(true);
- ModelUtil.verifyFilterCondition(factTableName, formatter.formatSentence(desc.getFilterCondition()),
- tableDesc);
+ ModelUtil.verifyFilterCondition(project, getTableManager(), desc,
+ formatter.formatSentence(desc.getFilterCondition()));
} catch (Exception e) {
throw new BadRequestException(e.toString());
}
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 b6ff06d..d1c48c4 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
@@ -79,7 +79,7 @@ public class ModelServiceTest extends ServiceTestBase {
deserialize.setFilterCondition("TRANS_ID = 1");
modelService.validateModel("default", deserialize);
try {
- deserialize.setFilterCondition("TRANS_IDD = 1");
+ deserialize.setFilterCondition("kylin_account.TRANS_IDD = 1");
modelService.validateModel("default", deserialize);
Assert.fail("should throw an exception");
} catch (Exception e){