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 d39787c91cf70e8c6b7c137a8b607c46ea89d804
Author: fengguangyuan <qq272101...@gmail.com>
AuthorDate: Tue Jun 6 15:35:24 2023 +0800
KYLIN-5709 Support modifying column comment attribute
---
.../metadata/model/schema/ReloadTableContext.java | 6 ++-
.../apache/kylin/rest/service/TableService.java | 39 ++++++++++++-------
.../kylin/rest/service/TableReloadServiceTest.java | 45 ++++++++++++++++++++--
3 files changed, 70 insertions(+), 20 deletions(-)
diff --git
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/schema/ReloadTableContext.java
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/schema/ReloadTableContext.java
index 3774177bd4..035c373d6d 100644
---
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/schema/ReloadTableContext.java
+++
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/schema/ReloadTableContext.java
@@ -44,6 +44,8 @@ public class ReloadTableContext {
private Set<String> favoriteQueries = Sets.newHashSet();
+ private Set<String> changedColumns = Sets.newHashSet();
+
private Set<String> addColumns = Sets.newHashSet();
private Set<String> removeColumns = Sets.newHashSet();
@@ -97,11 +99,11 @@ public class ReloadTableContext {
}
public boolean isOnlyAddCols() {
- return removeColumns.isEmpty() && changeTypeColumns.isEmpty();
+ return removeColumns.isEmpty() && changedColumns.isEmpty();
}
public boolean isNeedProcess() {
return CollectionUtils.isNotEmpty(addColumns) ||
CollectionUtils.isNotEmpty(removeColumns)
- || CollectionUtils.isNotEmpty(changeTypeColumns) ||
isTableCommentChanged;
+ || CollectionUtils.isNotEmpty(changedColumns) ||
isTableCommentChanged;
}
}
diff --git
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
index d8f49d96c3..b6498f1a6f 100644
---
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
+++
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
@@ -1482,8 +1482,8 @@ public class TableService extends BasicService {
.collect(Collectors.toMap(ColumnDesc::getName,
Function.identity()));
val newColMap = Stream.of(context.getTableDesc().getColumns())
.collect(Collectors.toMap(ColumnDesc::getName,
Function.identity()));
- for (String changedTypeColumn : context.getChangeTypeColumns()) {
- originColMap.put(changedTypeColumn,
newColMap.get(changedTypeColumn));
+ for (String changedColumn : context.getChangedColumns()) {
+ originColMap.put(changedColumn, newColMap.get(changedColumn));
}
for (String addColumn : context.getAddColumns()) {
originColMap.put(addColumn, newColMap.get(addColumn));
@@ -1600,15 +1600,23 @@ public class TableService extends BasicService {
handleExcludedColumns(project, context, newTableDesc, tableIdentity);
TableDesc originTableDesc = getManager(NTableMetadataManager.class,
project).getTableDesc(tableIdentity);
- val collector = Collectors.toMap(ColumnDesc::getName, col ->
Pair.newPair(col.getName(), col.getDatatype()));
- val originCols =
Stream.of(originTableDesc.getColumns()).collect(collector);
- val newCols = Stream.of(newTableDesc.getColumns()).collect(collector);
- val diff = Maps.difference(newCols, originCols);
- context.setAddColumns(diff.entriesOnlyOnLeft().keySet());
- context.setRemoveColumns(diff.entriesOnlyOnRight().keySet());
- context.setChangeTypeColumns(diff.entriesDiffering().keySet());
+ val collector = Collectors.toMap(ColumnDesc::getName, col ->
Pair.newPair(col.getDatatype(), col.getComment()));
+ val diff =
Maps.difference(Stream.of(originTableDesc.getColumns()).collect(collector),
+ Stream.of(newTableDesc.getColumns()).collect(collector));
+ val dataTypeCollector = Collectors.toMap(ColumnDesc::getName,
ColumnDesc::getDatatype);
+ val originCols =
Stream.of(originTableDesc.getColumns()).collect(dataTypeCollector);
+ val newCols =
Stream.of(newTableDesc.getColumns()).collect(dataTypeCollector);
+ val dataTypeDiff = Maps.difference(newCols, originCols);
+
+ assert
diff.entriesDiffering().keySet().containsAll(dataTypeDiff.entriesDiffering().keySet());
+
+ context.setAddColumns(dataTypeDiff.entriesOnlyOnLeft().keySet());
+ context.setRemoveColumns(dataTypeDiff.entriesOnlyOnRight().keySet());
+ context.setChangedColumns(diff.entriesDiffering().keySet());
+ context.setChangeTypeColumns(dataTypeDiff.entriesDiffering().keySet());
context.setTableCommentChanged(!Objects.equals(originTableDesc.getTableComment(),
newTableDesc.getTableComment()));
+
if (!context.isNeedProcess()) {
return context;
}
@@ -1633,7 +1641,7 @@ public class TableService extends BasicService {
val dependencyGraph = SchemaUtil.dependencyGraph(project,
tableIdentity);
Map<String, Set<Pair<NDataModel.Measure, NDataModel.Measure>>>
suitableColumnTypeChangedMeasuresMap = getSuitableColumnTypeChangedMeasures(
- dependencyGraph, project, originTableDesc,
diff.entriesDiffering());
+ dependencyGraph, project, originTableDesc,
dataTypeDiff.entriesDiffering());
BiFunction<Set<String>, Boolean, Map<String, AffectedModelContext>>
toAffectedModels = (cols, isDelete) -> {
Set<SchemaNode> affectedNodes = Sets.newHashSet();
@@ -1712,16 +1720,19 @@ public class TableService extends BasicService {
*/
private Map<String, Set<Pair<NDataModel.Measure, NDataModel.Measure>>>
getSuitableColumnTypeChangedMeasures(
Graph<SchemaNode> dependencyGraph, String project, TableDesc
tableDesc,
- Map<String, MapDifference.ValueDifference<Pair<String, String>>>
changeTypeDifference) {
+ Map<String, MapDifference.ValueDifference<String>>
changeTypeDifference) {
Map<String, Set<Pair<NDataModel.Measure, NDataModel.Measure>>> result
= Maps.newHashMap();
NDataModelManager dataModelManager =
NDataModelManager.getInstance(KylinConfig.getInstanceFromEnv(), project);
val columnMap = Arrays.stream(tableDesc.getColumns())
.collect(Collectors.toMap(ColumnDesc::getName,
Function.identity()));
- for (MapDifference.ValueDifference<Pair<String, String>> value :
changeTypeDifference.values()) {
+ for (val value : changeTypeDifference.entrySet()) {
+ val colName = value.getKey();
+ val colDiff = value.getValue();
+
Graphs.reachableNodes(dependencyGraph,
-
SchemaNode.ofTableColumn(columnMap.get(value.leftValue().getFirst()))).stream()
+ SchemaNode.ofTableColumn(columnMap.get(colName))).stream()
.filter(node -> node.getType() ==
SchemaNodeType.MODEL_MEASURE).forEach(node -> {
String modelAlias = node.getSubject();
String measureId = node.getDetail();
@@ -1735,7 +1746,7 @@ public class TableService extends BasicService {
FunctionDesc originalFunction =
measure.getFunction();
- String newColumnType =
value.leftValue().getSecond();
+ String newColumnType = colDiff.leftValue();
boolean datatypeSuitable = originalFunction
.isDatatypeSuitable(DataType.getType(newColumnType));
diff --git
a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/TableReloadServiceTest.java
b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/TableReloadServiceTest.java
index 48f8f5708c..f18ee8db22 100644
---
a/src/modeling-service/src/test/java/org/apache/kylin/rest/service/TableReloadServiceTest.java
+++
b/src/modeling-service/src/test/java/org/apache/kylin/rest/service/TableReloadServiceTest.java
@@ -26,6 +26,7 @@ import java.io.IOException;
import java.nio.charset.StandardCharsets;
//import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -46,6 +47,9 @@ import
org.apache.kylin.common.persistence.transaction.UnitOfWork;
import org.apache.kylin.common.scheduler.EventBusFactory;
import org.apache.kylin.common.util.JsonUtil;
import org.apache.kylin.cube.model.SelectRule;
+import org.apache.kylin.guava30.shaded.common.base.Joiner;
+import org.apache.kylin.guava30.shaded.common.collect.Lists;
+import org.apache.kylin.guava30.shaded.common.collect.Sets;
import org.apache.kylin.engine.spark.job.NSparkCubingJob;
import org.apache.kylin.engine.spark.job.NTableSamplingJob;
import org.apache.kylin.job.execution.AbstractExecutable;
@@ -95,10 +99,6 @@ import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.test.util.ReflectionTestUtils;
-import org.apache.kylin.guava30.shaded.common.base.Joiner;
-import org.apache.kylin.guava30.shaded.common.collect.Lists;
-import org.apache.kylin.guava30.shaded.common.collect.Sets;
-
//import io.kyligence.kap.clickhouse.MockSecondStorage;
import io.kyligence.kap.secondstorage.SecondStorageUtil;
import lombok.val;
@@ -1206,6 +1206,35 @@ public class TableReloadServiceTest extends
CSVSourceTestCase {
() -> tableService.preProcessBeforeReloadWithFailFast(PROJECT,
tableIdentity));
}
+ @Test
+ public void testReloadColumnCommentChanged() throws Exception {
+ val tableManager = NTableMetadataManager.getInstance(getTestConfig(),
PROJECT);
+ val tableName = "DEFAULT.TEST_COUNTRY";
+ val dimColName = "COUNTRY";
+ val measureColName = "LATITUDE";
+
+ prepareTableExt(tableName);
+ changeTypeColumn(tableName, Collections.emptyMap(), new
HashMap<String, String>() {
+ {
+ put("COUNTRY", "a new comment for dimension column");
+ put("LATITUDE", "a new comment for measure column");
+ }
+ }, true);
+ tableService.innerReloadTable(PROJECT, tableName, true, null);
+
+ val newTableDesc = tableManager.getTableDesc(tableName);
+ Assert.assertEquals("a new comment for dimension column",
+ findColumn(newTableDesc.getColumns(),
dimColName).get().getComment());
+ Assert.assertEquals("a new comment for measure column",
+ findColumn(newTableDesc.getColumns(),
measureColName).get().getComment());
+ }
+
+ private Optional<ColumnDesc> findColumn(ColumnDesc[] columns, String name)
{
+ return Stream.of(columns)
+ .filter(col -> col.getName().equalsIgnoreCase(name))
+ .findFirst();
+ }
+
@Test
public void testReloadAddTableComment() throws Exception {
val tableManager = NTableMetadataManager.getInstance(getTestConfig(),
PROJECT);
@@ -1889,6 +1918,11 @@ public class TableReloadServiceTest extends
CSVSourceTestCase {
private void changeTypeColumn(String tableIdentity, Map<String, String>
columns, boolean useMeta)
throws IOException {
+ changeTypeColumn(tableIdentity, columns, Collections.emptyMap(),
useMeta);
+ }
+
+ private void changeTypeColumn(String tableIdentity, Map<String, String>
columns, Map<String, String> comments, boolean useMeta)
+ throws IOException {
val tableManager = NTableMetadataManager.getInstance(getTestConfig(),
PROJECT);
val factTable = tableManager.getTableDesc(tableIdentity);
String resPath =
KylinConfig.getInstanceFromEnv().getMetadataUrl().getIdentifier();
@@ -1899,6 +1933,9 @@ public class TableReloadServiceTest extends
CSVSourceTestCase {
if (columns.containsKey(col.getName())) {
col.setDatatype(columns.get(col.getName()));
}
+ if (comments.containsKey(col.getName())) {
+ col.setComment(comments.get(col.getName()));
+ }
}).toArray(ColumnDesc[]::new);
tableMeta.setColumns(newColumns);
JsonUtil.writeValueIndent(new FileOutputStream(new File(tablePath)),
tableMeta);
