#1736 should check if computed column name duplicates with normal column name
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/3e777c0f Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/3e777c0f Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/3e777c0f Branch: refs/heads/2.1.x Commit: 3e777c0fd4d5d047467a69a1e48406b4808f5aa6 Parents: b962721 Author: Hongbin Ma <mahong...@apache.org> Authored: Mon Jul 31 16:38:13 2017 +0800 Committer: Hongbin Ma <mahong...@apache.org> Committed: Mon Jul 31 17:23:47 2017 +0800 ---------------------------------------------------------------------- .../metadata/model/ComputedColumnDesc.java | 4 ++++ .../apache/kylin/metadata/model/TableDesc.java | 23 +++++++++++++++----- .../kylin/rest/service/ModelServiceTest.java | 20 +++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/3e777c0f/core-metadata/src/main/java/org/apache/kylin/metadata/model/ComputedColumnDesc.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ComputedColumnDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ComputedColumnDesc.java index ab7e074..10a7dd3 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ComputedColumnDesc.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ComputedColumnDesc.java @@ -44,6 +44,10 @@ public class ComputedColumnDesc implements Serializable { Preconditions.checkNotNull(expression, "expression is null"); Preconditions.checkNotNull(datatype, "datatype is null"); + Preconditions.checkState(tableIdentity.equals(tableIdentity.trim()), "tableIdentity of ComputedColumnDesc has heading/tailing whitespace"); + Preconditions.checkState(columnName.equals(columnName.trim()), "columnName of ComputedColumnDesc has heading/tailing whitespace"); + Preconditions.checkState(datatype.equals(datatype.trim()), "datatype of ComputedColumnDesc has heading/tailing whitespace"); + tableIdentity = tableIdentity.toUpperCase(); columnName = columnName.toUpperCase(); http://git-wip-us.apache.org/repos/asf/kylin/blob/3e777c0f/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java index d3543dd..a2a2c24 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TableDesc.java @@ -63,18 +63,18 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { public TableDesc(TableDesc other) { this.uuid = other.uuid; this.lastModified = other.lastModified; - + this.name = other.name; this.sourceType = other.sourceType; this.tableType = other.tableType; this.dataGen = other.dataGen; - + this.columns = new ColumnDesc[other.columns.length]; for (int i = 0; i < other.columns.length; i++) { this.columns[i] = new ColumnDesc(other.columns[i]); this.columns[i].init(this); } - + this.database.setName(other.getDatabase()); this.identity = other.identity; } @@ -89,6 +89,15 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { ret.columns = new ColumnDesc[computedColumns.length + origin.length]; for (int i = 0; i < origin.length; i++) { ret.columns[i] = origin[i]; + + //check name conflict + for (int j = 0; j < computedColumns.length; j++) { + if (origin[i].getName().equalsIgnoreCase(computedColumns[j].getName())) { + throw new IllegalArgumentException(String.format( + "There is already a column named %s on table %s, please change your computed column name", + new Object[] { computedColumns[j].getName(), this.getIdentity() })); + } + } } for (int i = 0; i < computedColumns.length; i++) { computedColumns[i].init(ret); @@ -186,9 +195,9 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { if (columns == null) { return -1; } - + int max = -1; - + for (ColumnDesc col : columns) { int idx = col.getZeroBasedIndex(); max = Math.max(max, idx); @@ -270,7 +279,9 @@ public class TableDesc extends RootPersistentEntity implements ISourceAware { @Override public String toString() { - return "TableDesc{" + "name='" + name + '\'' + ", columns=" + Arrays.toString(columns) + ", sourceType=" + sourceType + ", tableType='" + tableType + '\'' + ", database=" + database + ", identity='" + getIdentity() + '\'' + '}'; + return "TableDesc{" + "name='" + name + '\'' + ", columns=" + Arrays.toString(columns) + ", sourceType=" + + sourceType + ", tableType='" + tableType + '\'' + ", database=" + database + ", identity='" + + getIdentity() + '\'' + '}'; } /** create a mockup table for unit test */ http://git-wip-us.apache.org/repos/asf/kylin/blob/3e777c0f/server/src/test/java/org/apache/kylin/rest/service/ModelServiceTest.java ---------------------------------------------------------------------- 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 756454a..78b1979 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 @@ -96,4 +96,24 @@ public class ModelServiceTest extends ServiceTestBase { field.set(deserialize.getComputedColumnDescs().get(0), "another expression"); DataModelDesc dataModelDesc = modelService.updateModelAndDesc(deserialize); } + + + @Test + public void testFailureModelUpdateDueToComputedColumnConflict2() throws IOException, JobException, NoSuchFieldException, IllegalAccessException { + expectedEx.expect(IllegalArgumentException.class); + expectedEx.expectMessage("There is already a column named cal_dt on table DEFAULT.TEST_KYLIN_FACT, please change your computed column name"); + + List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true); + Assert.assertTrue(dataModelDescs.size() == 1); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + MetadataManager.MODELDESC_SERIALIZER.serialize(dataModelDescs.get(0), new DataOutputStream(baos)); + ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); + DataModelDesc deserialize = MetadataManager.MODELDESC_SERIALIZER.deserialize(new DataInputStream(bais)); + + Field field = ComputedColumnDesc.class.getDeclaredField("columnName"); + field.setAccessible(true); + field.set(deserialize.getComputedColumnDescs().get(0), "cal_dt"); + DataModelDesc dataModelDesc = modelService.updateModelAndDesc(deserialize); + } }
