minor, enhance computed column checks
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/d61f867f Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/d61f867f Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/d61f867f Branch: refs/heads/master Commit: d61f867ff9fa170e155310edeb03db6059433f2f Parents: 69532cd Author: Hongbin Ma <mahong...@apache.org> Authored: Wed Jun 28 21:13:25 2017 +0800 Committer: Hongbin Ma <m...@kyligence.io> Committed: Thu Jun 29 13:51:45 2017 +0800 ---------------------------------------------------------------------- core-metadata/pom.xml | 16 +- .../apache/kylin/metadata/model/ColumnDesc.java | 15 +- .../metadata/model/ComputedColumnDesc.java | 3 + .../kylin/metadata/model/DataModelDesc.java | 30 ++- .../apache/kylin/metadata/model/TblColRef.java | 2 +- .../metadata/model/tool/CalciteParser.java | 203 +++++++++++++++++++ .../kylin/model/tool/CalciteParserTest.java | 111 ++++++++++ .../model_desc/ci_inner_join_model.json | 6 +- .../model_desc/ci_left_join_model.json | 6 +- .../query/util/ConvertToComputedColumn.java | 111 ++-------- .../query/util/ConvertToComputedColumnTest.java | 22 -- 11 files changed, 379 insertions(+), 146 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/core-metadata/pom.xml ---------------------------------------------------------------------- diff --git a/core-metadata/pom.xml b/core-metadata/pom.xml index f5ab12a..d45a838 100644 --- a/core-metadata/pom.xml +++ b/core-metadata/pom.xml @@ -17,7 +17,8 @@ limitations under the License. --> -<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> <modelVersion>4.0.0</modelVersion> @@ -60,6 +61,19 @@ <groupId>com.google.code.findbugs</groupId> <artifactId>jsr305</artifactId> </dependency> + <dependency> + <groupId>org.apache.kylin</groupId> + <artifactId>atopcalcite</artifactId> + <exclusions> + <exclusion> + <groupId>org.apache.calcite.avatica</groupId> + <artifactId>avatica-core</artifactId> + </exclusion> + </exclusions> + <!--set to provided on purpose, so that dependant on metadata module will not depend on calcite unnecessarily--> + <!--https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html--> + <scope>provided</scope> + </dependency> <!-- Compiled --> <dependency> http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java index 5d15d56..3c84c96 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ColumnDesc.java @@ -19,9 +19,9 @@ package org.apache.kylin.metadata.model; import java.io.Serializable; -import java.util.regex.Pattern; import org.apache.kylin.metadata.datatype.DataType; +import org.apache.kylin.metadata.model.tool.CalciteParser; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; @@ -82,7 +82,8 @@ public class ColumnDesc implements Serializable { this.index = other.index; } - public ColumnDesc(String id, String name, String datatype, String comment, String dataGen, String index, String computedColumnExpr) { + public ColumnDesc(String id, String name, String datatype, String comment, String dataGen, String index, + String computedColumnExpr) { this.id = id; this.name = name; this.datatype = datatype; @@ -193,11 +194,12 @@ public class ColumnDesc implements Serializable { return index; } - public String getComputedColumnExpr(String tableAlias, String tableIdentity) { + public String getComputedColumnExpr(String tableAlias) { Preconditions.checkState(computedColumnExpr != null); + Preconditions.checkState(tableAlias != null); - //http://stackoverflow.com/questions/5054995/how-to-replace-case-insensitive-literal-substrings-in-java - return computedColumnExpr.replaceAll("(?i)" + Pattern.quote(tableIdentity), tableAlias); + String s = CalciteParser.insertAliasInExpr(computedColumnExpr, tableAlias); + return s; } public boolean isComputedColumnn() { @@ -257,6 +259,7 @@ public class ColumnDesc implements Serializable { @Override public String toString() { - return "ColumnDesc{" + "id='" + id + '\'' + ", name='" + name + '\'' + ", datatype='" + datatype + '\'' + ", comment='" + comment + '\'' + '}'; + return "ColumnDesc{" + "id='" + id + '\'' + ", name='" + name + '\'' + ", datatype='" + datatype + '\'' + + ", comment='" + comment + '\'' + '}'; } } http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/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 013cb8c..540b5fc 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 @@ -20,6 +20,7 @@ package org.apache.kylin.metadata.model; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import org.apache.kylin.metadata.model.tool.CalciteParser; @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE, getterVisibility = JsonAutoDetect.Visibility.NONE, isGetterVisibility = JsonAutoDetect.Visibility.NONE, setterVisibility = JsonAutoDetect.Visibility.NONE) public class ComputedColumnDesc { @@ -42,6 +43,8 @@ public class ComputedColumnDesc { tableIdentity = tableIdentity.toUpperCase(); columnName = columnName.toUpperCase(); + + CalciteParser.ensureNoTableNameExists(expression); } public String getFullName() { http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/core-metadata/src/main/java/org/apache/kylin/metadata/model/DataModelDesc.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/DataModelDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/DataModelDesc.java index 7de955e..91802f7 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/DataModelDesc.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/DataModelDesc.java @@ -489,17 +489,11 @@ public class DataModelDesc extends RootPersistentEntity { } CCInfo other = ccInfoMap.get(thisCCName); - if (other == null || (other.getDataModelDescs().size() == 1 && other.getDataModelDescs().contains(this))) { - //check whether two computer columns's definition is the same. - for (CCInfo sysCCInfo : ccInfoMap.values()) { - String definition0 = thisCCDesc.getExpression(); - String definition1 = sysCCInfo.getComputedColumnDesc().getExpression(); - if (isTwoCCDefinitionEquals(definition0, definition1)) { - throw new IllegalStateException(String.format( - "Computed column %s'definition: %s is already defined in other models: %s. Please change another definition, or try to keep consistent definition", - thisCCName, definition0, sysCCInfo.getDataModelDescs())); - } - } + + if (other == null) { + checkSameCCDefinition(ccInfoMap, thisCCDesc, thisCCName); + ccInfoMap.put(thisCCName, new CCInfo(thisCCDesc, Sets.<DataModelDesc> newHashSet(this))); + } else if (other.getDataModelDescs().size() == 1 && other.getDataModelDescs().contains(this)) { ccInfoMap.put(thisCCName, new CCInfo(thisCCDesc, Sets.<DataModelDesc> newHashSet(this))); } else if (other.getComputedColumnDesc().equals(thisCCDesc)) { other.getDataModelDescs().add(this); @@ -511,6 +505,20 @@ public class DataModelDesc extends RootPersistentEntity { } } + private void checkSameCCDefinition(Map<String, CCInfo> ccInfoMap, ComputedColumnDesc thisCCDesc, + String thisCCName) { + //check whether two computer columns's definition is the same. + for (CCInfo sysCCInfo : ccInfoMap.values()) { + String definition0 = thisCCDesc.getExpression(); + String definition1 = sysCCInfo.getComputedColumnDesc().getExpression(); + if (isTwoCCDefinitionEquals(definition0, definition1)) { + throw new IllegalStateException(String.format( + "Computed column %s's definition: %s is already defined in other models: %s. Please change another definition, or try to keep consistent definition", + thisCCName, definition0, sysCCInfo.getDataModelDescs())); + } + } + } + private boolean isTwoCCDefinitionEquals(String definition0, String definition1) { definition0 = definition0.replaceAll("\\s*", ""); definition1 = definition1.replaceAll("\\s*", ""); http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java index 5a28e8b..5563856 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java @@ -158,7 +158,7 @@ public class TblColRef implements Serializable { if (!column.isComputedColumnn()) { return getIdentity(); } else { - return column.getComputedColumnExpr(getTableAlias(), table.getTableIdentity()); + return column.getComputedColumnExpr(getTableAlias()); } } http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/core-metadata/src/main/java/org/apache/kylin/metadata/model/tool/CalciteParser.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/tool/CalciteParser.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/tool/CalciteParser.java new file mode 100644 index 0000000..47a90bf --- /dev/null +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/tool/CalciteParser.java @@ -0,0 +1,203 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +package org.apache.kylin.metadata.model.tool; + +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; + +import org.apache.calcite.sql.SqlCall; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlLiteral; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.parser.SqlParseException; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.util.SqlBasicVisitor; +import org.apache.calcite.sql.util.SqlVisitor; +import org.apache.commons.lang3.tuple.Pair; + +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; + +public class CalciteParser { + public static SqlNode parse(String sql) throws SqlParseException { + SqlParser.ConfigBuilder parserBuilder = SqlParser.configBuilder(); + SqlParser sqlParser = SqlParser.create(sql, parserBuilder.build()); + return sqlParser.parseQuery(); + } + + public static SqlNode getOnlySelectNode(String sql) { + SqlNodeList selectList = null; + try { + selectList = ((SqlSelect) CalciteParser.parse(sql)).getSelectList(); + } catch (SqlParseException e) { + throw new RuntimeException("Failed to parse expression \'" + sql + + "\', please make sure the expression is valid and no table name exists in the expression"); + } + + Preconditions.checkArgument(selectList.size() == 1, + "Expression is invalid because size of select list exceeds one"); + + return selectList.get(0); + } + + public static SqlNode getExpNode(String expr) { + return getOnlySelectNode("select " + expr + " from t"); + + } + + public static boolean isNodeEqual(SqlNode node0, SqlNode node1) { + if (node0 == null) { + return node1 == null; + } else if (node1 == null) { + return false; + } + + if (!Objects.equals(node0.getClass().getSimpleName(), node1.getClass().getSimpleName())) { + return false; + } + + if (node0 instanceof SqlCall) { + SqlCall thisNode = (SqlCall) node0; + SqlCall thatNode = (SqlCall) node1; + if (!thisNode.getOperator().getName().equalsIgnoreCase(thatNode.getOperator().getName())) { + return false; + } + return isNodeEqual(thisNode.getOperandList(), thatNode.getOperandList()); + } + if (node0 instanceof SqlLiteral) { + SqlLiteral thisNode = (SqlLiteral) node0; + SqlLiteral thatNode = (SqlLiteral) node1; + return Objects.equals(thisNode.getValue(), thatNode.getValue()); + } + if (node0 instanceof SqlNodeList) { + SqlNodeList thisNode = (SqlNodeList) node0; + SqlNodeList thatNode = (SqlNodeList) node1; + if (thisNode.getList().size() != thatNode.getList().size()) { + return false; + } + for (int i = 0; i < thisNode.getList().size(); i++) { + SqlNode thisChild = thisNode.getList().get(i); + final SqlNode thatChild = thatNode.getList().get(i); + if (!isNodeEqual(thisChild, thatChild)) { + return false; + } + } + return true; + } + if (node0 instanceof SqlIdentifier) { + SqlIdentifier thisNode = (SqlIdentifier) node0; + SqlIdentifier thatNode = (SqlIdentifier) node1; + // compare ignore table alias.eg: expression like "a.b + a.c + a.d" ,alias a will be ignored when compared + String name0 = thisNode.names.get(thisNode.names.size() - 1).replace("\"", ""); + String name1 = thatNode.names.get(thatNode.names.size() - 1).replace("\"", ""); + return name0.equalsIgnoreCase(name1); + } + + return false; + } + + private static boolean isNodeEqual(List<SqlNode> operands0, List<SqlNode> operands1) { + if (operands0.size() != operands1.size()) { + return false; + } + for (int i = 0; i < operands0.size(); i++) { + if (!isNodeEqual(operands0.get(i), operands1.get(i))) { + return false; + } + } + return true; + } + + public static void ensureNoTableNameExists(String expr) { + SqlNode sqlNode = getExpNode(expr); + + SqlVisitor sqlVisitor = new SqlBasicVisitor() { + @Override + public Object visit(SqlIdentifier id) { + if (id.names.size() > 1) { + throw new IllegalArgumentException("SqlIdentifier " + id + " contains DB/Table name"); + } + return null; + } + }; + + sqlNode.accept(sqlVisitor); + } + + public static String insertAliasInExpr(String expr, String alias) { + String prefix = "select "; + String suffix = " from t"; + String sql = prefix + expr + suffix; + SqlNode sqlNode = getOnlySelectNode(sql); + + final List<SqlIdentifier> sqlIdentifiers = Lists.newArrayList(); + SqlVisitor sqlVisitor = new SqlBasicVisitor() { + @Override + public Object visit(SqlIdentifier id) { + if (id.names.size() > 1) { + throw new IllegalArgumentException("SqlIdentifier " + id + " contains DB/Table name"); + } + sqlIdentifiers.add(id); + return null; + } + }; + + sqlNode.accept(sqlVisitor); + + Collections.sort(sqlIdentifiers, new Comparator<SqlIdentifier>() { + @Override + public int compare(SqlIdentifier o1, SqlIdentifier o2) { + int linegap = o2.getParserPosition().getLineNum() - o1.getParserPosition().getLineNum(); + if (linegap != 0) + return linegap; + + return o2.getParserPosition().getColumnNum() - o1.getParserPosition().getColumnNum(); + } + }); + + for (SqlIdentifier sqlIdentifier : sqlIdentifiers) { + Pair<Integer, Integer> replacePos = getReplacePos(sqlIdentifier, sql); + int start = replacePos.getLeft(); + sql = sql.substring(0, start) + alias + "." + sql.substring(start); + } + + return sql.substring(prefix.length(), sql.length() - suffix.length()); + } + + public static Pair<Integer, Integer> getReplacePos(SqlNode node, String... lines) { + SqlParserPos pos = node.getParserPosition(); + int lineStart = pos.getLineNum(); + int lineEnd = pos.getEndLineNum(); + int columnStart = pos.getColumnNum() - 1; + int columnEnd = pos.getEndColumnNum(); + //for the case that sql is multi lines + for (int i = 0; i < lineStart - 1; i++) { + columnStart += lines[i].length() + 1; + } + for (int i = 0; i < lineEnd - 1; i++) { + columnEnd += lines[i].length() + 1; + } + return Pair.of(columnStart, columnEnd); + } +} http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/core-metadata/src/test/java/org/apache/kylin/model/tool/CalciteParserTest.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/test/java/org/apache/kylin/model/tool/CalciteParserTest.java b/core-metadata/src/test/java/org/apache/kylin/model/tool/CalciteParserTest.java new file mode 100644 index 0000000..acf7b5a --- /dev/null +++ b/core-metadata/src/test/java/org/apache/kylin/model/tool/CalciteParserTest.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +package org.apache.kylin.model.tool; + +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.parser.SqlParseException; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.kylin.metadata.model.tool.CalciteParser; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import com.google.common.base.Preconditions; + +public class CalciteParserTest { + + @Rule + public final ExpectedException exception = ExpectedException.none(); + + @Test + public void testNoTableNameExists() throws SqlParseException { + String expr1 = "a + b"; + Assert.assertEquals("x.a + x.b", CalciteParser.insertAliasInExpr(expr1, "x")); + + String expr2 = "a + year(b)"; + Assert.assertEquals("x.a + year(x.b)", CalciteParser.insertAliasInExpr(expr2, "x")); + + String expr3 = "a + hiveudf(b)"; + Assert.assertEquals("x.a + hiveudf(x.b)", CalciteParser.insertAliasInExpr(expr3, "x")); + } + + @Test + public void testTableNameExists1() throws SqlParseException { + String expr1 = "a + x.b"; + + exception.expect(IllegalArgumentException.class); + exception.expectMessage("SqlIdentifier X.B contains DB/Table name"); + CalciteParser.insertAliasInExpr(expr1, "x"); + } + + @Test + public void testTableNameExists2() throws SqlParseException { + String expr1 = "a + year(x.b)"; + + exception.expect(IllegalArgumentException.class); + exception.expectMessage("SqlIdentifier X.B contains DB/Table name"); + CalciteParser.insertAliasInExpr(expr1, "x"); + } + + @Test + public void testTableNameExists3() throws SqlParseException { + String expr1 = "a + hiveudf(x.b)"; + + exception.expect(IllegalArgumentException.class); + exception.expectMessage("SqlIdentifier X.B contains DB/Table name"); + CalciteParser.insertAliasInExpr(expr1, "x"); + } + + @Test + public void testPos() throws SqlParseException { + String[] sqls = new String[] { "select \n a \n + \n b \n from t", // + "select\na\n+\nb\nfrom t", // + "select \r\n a \r\n + \r\n b \r\n from t", // + "select\r\na\r\n+\r\nb\r\nfrom t" }; + + for (String sql : sqls) { + SqlNode parse = ((SqlSelect) CalciteParser.parse(sql)).getSelectList().get(0); + String[] lines = sql.split("\n"); + Pair<Integer, Integer> replacePos = CalciteParser.getReplacePos(parse, lines); + String substring = sql.substring(replacePos.getLeft(), replacePos.getRight()); + Preconditions.checkArgument(substring.startsWith("a")); + Preconditions.checkArgument(substring.endsWith("b")); + } + + } + + @Test + public void testEqual() throws SqlParseException { + String sql0 = "select a.a + a.b + a.c from t as a"; + String sql1 = "select (((a . a + a.b + a.c))) from t as a"; + String sql2 = "select (a + b) + c from t"; + String sql3 = "select a.a + (a.b + a.c) from t as a"; + + SqlNode sn0 = CalciteParser.getOnlySelectNode(sql0); + SqlNode sn1 = CalciteParser.getOnlySelectNode(sql1); + SqlNode sn2 = CalciteParser.getOnlySelectNode(sql2); + SqlNode sn3 = CalciteParser.getOnlySelectNode(sql3); + + Assert.assertEquals(true, CalciteParser.isNodeEqual(sn0, sn1)); + Assert.assertEquals(true, CalciteParser.isNodeEqual(sn0, sn2)); + Assert.assertEquals(false, CalciteParser.isNodeEqual(sn0, sn3)); + } +} http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/examples/test_case_data/localmeta/model_desc/ci_inner_join_model.json ---------------------------------------------------------------------- diff --git a/examples/test_case_data/localmeta/model_desc/ci_inner_join_model.json b/examples/test_case_data/localmeta/model_desc/ci_inner_join_model.json index d084ce2..5225cf3 100644 --- a/examples/test_case_data/localmeta/model_desc/ci_inner_join_model.json +++ b/examples/test_case_data/localmeta/model_desc/ci_inner_join_model.json @@ -125,21 +125,21 @@ { "tableIdentity": "DEFAULT.TEST_KYLIN_FACT", "columnName": "DEAL_AMOUNT", - "expression": "DEFAULT.TEST_KYLIN_FACT.PRICE * DEFAULT.TEST_KYLIN_FACT.ITEM_COUNT", + "expression": "PRICE * ITEM_COUNT", "datatype": "decimal", "comment": "deal amount of inner join model" }, { "tableIdentity": "DEFAULT.TEST_KYLIN_FACT", "columnName": "DEAL_YEAR", - "expression": "year(DEFAULT.TEST_KYLIN_FACT.CAL_DT)", + "expression": "year(CAL_DT)", "datatype": "integer", "comment": "the year of the deal of the inner join model" }, { "tableIdentity": "DEFAULT.TEST_ACCOUNT", "columnName": "COUNTRY_ABBR", - "expression": "SUBSTR(DEFAULT.TEST_ACCOUNT.ACCOUNT_COUNTRY,0,1)", + "expression": "SUBSTR(ACCOUNT_COUNTRY,0,1)", "datatype": "string", "comment": "first char of country of the inner join model" } http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/examples/test_case_data/localmeta/model_desc/ci_left_join_model.json ---------------------------------------------------------------------- diff --git a/examples/test_case_data/localmeta/model_desc/ci_left_join_model.json b/examples/test_case_data/localmeta/model_desc/ci_left_join_model.json index 4a10899..0058425 100644 --- a/examples/test_case_data/localmeta/model_desc/ci_left_join_model.json +++ b/examples/test_case_data/localmeta/model_desc/ci_left_join_model.json @@ -125,21 +125,21 @@ { "tableIdentity": "DEFAULT.TEST_KYLIN_FACT", "columnName": "DEAL_AMOUNT", - "expression": "DEFAULT.TEST_KYLIN_FACT.PRICE * DEFAULT.TEST_KYLIN_FACT.ITEM_COUNT", + "expression": "PRICE * ITEM_COUNT", "datatype": "decimal", "comment": "deal amount of left join model" }, { "tableIdentity": "DEFAULT.TEST_KYLIN_FACT", "columnName": "DEAL_YEAR", - "expression": "year(DEFAULT.TEST_KYLIN_FACT.CAL_DT)", + "expression": "year(CAL_DT)", "datatype": "integer", "comment": "the year of the deal of the left join model" }, { "tableIdentity": "DEFAULT.TEST_ACCOUNT", "columnName": "COUNTRY_ABBR", - "expression": "SUBSTR(DEFAULT.TEST_ACCOUNT.ACCOUNT_COUNTRY,0,1)", + "expression": "SUBSTR(ACCOUNT_COUNTRY,0,1)", "datatype": "string", "comment": "first char of country of the left join model" } http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/query/src/main/java/org/apache/kylin/query/util/ConvertToComputedColumn.java ---------------------------------------------------------------------- diff --git a/query/src/main/java/org/apache/kylin/query/util/ConvertToComputedColumn.java b/query/src/main/java/org/apache/kylin/query/util/ConvertToComputedColumn.java index d8f1220..7f188c1 100644 --- a/query/src/main/java/org/apache/kylin/query/util/ConvertToComputedColumn.java +++ b/query/src/main/java/org/apache/kylin/query/util/ConvertToComputedColumn.java @@ -23,7 +23,6 @@ import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import javax.annotation.Nullable; @@ -35,15 +34,14 @@ import org.apache.calcite.sql.SqlIntervalQualifier; import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; -import org.apache.calcite.sql.SqlSelect; import org.apache.calcite.sql.parser.SqlParseException; -import org.apache.calcite.sql.parser.SqlParser; -import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.util.SqlVisitor; +import org.apache.commons.lang.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.kylin.common.KylinConfig; import org.apache.kylin.metadata.MetadataManager; import org.apache.kylin.metadata.model.DataModelDesc; +import org.apache.kylin.metadata.model.tool.CalciteParser; import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.metadata.project.ProjectManager; import org.slf4j.Logger; @@ -65,7 +63,11 @@ public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { return sql; } ImmutableSortedMap<String, String> computedColumns = getSortedComputedColumnWithProject(project); - return replaceComputedColumn(sql, computedColumns); + String s = replaceComputedColumn(sql, computedColumns); + if (!StringUtils.equals(sql, s)) { + logger.debug("change sql to " + s); + } + return s; } static String replaceComputedColumn(String inputSql, ImmutableSortedMap<String, String> computedColumn) { @@ -88,7 +90,7 @@ public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { logger.error("Convert to computedColumn Fail,parse sql fail ", e.getMessage()); } for (SqlNode node : matchedNodes) { - Pair<Integer, Integer> startEndPos = getReplacePos(lines, node); + Pair<Integer, Integer> startEndPos = CalciteParser.getReplacePos(node, lines); int start = startEndPos.getLeft(); int end = startEndPos.getRight(); //add table alias like t1.column,if exists alias @@ -104,32 +106,18 @@ public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { return result; } - private static Pair<Integer, Integer> getReplacePos(String[] lines, SqlNode node) { - SqlParserPos pos = node.getParserPosition(); - int lineStart = pos.getLineNum(); - int columnStart = pos.getColumnNum() - 1; - int columnEnd = pos.getEndColumnNum(); - //for the case that sql is multi lines - for (int i = 0; i < lineStart - 1; i++) { - int offset = lines[i].length(); - columnStart += offset + 1; - columnEnd += offset + 1; - } - return Pair.of(columnStart, columnEnd); - } - //Return matched node's position and its alias(if exists).If can not find matches, return an empty capacity list private static List<SqlNode> getMatchedNodes(String inputSql, String ccExp) throws SqlParseException { if (ccExp == null || ccExp.equals("")) { return new ArrayList<>(); } ArrayList<SqlNode> toBeReplacedNodes = new ArrayList<>(); - SqlNode ccNode = getCCExpNode(ccExp); + SqlNode ccNode = CalciteParser.getExpNode(ccExp); List<SqlNode> inputNodes = getInputTreeNodes(inputSql); // find whether user input sql's tree node equals computed columns's define expression for (SqlNode inputNode : inputNodes) { - if (isNodeEqual(inputNode, ccNode)) { + if (CalciteParser.isNodeEqual(inputNode, ccNode)) { toBeReplacedNodes.add(inputNode); } } @@ -138,85 +126,10 @@ public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { private static List<SqlNode> getInputTreeNodes(String inputSql) throws SqlParseException { SqlTreeVisitor stv = new SqlTreeVisitor(); - parse(inputSql).accept(stv); + CalciteParser.parse(inputSql).accept(stv); return stv.getSqlNodes(); } - private static SqlNode getCCExpNode(String ccExp) throws SqlParseException { - ccExp = "select " + ccExp + " from t"; - return ((SqlSelect) parse(ccExp)).getSelectList().get(0); - } - - static SqlNode parse(String sql) throws SqlParseException { - SqlParser.ConfigBuilder parserBuilder = SqlParser.configBuilder(); - SqlParser sqlParser = SqlParser.create(sql, parserBuilder.build()); - return sqlParser.parseQuery(); - } - - static boolean isNodeEqual(SqlNode node0, SqlNode node1) { - if (node0 == null) { - return node1 == null; - } else if (node1 == null) { - return false; - } - - if (!Objects.equals(node0.getClass().getSimpleName(), node1.getClass().getSimpleName())) { - return false; - } - - if (node0 instanceof SqlCall) { - SqlCall thisNode = (SqlCall) node0; - SqlCall thatNode = (SqlCall) node1; - if (!thisNode.getOperator().getName().equalsIgnoreCase(thatNode.getOperator().getName())) { - return false; - } - return isNodeEqual(thisNode.getOperandList(), thatNode.getOperandList()); - } - if (node0 instanceof SqlLiteral) { - SqlLiteral thisNode = (SqlLiteral) node0; - SqlLiteral thatNode = (SqlLiteral) node1; - return Objects.equals(thisNode.getValue(), thatNode.getValue()); - } - if (node0 instanceof SqlNodeList) { - SqlNodeList thisNode = (SqlNodeList) node0; - SqlNodeList thatNode = (SqlNodeList) node1; - if (thisNode.getList().size() != thatNode.getList().size()) { - return false; - } - for (int i = 0; i < thisNode.getList().size(); i++) { - SqlNode thisChild = thisNode.getList().get(i); - final SqlNode thatChild = thatNode.getList().get(i); - if (!isNodeEqual(thisChild, thatChild)) { - return false; - } - } - return true; - } - if (node0 instanceof SqlIdentifier) { - SqlIdentifier thisNode = (SqlIdentifier) node0; - SqlIdentifier thatNode = (SqlIdentifier) node1; - // compare ignore table alias.eg: expression like "a.b + a.c + a.d" ,alias a will be ignored when compared - String name0 = thisNode.names.get(thisNode.names.size() - 1).replace("\"", ""); - String name1 = thatNode.names.get(thatNode.names.size() - 1).replace("\"", ""); - return name0.equalsIgnoreCase(name1); - } - - logger.error("Convert to computed column fail,failed to compare two nodes,unknown instance type"); - return false; - } - - private static boolean isNodeEqual(List<SqlNode> operands0, List<SqlNode> operands1) { - if (operands0.size() != operands1.size()) { - return false; - } - for (int i = 0; i < operands0.size(); i++) { - if (!isNodeEqual(operands0.get(i), operands1.get(i))) { - return false; - } - } - return true; - } - private static String getTableAlias(SqlNode node) { if (node instanceof SqlCall) { SqlCall call = (SqlCall) node; @@ -350,4 +263,4 @@ class SqlTreeVisitor implements SqlVisitor<SqlNode> { public SqlNode visit(SqlIntervalQualifier intervalQualifier) { return null; } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/kylin/blob/d61f867f/query/src/test/java/org/apache/kylin/query/util/ConvertToComputedColumnTest.java ---------------------------------------------------------------------- diff --git a/query/src/test/java/org/apache/kylin/query/util/ConvertToComputedColumnTest.java b/query/src/test/java/org/apache/kylin/query/util/ConvertToComputedColumnTest.java index c3efe8d..f9b8d8b 100644 --- a/query/src/test/java/org/apache/kylin/query/util/ConvertToComputedColumnTest.java +++ b/query/src/test/java/org/apache/kylin/query/util/ConvertToComputedColumnTest.java @@ -21,8 +21,6 @@ package org.apache.kylin.query.util; import java.util.HashMap; import java.util.Map; -import org.apache.calcite.sql.SqlNode; -import org.apache.calcite.sql.SqlSelect; import org.apache.calcite.sql.parser.SqlParseException; import org.junit.Assert; import org.junit.Test; @@ -30,22 +28,6 @@ import org.junit.Test; import com.google.common.collect.ImmutableSortedMap; public class ConvertToComputedColumnTest { - @Test - public void testEqual() throws SqlParseException { - String sql0 = "select a.a + a.b + a.c from t as a"; - String sql1 = "select (((a . a + a.b + a.c))) from t as a"; - String sql2 = "select (a + b) + c from t"; - String sql3 = "select a.a + (a.b + a.c) from t as a"; - - SqlNode sn0 = getSelectNode(sql0); - SqlNode sn1 = getSelectNode(sql1); - SqlNode sn2 = getSelectNode(sql2); - SqlNode sn3 = getSelectNode(sql3); - - Assert.assertEquals(true, ConvertToComputedColumn.isNodeEqual(sn0, sn1)); - Assert.assertEquals(true, ConvertToComputedColumn.isNodeEqual(sn0, sn2)); - Assert.assertEquals(false, ConvertToComputedColumn.isNodeEqual(sn0, sn3)); - } @Test public void testErrorCase() { @@ -101,10 +83,6 @@ public class ConvertToComputedColumnTest { } - private static SqlNode getSelectNode(String sql) throws SqlParseException { - return ((SqlSelect) ConvertToComputedColumn.parse(sql)).getSelectList().get(0); - } - @Test public void testTwoCCHasSameSubExp() { String sql0 = "select a + b + c from t order by a + b";
