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 fb90074de24f60f7134c18da896e707e71d3241b
Author: Pengfei Zhan <dethr...@gmail.com>
AuthorDate: Thu Mar 2 15:03:34 2023 +0800
KYLIN-5523 [FOLLOW UP] Fix using a computed column as the join key leads to
a schema change failure
KYLIN-5523 [FOLLOW UP] expression of computed column change will be treated
as a significant change to the model
KYLIN-5523 [FOLLOW UP] Table reloading cannot be affected by a damaged join
key of computed column
---
.../java/org/apache/kylin/metadata/model/ColumnDesc.java | 12 +++++++++++-
.../test/java/org/apache/kylin/event/SchemaChangeTest.java | 5 +----
.../java/org/apache/kylin/rest/service/ModelService.java | 7 ++++++-
.../main/java/org/apache/kylin/query/schema/OLAPTable.java | 4 ++--
4 files changed, 20 insertions(+), 8 deletions(-)
diff --git
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java
index 49094bf6a3..71cf9fedc4 100644
---
a/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java
+++
b/src/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java
@@ -272,7 +272,17 @@ public class ColumnDesc implements Serializable {
} else if (!name.equals(other.name))
return false;
- return table == null ? other.table == null :
table.getIdentity().equals(other.table.getIdentity());
+ boolean isSameTable = table == null ? other.table == null
+ : table.getIdentity().equals(other.table.getIdentity());
+ if (!isSameTable) {
+ return false;
+ }
+ if (this.isComputedColumn() && other.isComputedColumn()) {
+ // add a simple check of computed column's inner expression
+ return StringUtils.equals(computedColumnExpr,
other.getComputedColumnExpr());
+ }
+
+ return !this.isComputedColumn() && !other.isComputedColumn();
}
@Override
diff --git
a/src/kylin-it/src/test/java/org/apache/kylin/event/SchemaChangeTest.java
b/src/kylin-it/src/test/java/org/apache/kylin/event/SchemaChangeTest.java
index 831bf2f6dd..00b58e7418 100644
--- a/src/kylin-it/src/test/java/org/apache/kylin/event/SchemaChangeTest.java
+++ b/src/kylin-it/src/test/java/org/apache/kylin/event/SchemaChangeTest.java
@@ -79,7 +79,6 @@ import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
-import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
@@ -94,7 +93,6 @@ import lombok.val;
import lombok.var;
import lombok.extern.slf4j.Slf4j;
-@Ignore("disable unstable test")
@Slf4j
public class SchemaChangeTest extends AbstractMVCIntegrationTestCase {
@@ -246,7 +244,6 @@ public class SchemaChangeTest extends
AbstractMVCIntegrationTestCase {
assertSqls();
}
- @Ignore("TODO: remove or adapt")
@Test
public void testChangeColumnType() throws Exception {
changeColumns(TABLE_IDENTITY, Sets.newHashSet("SRC_ID"), columnDesc ->
columnDesc.setDatatype("string"));
@@ -334,7 +331,7 @@ public class SchemaChangeTest extends
AbstractMVCIntegrationTestCase {
private void setupPushdownEnv() throws Exception {
getTestConfig().setProperty("kylin.query.pushdown.runner-class-name",
- "io.kyligence.kap.query.pushdown.PushDownRunnerJdbcImpl");
+ "org.apache.kylin.query.pushdown.PushDownRunnerJdbcImpl");
getTestConfig().setProperty("kylin.query.pushdown-enabled", "true");
// Load H2 Tables (inner join)
Connection h2Connection =
DriverManager.getConnection("jdbc:h2:mem:db_default;DB_CLOSE_DELAY=-1", "sa",
"");
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 21d52c7956..d0073de148 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
@@ -128,6 +128,7 @@ import org.apache.kylin.common.util.JsonUtil;
import org.apache.kylin.common.util.Pair;
import org.apache.kylin.common.util.RandomUtil;
import org.apache.kylin.common.util.StringHelper;
+import org.apache.kylin.common.util.ThreadUtil;
import org.apache.kylin.engine.spark.utils.ComputedColumnEvalUtil;
import org.apache.kylin.job.SecondStorageJobParamUtil;
import org.apache.kylin.job.common.SegmentUtil;
@@ -3264,7 +3265,11 @@ public class ModelService extends AbstractModelService
implements TableModelSupp
val modelManager = getManager(NDataModelManager.class, project);
val origin = modelManager.getDataModelDesc(modelRequest.getUuid());
val copyModel = modelManager.copyForWrite(origin);
- semanticUpdater.updateModelColumns(copyModel, modelRequest);
+ try {
+ semanticUpdater.updateModelColumns(copyModel, modelRequest);
+ } catch (Exception e) {
+ log.warn("Update existing model failed.{}",
ThreadUtil.getKylinStackTrace());
+ }
copyModel.setBrokenReason(NDataModel.BrokenReason.SCHEMA);
copyModel.getAllNamedColumns().forEach(namedColumn -> {
if (columnIds.contains(namedColumn.getId())) {
diff --git
a/src/query-common/src/main/java/org/apache/kylin/query/schema/OLAPTable.java
b/src/query-common/src/main/java/org/apache/kylin/query/schema/OLAPTable.java
index ff3ba69038..5b19ad4d4f 100644
---
a/src/query-common/src/main/java/org/apache/kylin/query/schema/OLAPTable.java
+++
b/src/query-common/src/main/java/org/apache/kylin/query/schema/OLAPTable.java
@@ -19,7 +19,7 @@
package org.apache.kylin.query.schema;
import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -249,7 +249,7 @@ public class OLAPTable extends AbstractQueryableTable
implements TranslatableTab
}
}
- Collections.sort(tableColumns, (o1, o2) -> o1.getZeroBasedIndex() -
o2.getZeroBasedIndex());
+
tableColumns.sort(Comparator.comparingInt(ColumnDesc::getZeroBasedIndex));
return Lists.newArrayList(Iterables.concat(tableColumns,
metricColumns));
}
