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/02/27 08:01:14 UTC
[kylin] 34/34: KYLIN-5450 fix NPE while col_order is not required
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 7fe52b87411451debd40f2b92eee6111f764de6c
Author: Ruixuan Zhang <ru...@kyligence.io>
AuthorDate: Wed Jan 11 15:21:26 2023 +0800
KYLIN-5450 fix NPE while col_order is not required
---
.../rest/controller/NIndexPlanController.java | 10 ---
.../rest/controller/IndexPlanControllerTest.java | 20 ++---
.../kylin/rest/service/IndexPlanService.java | 4 +
.../kylin/rest/service/IndexPlanServiceTest.java | 90 ++++++++++++++++++++++
4 files changed, 104 insertions(+), 20 deletions(-)
diff --git a/src/metadata-server/src/main/java/org/apache/kylin/rest/controller/NIndexPlanController.java b/src/metadata-server/src/main/java/org/apache/kylin/rest/controller/NIndexPlanController.java
index 3bca9f0ecf..24124071a5 100644
--- a/src/metadata-server/src/main/java/org/apache/kylin/rest/controller/NIndexPlanController.java
+++ b/src/metadata-server/src/main/java/org/apache/kylin/rest/controller/NIndexPlanController.java
@@ -21,7 +21,6 @@ package org.apache.kylin.rest.controller;
import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_JSON;
import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON;
import static org.apache.kylin.common.exception.code.ErrorCodeServer.LAYOUT_LIST_EMPTY;
-import static org.apache.kylin.common.exception.code.ErrorCodeServer.SHARD_BY_COLUMN_NOT_IN_INDEX;
import java.util.List;
import java.util.Set;
@@ -144,19 +143,10 @@ public class NIndexPlanController extends NBasicController {
checkRequiredArg(MODEL_ID, request.getModelId());
checkRequiredArg("id", request.getId());
modelService.validateCCType(request.getModelId(), request.getProject());
- List<String> shardByColumns = request.getShardByColumns();
- List<String> colOrder = request.getColOrder();
- checkShardbyCol(shardByColumns, colOrder);
val response = fusionIndexService.updateTableIndex(request.getProject(), request);
return new EnvelopeResponse<>(KylinException.CODE_SUCCESS, response, "");
}
- private void checkShardbyCol(List<String> shardByColumns, List<String> colOrder) {
- if (!colOrder.containsAll(shardByColumns)) {
- throw new KylinException(SHARD_BY_COLUMN_NOT_IN_INDEX);
- }
- }
-
@Deprecated
@ApiOperation(value = "deleteTableIndex", tags = { "AI" }, notes = "Update URL: {project}, Update Param: project")
@DeleteMapping(value = "/table_index/{id:.+}")
diff --git a/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/IndexPlanControllerTest.java b/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/IndexPlanControllerTest.java
index bcb9ef7ec1..7a1de9708c 100644
--- a/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/IndexPlanControllerTest.java
+++ b/src/metadata-server/src/test/java/org/apache/kylin/rest/controller/IndexPlanControllerTest.java
@@ -18,9 +18,7 @@
package org.apache.kylin.rest.controller;
import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_JSON;
-import static org.apache.kylin.common.exception.code.ErrorCodeServer.SHARD_BY_COLUMN_NOT_IN_INDEX;
-import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.util.JsonUtil;
import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
import org.apache.kylin.common.util.Pair;
@@ -36,7 +34,6 @@ import org.apache.kylin.rest.service.FusionIndexService;
import org.apache.kylin.rest.service.IndexPlanService;
import org.apache.kylin.rest.service.ModelService;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.InjectMocks;
@@ -162,13 +159,16 @@ public class IndexPlanControllerTest extends NLocalFileMetadataTestCase {
}
@Test
- public void testUpdateTableIndex() {
+ public void testUpdateTableIndex() throws Exception {
CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest.builder().project("default")
- .modelId("89af4ee2-2cdb-4b07-b39e-4c29856309aa").id(20000010000L)
- .colOrder(Lists.newArrayList("1", "0", "2")).shardByColumns(Lists.newArrayList("4"))
- .sortByColumns(Lists.newArrayList("0", "2")).build();
- Assert.assertThrows(SHARD_BY_COLUMN_NOT_IN_INDEX.getMsg(), KylinException.class, () -> {
- indexPlanController.updateTableIndex(tableIndexRequest);
- });
+ .modelId("89af4ee2-2cdb-4b07-b39e-4c29856309aa").id(20000010001L).colOrder(Lists
+ .newArrayList("TEST_KYLIN_FACT.TRANS_ID", "TEST_SITES.SITE_NAME", "TEST_KYLIN_FACT.CAL_DT"))
+ .sortByColumns(Lists.newArrayList()).build();
+ Mockito.when(indexPlanService.updateTableIndex(Mockito.anyString(), Mockito.any(CreateTableIndexRequest.class)))
+ .thenReturn(new BuildIndexResponse());
+ mockMvc.perform(MockMvcRequestBuilders.put("/api/index_plans/table_index")
+ .contentType(MediaType.APPLICATION_JSON).content(JsonUtil.writeValueAsString(tableIndexRequest))
+ .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON)))
+ .andExpect(MockMvcResultMatchers.status().isOk());
}
}
diff --git a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/IndexPlanService.java b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/IndexPlanService.java
index e45016aeeb..62d597daf5 100644
--- a/src/modeling-service/src/main/java/org/apache/kylin/rest/service/IndexPlanService.java
+++ b/src/modeling-service/src/main/java/org/apache/kylin/rest/service/IndexPlanService.java
@@ -21,6 +21,7 @@ import static org.apache.kylin.common.exception.ServerErrorCode.PERMISSION_DENIE
import static org.apache.kylin.common.exception.code.ErrorCodeServer.INDEX_DUPLICATE;
import static org.apache.kylin.common.exception.code.ErrorCodeServer.LAYOUT_LIST_EMPTY;
import static org.apache.kylin.common.exception.code.ErrorCodeServer.LAYOUT_NOT_EXISTS;
+import static org.apache.kylin.common.exception.code.ErrorCodeServer.SHARD_BY_COLUMN_NOT_IN_INDEX;
import java.util.ArrayList;
import java.util.Arrays;
@@ -235,6 +236,9 @@ public class IndexPlanService extends BasicService implements TableIndexPlanSupp
newLayout.setOwner(BasicService.getUsername());
newLayout.setManual(true);
newLayout.setIndexRange(request.getIndexRange());
+ if (!newLayout.getColOrder().containsAll(newLayout.getShardByColumns())) {
+ throw new KylinException(SHARD_BY_COLUMN_NOT_IN_INDEX);
+ }
Map<Integer, String> layoutOverride = Maps.newHashMap();
if (request.getLayoutOverrideIndexes() != null) {
diff --git a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/IndexPlanServiceTest.java b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/IndexPlanServiceTest.java
index 2a8db33ac3..c5f257e3fb 100644
--- a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/IndexPlanServiceTest.java
+++ b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/IndexPlanServiceTest.java
@@ -19,6 +19,7 @@ package org.apache.kylin.rest.service;
import static org.apache.kylin.common.exception.code.ErrorCodeServer.INDEX_DUPLICATE;
import static org.apache.kylin.common.exception.code.ErrorCodeServer.LAYOUT_NOT_EXISTS;
+import static org.apache.kylin.common.exception.code.ErrorCodeServer.SHARD_BY_COLUMN_NOT_IN_INDEX;
import static org.apache.kylin.metadata.cube.model.IndexEntity.Source.CUSTOM_TABLE_INDEX;
import static org.apache.kylin.metadata.cube.model.IndexEntity.Source.RECOMMENDED_TABLE_INDEX;
import static org.apache.kylin.metadata.model.SegmentStatusEnum.READY;
@@ -1447,4 +1448,93 @@ public class IndexPlanServiceTest extends SourceTestCase {
val response2 = indexPlanService.getShardByColumns("default", modelId);
Assert.assertFalse(response2.isShowLoadData());
}
+
+ @Test
+ public void testCheckShardByColumns() {
+ CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest.builder().project("default")
+ .modelId("89af4ee2-2cdb-4b07-b39e-4c29856309aa").id(20000010000L)
+ .colOrder(Lists.newArrayList("TEST_KYLIN_FACT.TRANS_ID", "TEST_SITES.SITE_NAME",
+ "TEST_KYLIN_FACT.CAL_DT"))
+ .shardByColumns(Lists.newArrayList("TEST_KYLIN_FACT.LSTG_SITE_ID")).sortByColumns(Lists.newArrayList())
+ .build();
+
+ Assert.assertThrows(SHARD_BY_COLUMN_NOT_IN_INDEX.getMsg(), KylinException.class, () -> {
+ indexPlanService.updateTableIndex("default", tableIndexRequest);
+ });
+ }
+
+ @Test
+ public void testUpdateTableIndexWithNullColOrder() {
+ String project = "default";
+ String modelId = "89af4ee2-2cdb-4b07-b39e-4c29856309aa";
+ long layoutId = 20000010001L;
+ CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest.builder().project(project).modelId(modelId)
+ .id(layoutId).shardByColumns(Lists.newArrayList()).sortByColumns(Lists.newArrayList()).build();
+ BuildIndexResponse response = indexPlanService.updateTableIndex(project, tableIndexRequest);
+ Assert.assertEquals(BuildIndexResponse.BuildIndexType.NORM_BUILD, response.getType());
+ LayoutEntity layoutEntity = NIndexPlanManager.getInstance(KylinConfig.getInstanceFromEnv(), project)
+ .getIndexPlan(modelId).getLayoutEntity(layoutId);
+ Assert.assertEquals(Lists.newArrayList(1, 0, 2), layoutEntity.getColOrder());
+ }
+
+ @Test
+ public void testUpdateTableIndexWithNullColOrderThrowsException() {
+ String project = "default";
+ String modelId = "89af4ee2-2cdb-4b07-b39e-4c29856309aa";
+ long layoutId = 20000010001L;
+ CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest.builder().project(project).modelId(modelId)
+ .id(layoutId).shardByColumns(Lists.newArrayList("TEST_KYLIN_FACT.TRANS_ID"))
+ .sortByColumns(Lists.newArrayList()).build();
+ Assert.assertThrows(SHARD_BY_COLUMN_NOT_IN_INDEX.getMsg(), KylinException.class, () -> {
+ indexPlanService.updateTableIndex("default", tableIndexRequest);
+ });
+ }
+
+ @Test
+ public void testUpdateTableIndexWithNullShardByCols() {
+ String project = "default";
+ String modelId = "89af4ee2-2cdb-4b07-b39e-4c29856309aa";
+ long layoutId = 20000010001L;
+ CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest
+ .builder().project(project).modelId(modelId).id(layoutId).colOrder(Lists
+ .newArrayList("TEST_KYLIN_FACT.TRANS_ID", "TEST_SITES.SITE_NAME", "TEST_KYLIN_FACT.CAL_DT"))
+ .sortByColumns(Lists.newArrayList()).build();
+ BuildIndexResponse response = indexPlanService.updateTableIndex(project, tableIndexRequest);
+ Assert.assertEquals(BuildIndexResponse.BuildIndexType.NORM_BUILD, response.getType());
+ LayoutEntity layoutEntity = NIndexPlanManager.getInstance(KylinConfig.getInstanceFromEnv(), project)
+ .getIndexPlan(modelId).getLayoutEntity(layoutId);
+ Assert.assertEquals(Lists.newArrayList(1, 0, 2), layoutEntity.getColOrder());
+ }
+
+ @Test
+ public void testUpdateTableIndexWithCreateEmptyIndex() {
+ String project = "default";
+ String modelId = "89af4ee2-2cdb-4b07-b39e-4c29856309aa";
+ long layoutId = 20000180001L;
+ CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest.builder().project(project).modelId(modelId)
+ .id(layoutId).colOrder(Lists.newArrayList()).sortByColumns(Lists.newArrayList()).build();
+ BuildIndexResponse response = indexPlanService.updateTableIndex(project, tableIndexRequest);
+ Assert.assertEquals(BuildIndexResponse.BuildIndexType.NORM_BUILD, response.getType());
+ IndexPlan indexPlan = NIndexPlanManager.getInstance(KylinConfig.getInstanceFromEnv(), project)
+ .getIndexPlan(modelId);
+ Assert.assertTrue(
+ indexPlan.getAllLayouts().stream().anyMatch(layoutEntity -> layoutEntity.getColOrder().size() == 0));
+ }
+
+ @Test
+ public void testUpdateTableIndexWithUpdateEmptyIndex() {
+ String project = "default";
+ String modelId = "89af4ee2-2cdb-4b07-b39e-4c29856309aa";
+ long layoutId = 20000010001L;
+ CreateTableIndexRequest tableIndexRequest = CreateTableIndexRequest.builder().project("default")
+ .modelId(modelId).id(layoutId).colOrder(Lists.newArrayList()).sortByColumns(Lists.newArrayList())
+ .build();
+ BuildIndexResponse response = indexPlanService.updateTableIndex("default", tableIndexRequest);
+ Assert.assertEquals(BuildIndexResponse.BuildIndexType.NORM_BUILD, response.getType());
+ IndexPlan indexPlan = NIndexPlanManager.getInstance(KylinConfig.getInstanceFromEnv(), project)
+ .getIndexPlan(modelId);
+ Assert.assertTrue(indexPlan.getLayoutEntity(layoutId).isToBeDeleted());
+ Assert.assertTrue(
+ indexPlan.getAllLayouts().stream().anyMatch(layoutEntity -> layoutEntity.getColOrder().size() == 0));
+ }
}