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 2023/01/29 09:31:43 UTC
[kylin] 01/06: KYLIN-5402 fix performance of checking computed columns
This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 618c2e37c72a40774cbfdb2222676b06210f4319
Author: Pengfei Zhan <pe...@kyligence.io>
AuthorDate: Fri Nov 11 22:57:51 2022 +0800
KYLIN-5402 fix performance of checking computed columns
1. promote the performance of checking computed columns;
2. promote the performance of saving model;
3. resolve the code complexity of ModelService#checkModelMeasures.
---
.../kylin/rest/service/ModelServiceBuildTest.java | 2 +-
.../kylin/rest/service/ModelSemanticHelper.java | 2 +-
.../apache/kylin/rest/service/ModelService.java | 79 ++++++++++++----------
.../kylin/rest/service/ModelServiceTest.java | 20 +++++-
.../kylin/rest/service/ModelServiceQueryTest.java | 2 +-
.../service/ModelServiceWithSecondStorageTest.java | 2 +-
.../apache/kylin/rest/util/AclPermissionUtil.java | 4 +-
7 files changed, 69 insertions(+), 42 deletions(-)
diff --git a/src/data-loading-service/src/test/java/org/apache/kylin/rest/service/ModelServiceBuildTest.java b/src/data-loading-service/src/test/java/org/apache/kylin/rest/service/ModelServiceBuildTest.java
index 72df60801a..de7cef0bf1 100644
--- a/src/data-loading-service/src/test/java/org/apache/kylin/rest/service/ModelServiceBuildTest.java
+++ b/src/data-loading-service/src/test/java/org/apache/kylin/rest/service/ModelServiceBuildTest.java
@@ -201,7 +201,7 @@ public class ModelServiceBuildTest extends SourceTestCase {
ReflectionTestUtils.setField(semanticService, "expandableMeasureUtil",
new ExpandableMeasureUtil((model, ccDesc) -> {
String ccExpression = QueryUtil.massageComputedColumn(model, model.getProject(), ccDesc,
- AclPermissionUtil.prepareQueryContextACLInfo(model.getProject(),
+ AclPermissionUtil.createAclInfo(model.getProject(),
semanticService.getCurrentUserGroups()));
ccDesc.setInnerExpression(ccExpression);
ComputedColumnEvalUtil.evaluateExprAndType(model, ccDesc);
diff --git a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelSemanticHelper.java b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelSemanticHelper.java
index f6866f2708..54190bcd77 100644
--- a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelSemanticHelper.java
+++ b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelSemanticHelper.java
@@ -131,7 +131,7 @@ public class ModelSemanticHelper extends BasicService {
private static final Logger logger = LoggerFactory.getLogger(ModelSemanticHelper.class);
private final ExpandableMeasureUtil expandableMeasureUtil = new ExpandableMeasureUtil((model, ccDesc) -> {
String ccExpression = QueryUtil.massageComputedColumn(model, model.getProject(), ccDesc,
- AclPermissionUtil.prepareQueryContextACLInfo(model.getProject(), getCurrentUserGroups()));
+ AclPermissionUtil.createAclInfo(model.getProject(), getCurrentUserGroups()));
ccDesc.setInnerExpression(ccExpression);
ComputedColumnEvalUtil.evaluateExprAndType(model, ccDesc);
});
diff --git a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelService.java b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelService.java
index 77fd3c6c5f..1f915c3dfa 100644
--- a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelService.java
+++ b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/ModelService.java
@@ -108,6 +108,7 @@ import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.apache.kylin.common.KapConfig;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.QueryContext;
import org.apache.kylin.common.event.ModelAddEvent;
import org.apache.kylin.common.event.ModelDropEvent;
import org.apache.kylin.common.exception.JobErrorCode;
@@ -303,6 +304,7 @@ public class ModelService extends AbstractModelService implements TableModelSupp
public static final Pattern VALID_NAME_FOR_MEASURE = Pattern.compile("^[\\u4E00-\\u9FA5a-zA-Z0-9 _\\-()%?（）.]+$");
private static final List<String> MODEL_CONFIG_BLOCK_LIST = Lists.newArrayList("kylin.index.rule-scheduler-data");
+ private static final Set<String> STRING_TYPE_SET = Sets.newHashSet("STRING", "CHAR", "VARCHAR");
//The front-end supports only the following formats
private static final List<String> SUPPORTED_FORMATS = ImmutableList.of("ZZ", "DD", "D", "Do", "dddd", "ddd", "dd", //
@@ -1714,8 +1716,9 @@ public class ModelService extends AbstractModelService implements TableModelSupp
@VisibleForTesting
public void checkFlatTableSql(NDataModel dataModel) {
- if (KylinConfig.getInstanceFromEnv().isUTEnv() || getManager(NProjectManager.class)
- .getProject(dataModel.getProject()).getConfig().isSkipCheckFlatTable()) {
+ String project = dataModel.getProject();
+ ProjectInstance prjInstance = getManager(NProjectManager.class).getProject(project);
+ if (KylinConfig.getInstanceFromEnv().isUTEnv() || prjInstance.getConfig().isSkipCheckFlatTable()) {
return;
}
if (getModelConfig(dataModel).skipCheckFlatTable()) {
@@ -1729,15 +1732,13 @@ public class ModelService extends AbstractModelService implements TableModelSupp
}
try {
- ProjectInstance projectInstance = getManager(NProjectManager.class).getProject(dataModel.getProject());
- if (projectInstance.getSourceType() == ISourceAware.ID_SPARK
+ if (prjInstance.getSourceType() == ISourceAware.ID_SPARK
&& dataModel.getModelType() == NDataModel.ModelType.BATCH) {
SparkSession ss = SparderEnv.getSparkSession();
String flatTableSql = JoinedFlatTable.generateSelectDataStatement(dataModel, false);
- QueryParams queryParams = new QueryParams(dataModel.getProject(), flatTableSql, "default", false);
- queryParams.setKylinConfig(projectInstance.getConfig());
- queryParams.setAclInfo(
- AclPermissionUtil.prepareQueryContextACLInfo(dataModel.getProject(), getCurrentUserGroups()));
+ QueryParams queryParams = new QueryParams(project, flatTableSql, "default", false);
+ queryParams.setKylinConfig(prjInstance.getConfig());
+ queryParams.setAclInfo(AclPermissionUtil.createAclInfo(project, getCurrentUserGroups()));
String pushdownSql = QueryUtil.massagePushDownSql(queryParams);
ss.sql(pushdownSql);
}
@@ -2163,27 +2164,38 @@ public class ModelService extends AbstractModelService implements TableModelSupp
}
}
- @VisibleForTesting
- public void checkModelMeasures(ModelRequest request) {
- Set<String> measureNames = new HashSet<>();
- Set<SimplifiedMeasure> measures = new HashSet<>();
- KylinConfig kylinConfig = getManager(NProjectManager.class).getProject(request.getProject()).getConfig();
- int maxModelDimensionMeasureNameLength = kylinConfig.getMaxModelDimensionMeasureNameLength();
-
+ private void checkMeasureNameValid(ModelRequest request) {
+ int maxLen = NProjectManager.getProjectConfig(request.getProject()).getMaxModelDimensionMeasureNameLength();
+ String invalidPattern = MsgPicker.getMsg().getInvalidMeasureName();
for (SimplifiedMeasure measure : request.getSimplifiedMeasures()) {
- measure.setName(StringUtils.trim(measure.getName()));
- // check if the measure name is valid
- if (StringUtils.length(measure.getName()) > maxModelDimensionMeasureNameLength
- || !checkIsValidMeasureName(measure.getName()))
- throw new KylinException(INVALID_NAME,
- String.format(Locale.ROOT, MsgPicker.getMsg().getInvalidMeasureName(), measure.getName(),
- maxModelDimensionMeasureNameLength));
+ String name = measure.getName();
+ if (StringUtils.length(name) <= maxLen && checkIsValidMeasureName(name)) {
+ continue;
+ }
+ throw new KylinException(INVALID_NAME, String.format(Locale.ROOT, invalidPattern, name, maxLen));
+ }
+ }
- // check duplicate measure names
- if (measureNames.contains(measure.getName()))
+ private void checkMeasureNameDuplicate(ModelRequest modelRequest) {
+ Set<String> measureNames = Sets.newHashSet();
+ for (SimplifiedMeasure measure : modelRequest.getSimplifiedMeasures()) {
+ if (measureNames.contains(measure.getName())) {
throw new KylinException(DUPLICATE_MEASURE_NAME,
String.format(Locale.ROOT, MsgPicker.getMsg().getDuplicateMeasureName(), measure.getName()));
+ } else {
+ measureNames.add(measure.getName());
+ }
+ }
+ }
+ @VisibleForTesting
+ public void checkModelMeasures(ModelRequest request) {
+ Set<SimplifiedMeasure> measures = new HashSet<>();
+ request.getSimplifiedMeasures().forEach(measure -> measure.setName(StringUtils.trim(measure.getName())));
+ checkMeasureNameValid(request);
+ checkMeasureNameDuplicate(request);
+
+ for (SimplifiedMeasure measure : request.getSimplifiedMeasures()) {
// check duplicate measure definitions
SimplifiedMeasure dupMeasure = null;
for (SimplifiedMeasure m : measures) {
@@ -2207,7 +2219,6 @@ public class ModelService extends AbstractModelService implements TableModelSupp
MsgPicker.getMsg().getDuplicateMeasureDefinition(), measure.getName()));
}
- measureNames.add(measure.getName());
measures.add(measure);
}
}
@@ -2630,6 +2641,7 @@ public class ModelService extends AbstractModelService implements TableModelSupp
checkCCNameAmbiguity(model);
ComputedColumnDesc checkedCC = null;
+ QueryContext.AclInfo aclInfo = AclPermissionUtil.createAclInfo(project, getCurrentUserGroups());
Set<String> excludedTables = getManager(FavoriteRuleManager.class, project).getExcludedTables();
ExcludedLookupChecker checker = new ExcludedLookupChecker(excludedTables, model.getJoinTables(), model);
for (ComputedColumnDesc cc : model.getComputedColumnDescs()) {
@@ -2645,8 +2657,7 @@ public class ModelService extends AbstractModelService implements TableModelSupp
MsgPicker.getMsg().getccOnAntiFlattenLookup(), antiFlattenLookup));
}
ComputedColumnDesc.simpleParserCheck(cc.getExpression(), model.getAliasMap().keySet());
- String innerExpression = QueryUtil.massageComputedColumn(model, project, cc,
- AclPermissionUtil.prepareQueryContextACLInfo(project, getCurrentUserGroups()));
+ String innerExpression = QueryUtil.massageComputedColumn(model, project, cc, aclInfo);
cc.setInnerExpression(innerExpression);
//check by data source, this could be slow
@@ -2746,9 +2757,9 @@ public class ModelService extends AbstractModelService implements TableModelSupp
checkCCNameAmbiguity(model);
// Update CC expression from query transformers
+ QueryContext.AclInfo aclInfo = AclPermissionUtil.createAclInfo(project, getCurrentUserGroups());
for (ComputedColumnDesc ccDesc : model.getComputedColumnDescs()) {
- String ccExpression = QueryUtil.massageComputedColumn(model, project, ccDesc,
- AclPermissionUtil.prepareQueryContextACLInfo(project, getCurrentUserGroups()));
+ String ccExpression = QueryUtil.massageComputedColumn(model, project, ccDesc, aclInfo);
ccDesc.setInnerExpression(ccExpression);
TblColRef tblColRef = model.findColumn(ccDesc.getTableAlias(), ccDesc.getColumnName());
tblColRef.getColumnDesc().setComputedColumn(ccExpression);
@@ -3599,7 +3610,7 @@ public class ModelService extends AbstractModelService implements TableModelSupp
}
String massagedFilterCond = QueryUtil.massageExpression(model, model.getProject(), model.getFilterCondition(),
- AclPermissionUtil.prepareQueryContextACLInfo(model.getProject(), getCurrentUserGroups()), false);
+ AclPermissionUtil.createAclInfo(model.getProject(), getCurrentUserGroups()), false);
String filterConditionWithTableName = addTableNameIfNotExist(massagedFilterCond, model);
@@ -3853,7 +3864,7 @@ public class ModelService extends AbstractModelService implements TableModelSupp
}
public NDataModel updateResponseAcl(NDataModel model, String project) {
- List<NDataModel> models = updateResponseAcl(Arrays.asList(model), project);
+ List<NDataModel> models = updateResponseAcl(Collections.singletonList(model), project);
return models.get(0);
}
@@ -3905,8 +3916,6 @@ public class ModelService extends AbstractModelService implements TableModelSupp
}
}
- private static final Set<String> StringTypes = Sets.newHashSet("STRING", "CHAR", "VARCHAR");
-
private boolean matchCCDataType(String actual, String expected) {
if (actual == null) {
return false;
@@ -3915,8 +3924,8 @@ public class ModelService extends AbstractModelService implements TableModelSupp
actual = actual.toUpperCase(Locale.ROOT);
expected = expected.toUpperCase(Locale.ROOT);
// char, varchar, string are of the same
- if (StringTypes.contains(actual)) {
- return StringTypes.contains(expected);
+ if (STRING_TYPE_SET.contains(actual)) {
+ return STRING_TYPE_SET.contains(expected);
}
// if actual type is of decimal type with no prec, scala
diff --git a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
index e8edf45716..26a2cf1eaf 100644
--- a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
+++ b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java
@@ -18,6 +18,7 @@
package org.apache.kylin.rest.service;
+import static org.apache.kylin.common.exception.ServerErrorCode.FAILED_EXECUTE_MODEL_SQL;
import static org.apache.kylin.common.exception.ServerErrorCode.INVALID_PARTITION_COLUMN;
import static org.apache.kylin.common.exception.ServerErrorCode.PERMISSION_DENIED;
import static org.apache.kylin.common.exception.code.ErrorCodeServer.COMPUTED_COLUMN_CONFLICT;
@@ -289,7 +290,7 @@ public class ModelServiceTest extends SourceTestCase {
ReflectionTestUtils.setField(semanticService, "expandableMeasureUtil",
new ExpandableMeasureUtil((model, ccDesc) -> {
String ccExpression = QueryUtil.massageComputedColumn(model, model.getProject(), ccDesc,
- AclPermissionUtil.prepareQueryContextACLInfo(model.getProject(),
+ AclPermissionUtil.createAclInfo(model.getProject(),
semanticService.getCurrentUserGroups()));
ccDesc.setInnerExpression(ccExpression);
ComputedColumnEvalUtil.evaluateExprAndType(model, ccDesc);
@@ -4870,6 +4871,23 @@ public class ModelServiceTest extends SourceTestCase {
Assert.assertNull(model.getMeasures().get(1).getComment());
}
+ @Test
+ public void testCheckFlatTableSql() {
+ String project = "default";
+ String modelId = "a8ba3ff1-83bd-4066-ad54-d2fb3d1f0e94";
+ NDataModel model = NDataModelManager.getInstance(getTestConfig(), project).getDataModelDesc(modelId);
+ modelService.checkFlatTableSql(model);
+
+ getTestConfig().setProperty("kylin.env", "PROD");
+ try {
+ modelService.checkFlatTableSql(model);
+ Assert.fail();
+ } catch (KylinException e) {
+ Assert.assertEquals(FAILED_EXECUTE_MODEL_SQL.toErrorCode().getCodeString(),
+ e.getErrorCode().getCodeString());
+ }
+ }
+
@Test
public void testUpdateModelWithDirtyMeasures() {
String project = "gc_test";
diff --git a/src/query-service/src/test/java/org/apache/kylin/rest/service/ModelServiceQueryTest.java b/src/query-service/src/test/java/org/apache/kylin/rest/service/ModelServiceQueryTest.java
index c112280a1b..b28c4e96b6 100644
--- a/src/query-service/src/test/java/org/apache/kylin/rest/service/ModelServiceQueryTest.java
+++ b/src/query-service/src/test/java/org/apache/kylin/rest/service/ModelServiceQueryTest.java
@@ -115,7 +115,7 @@ public class ModelServiceQueryTest extends SourceTestCase {
ReflectionTestUtils.setField(semanticService, "expandableMeasureUtil",
new ExpandableMeasureUtil((model, ccDesc) -> {
String ccExpression = QueryUtil.massageComputedColumn(model, model.getProject(), ccDesc,
- AclPermissionUtil.prepareQueryContextACLInfo(model.getProject(),
+ AclPermissionUtil.createAclInfo(model.getProject(),
semanticService.getCurrentUserGroups()));
ccDesc.setInnerExpression(ccExpression);
ComputedColumnEvalUtil.evaluateExprAndType(model, ccDesc);
diff --git a/src/second-storage/core-ui/src/test/java/org/apache/kylin/rest/service/ModelServiceWithSecondStorageTest.java b/src/second-storage/core-ui/src/test/java/org/apache/kylin/rest/service/ModelServiceWithSecondStorageTest.java
index 3df761f6c2..787b16f4c6 100644
--- a/src/second-storage/core-ui/src/test/java/org/apache/kylin/rest/service/ModelServiceWithSecondStorageTest.java
+++ b/src/second-storage/core-ui/src/test/java/org/apache/kylin/rest/service/ModelServiceWithSecondStorageTest.java
@@ -142,7 +142,7 @@ public class ModelServiceWithSecondStorageTest extends NLocalFileMetadataTestCas
ReflectionTestUtils.setField(semanticService, "expandableMeasureUtil",
new ExpandableMeasureUtil((model, ccDesc) -> {
String ccExpression = QueryUtil.massageComputedColumn(model, model.getProject(), ccDesc,
- AclPermissionUtil.prepareQueryContextACLInfo(model.getProject(),
+ AclPermissionUtil.createAclInfo(model.getProject(),
semanticService.getCurrentUserGroups()));
ccDesc.setInnerExpression(ccExpression);
ComputedColumnEvalUtil.evaluateExprAndType(model, ccDesc);
diff --git a/src/systools/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java b/src/systools/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java
index 36b5b3723a..6a70456aba 100644
--- a/src/systools/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java
+++ b/src/systools/src/main/java/org/apache/kylin/rest/util/AclPermissionUtil.java
@@ -33,6 +33,7 @@ import org.apache.kylin.common.QueryContext;
import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.msg.MsgPicker;
import org.apache.kylin.common.persistence.AclEntity;
+import org.apache.kylin.metadata.project.NProjectManager;
import org.apache.kylin.metadata.project.ProjectInstance;
import org.apache.kylin.rest.constant.Constant;
import org.apache.kylin.rest.security.AclEntityFactory;
@@ -43,7 +44,6 @@ import org.apache.kylin.rest.security.AclPermissionFactory;
import org.apache.kylin.rest.security.CompositeAclPermission;
import org.apache.kylin.rest.security.MutableAclRecord;
import org.apache.kylin.rest.security.ObjectIdentityImpl;
-import org.apache.kylin.metadata.project.NProjectManager;
import org.springframework.security.acls.domain.BasePermission;
import org.springframework.security.acls.domain.GrantedAuthoritySid;
import org.springframework.security.acls.domain.PrincipalSid;
@@ -231,7 +231,7 @@ public class AclPermissionUtil {
}
}
- public static QueryContext.AclInfo prepareQueryContextACLInfo(String project, Set<String> groups) {
+ public static QueryContext.AclInfo createAclInfo(String project, Set<String> groups) {
return new QueryContext.AclInfo(getCurrentUsername(), groups, isAdminInProject(project, groups));
}