KYLIN-2575 code refactor
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/6e28a020 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/6e28a020 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/6e28a020 Branch: refs/heads/master Commit: 6e28a020309796289d892a704f6a7f62cc7512c9 Parents: aed840f Author: Hongbin Ma <mahong...@apache.org> Authored: Mon Aug 21 20:35:15 2017 +0800 Committer: Hongbin Ma <m...@kyligence.io> Committed: Mon Aug 21 22:55:59 2017 +0800 ---------------------------------------------------------------------- .../kylin/measure/MeasureTypeFactory.java | 24 ++- .../metadata/model/ComputedColumnDesc.java | 37 +++- .../kylin/metadata/model/DataModelDesc.java | 19 ++ .../metadata/model/tool/CalciteParser.java | 43 ++-- .../adhocquery/HivePushDownConverter.java | 2 +- .../source/adhocquery/IPushDownConverter.java | 2 +- .../kylin/model/tool/CalciteParserTest.java | 6 +- .../apache/kylin/query/util/PushDownUtil.java | 119 ++--------- .../kylin/query/util/PushDownUtilTest.java | 207 +++++-------------- .../apache/kylin/rest/service/ModelService.java | 3 +- 10 files changed, 150 insertions(+), 312 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/core-metadata/src/main/java/org/apache/kylin/measure/MeasureTypeFactory.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/measure/MeasureTypeFactory.java b/core-metadata/src/main/java/org/apache/kylin/measure/MeasureTypeFactory.java index 7f3a5f1..be5e12c 100644 --- a/core-metadata/src/main/java/org/apache/kylin/measure/MeasureTypeFactory.java +++ b/core-metadata/src/main/java/org/apache/kylin/measure/MeasureTypeFactory.java @@ -121,7 +121,8 @@ abstract public class MeasureTypeFactory<T> { logger.info("Checking custom measure types from kylin config: " + customFactory); factoryInsts.add((MeasureTypeFactory<?>) Class.forName(customFactory).newInstance()); } catch (Exception e) { - throw new IllegalArgumentException("Unrecognized MeasureTypeFactory classname: " + customFactory, e); + throw new IllegalArgumentException("Unrecognized MeasureTypeFactory classname: " + customFactory, + e); } } } catch (KylinConfigCannotInitException e) { @@ -132,10 +133,12 @@ abstract public class MeasureTypeFactory<T> { for (MeasureTypeFactory<?> factory : factoryInsts) { String funcName = factory.getAggrFunctionName(); if (funcName.equals(funcName.toUpperCase()) == false) - throw new IllegalArgumentException("Aggregation function name '" + funcName + "' must be in upper case"); + throw new IllegalArgumentException( + "Aggregation function name '" + funcName + "' must be in upper case"); String dataTypeName = factory.getAggrDataTypeName(); if (dataTypeName.equals(dataTypeName.toLowerCase()) == false) - throw new IllegalArgumentException("Aggregation data type name '" + dataTypeName + "' must be in lower case"); + throw new IllegalArgumentException( + "Aggregation data type name '" + dataTypeName + "' must be in lower case"); Class<? extends DataTypeSerializer<?>> serializer = factory.getAggrDataTypeSerializer(); logger.info("registering " + funcName + "(" + dataTypeName + "), " + factory.getClass()); @@ -153,7 +156,8 @@ abstract public class MeasureTypeFactory<T> { } private static void registerUDAF(MeasureTypeFactory<?> factory) { - MeasureType<?> type = factory.createMeasureType(factory.getAggrFunctionName(), DataType.getType(factory.getAggrDataTypeName())); + MeasureType<?> type = factory.createMeasureType(factory.getAggrFunctionName(), + DataType.getType(factory.getAggrDataTypeName())); Map<String, Class<?>> udafs = type.getRewriteCalciteAggrFunctions(); if (type.needRewrite() == false || udafs == null) return; @@ -164,7 +168,8 @@ abstract public class MeasureTypeFactory<T> { continue; // skip built-in function if (udafFactories.containsKey(udaf)) - throw new IllegalStateException("UDAF '" + udaf + "' was dup declared by " + udafFactories.get(udaf) + " and " + factory); + throw new IllegalStateException( + "UDAF '" + udaf + "' was dup declared by " + udafFactories.get(udaf) + " and " + factory); udafFactories.put(udaf, factory); udafMap.put(udaf, udafs.get(udaf)); @@ -186,7 +191,8 @@ abstract public class MeasureTypeFactory<T> { public static MeasureType<?> createNoRewriteFieldsMeasureType(String funcName, DataType dataType) { // currently only has DimCountDistinctAgg if (funcName.equalsIgnoreCase(FunctionDesc.FUNC_COUNT_DISTINCT)) { - return new DimCountDistinctMeasureType.DimCountDistinctMeasureTypeFactory().createMeasureType(funcName, dataType); + return new DimCountDistinctMeasureType.DimCountDistinctMeasureTypeFactory().createMeasureType(funcName, + dataType); } throw new UnsupportedOperationException("No measure type found."); @@ -214,7 +220,8 @@ abstract public class MeasureTypeFactory<T> { if (f.getAggrDataTypeName().equals(dataType.getName())) return f.createMeasureType(funcName, dataType); } - throw new IllegalStateException(); + throw new IllegalStateException( + "failed to create MeasureType with funcName: " + funcName + ", dataType: " + dataType); } @SuppressWarnings("rawtypes") @@ -228,7 +235,8 @@ abstract public class MeasureTypeFactory<T> { if (needRewrite == null) needRewrite = Boolean.valueOf(b); else if (needRewrite.booleanValue() != b) - throw new IllegalStateException("needRewrite() of factorys " + factory + " does not have consensus"); + throw new IllegalStateException( + "needRewrite() of factorys " + factory + " does not have consensus"); } } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/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 ab8a634..9911fd2 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,13 @@ package org.apache.kylin.metadata.model; import java.io.Serializable; import java.util.Set; +import org.apache.calcite.sql.SqlAsOperator; +import org.apache.calcite.sql.SqlBasicCall; +import org.apache.calcite.sql.SqlCall; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.util.SqlBasicVisitor; +import org.apache.calcite.sql.util.SqlVisitor; import org.apache.commons.lang.StringUtils; import org.apache.kylin.metadata.model.tool.CalciteParser; import org.slf4j.Logger; @@ -76,7 +83,7 @@ public class ComputedColumnDesc implements Serializable { if ("true".equals(System.getProperty("needCheckCC"))) { //conditional execute this because of the calcite dependency is to available every where try { - CalciteParser.ensureAliasInExpr(expression, aliasSet); + simpleParserCheck(expression, aliasSet); } catch (Exception e) { String legacyHandled = handleLegacyCC(expression, rootFactTableName, aliasSet); if (legacyHandled != null) { @@ -92,7 +99,7 @@ public class ComputedColumnDesc implements Serializable { try { CalciteParser.ensureNoAliasInExpr(expr); String ret = CalciteParser.insertAliasInExpr(expr, rootFact); - CalciteParser.ensureAliasInExpr(ret, aliasSet); + simpleParserCheck(ret, aliasSet); return ret; } catch (Exception e) { logger.error("failed to handle legacy CC " + expr); @@ -100,6 +107,32 @@ public class ComputedColumnDesc implements Serializable { } } + public void simpleParserCheck(final String expr, final Set<String> aliasSet) { + SqlNode sqlNode = CalciteParser.getExpNode(expr); + + SqlVisitor sqlVisitor = new SqlBasicVisitor() { + @Override + public Object visit(SqlIdentifier id) { + if (id.names.size() != 2 || !aliasSet.contains(id.names.get(0))) { + throw new IllegalArgumentException("Column Identifier in the computed column " + expr + + "expression should comply to ALIAS.COLUMN "); + } + return null; + } + + @Override + public Object visit(SqlCall call) { + if (call instanceof SqlBasicCall && call.getOperator() instanceof SqlAsOperator) { + throw new IllegalArgumentException( + "Computed column expression " + expr + " should not contain AS "); + } + return call.getOperator().acceptCall(this, call); + } + }; + + sqlNode.accept(sqlVisitor); + } + public String getFullName() { return tableIdentity + "." + columnName; } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/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 8fcbc4c..fd609b8 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 @@ -54,6 +54,7 @@ import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -812,6 +813,24 @@ public class DataModelDesc extends RootPersistentEntity { return dimensions; } + public ComputedColumnDesc findCCByCCColumnName(final String columnName) { + return Iterables.find(this.computedColumnDescs, new Predicate<ComputedColumnDesc>() { + @Override + public boolean apply(@Nullable ComputedColumnDesc input) { + Preconditions.checkNotNull(input); + return columnName.equals(input.getColumnName()); + } + }); + } + + public Set<String> getComputedColumnNames() { + Set<String> ccColumnNames = Sets.newHashSet(); + for (ComputedColumnDesc cc : this.getComputedColumnDescs()) { + ccColumnNames.add(cc.getColumnName()); + } + return Collections.unmodifiableSet(ccColumnNames); + } + public List<ComputedColumnDesc> getComputedColumnDescs() { return computedColumnDescs; } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/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 index 08cbf09..fdad33a 100644 --- 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 @@ -32,11 +32,11 @@ 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; import com.google.common.collect.Sets; +import org.apache.kylin.common.util.Pair; public class CalciteParser { public static SqlNode parse(String sql) throws SqlParseException { @@ -64,23 +64,6 @@ public class CalciteParser { return getOnlySelectNode("select " + expr + " from t"); } - public static void ensureAliasInExpr(final String expr, final Set<String> aliasSet) { - SqlNode sqlNode = getExpNode(expr); - - SqlVisitor sqlVisitor = new SqlBasicVisitor() { - @Override - public Object visit(SqlIdentifier id) { - if (id.names.size() < 2 || !aliasSet.contains(id.names.get(0))) { - throw new IllegalArgumentException("Column Identifier in the computed column " + expr - + "expression should comply to ALIAS.COLUMN "); - } - return null; - } - }; - - sqlNode.accept(sqlVisitor); - } - public static void ensureNoAliasInExpr(String expr) { SqlNode sqlNode = getExpNode(expr); @@ -119,6 +102,18 @@ public class CalciteParser { sqlNode.accept(sqlVisitor); List<SqlIdentifier> sqlIdentifiers = Lists.newArrayList(s); + descSortByPosition(sqlIdentifiers); + + for (SqlIdentifier sqlIdentifier : sqlIdentifiers) { + Pair<Integer, Integer> replacePos = getReplacePos(sqlIdentifier, sql); + int start = replacePos.getFirst(); + sql = sql.substring(0, start) + alias + "." + sql.substring(start); + } + + return sql.substring(prefix.length(), sql.length() - suffix.length()); + } + + public static void descSortByPosition(List<SqlIdentifier> sqlIdentifiers) { Collections.sort(sqlIdentifiers, new Comparator<SqlIdentifier>() { @Override public int compare(SqlIdentifier o1, SqlIdentifier o2) { @@ -129,19 +124,11 @@ public class CalciteParser { 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 inputSql) { if (inputSql == null) { - return Pair.of(0, 0); + return Pair.newPair(0, 0); } String[] lines = inputSql.split("\n"); SqlParserPos pos = node.getParserPosition(); @@ -188,6 +175,6 @@ public class CalciteParser { right++; rightBracketNum++; } - return Pair.of(left, right); + return Pair.newPair(left, right); } } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/HivePushDownConverter.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/HivePushDownConverter.java b/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/HivePushDownConverter.java index 8d89294..bcd8608 100644 --- a/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/HivePushDownConverter.java +++ b/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/HivePushDownConverter.java @@ -291,7 +291,7 @@ public class HivePushDownConverter implements IPushDownConverter { } @Override - public String convert(String originSql) { + public String convert(String originSql, String project, String defaultSchema) { return doConvert(originSql); } } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/IPushDownConverter.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/IPushDownConverter.java b/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/IPushDownConverter.java index c4e7515..9835790 100644 --- a/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/IPushDownConverter.java +++ b/core-metadata/src/main/java/org/apache/kylin/source/adhocquery/IPushDownConverter.java @@ -21,5 +21,5 @@ package org.apache.kylin.source.adhocquery; * convert the query to satisfy the parser of push down query engine */ public interface IPushDownConverter { - String convert(String originSql); + String convert(String originSql, String project, String defaultSchema); } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/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 index 6483590..2c42ef3 100644 --- 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 @@ -23,7 +23,7 @@ import static org.junit.Assert.assertEquals; 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.common.util.Pair; import org.apache.kylin.metadata.model.tool.CalciteParser; import org.junit.Rule; import org.junit.Test; @@ -96,7 +96,7 @@ public class CalciteParserTest { for (String sql : sqls) { SqlNode parse = ((SqlSelect) CalciteParser.parse(sql)).getSelectList().get(0); Pair<Integer, Integer> replacePos = CalciteParser.getReplacePos(parse, sql); - String substring = sql.substring(replacePos.getLeft(), replacePos.getRight()); + String substring = sql.substring(replacePos.getFirst(), replacePos.getSecond()); Preconditions.checkArgument(substring.startsWith("a")); Preconditions.checkArgument(substring.endsWith("b")); } @@ -114,7 +114,7 @@ public class CalciteParserTest { for (String sql : sqls) { SqlNode parse = ((SqlSelect) CalciteParser.parse(sql)).getSelectList().get(0); Pair<Integer, Integer> replacePos = CalciteParser.getReplacePos(parse, sql); - String substring = sql.substring(replacePos.getLeft(), replacePos.getRight()); + String substring = sql.substring(replacePos.getFirst(), replacePos.getSecond()); Preconditions.checkArgument(substring.startsWith("(")); Preconditions.checkArgument(substring.endsWith(")")); } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java ---------------------------------------------------------------------- diff --git a/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java b/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java index d89b04e..ff88738 100644 --- a/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java +++ b/query/src/main/java/org/apache/kylin/query/util/PushDownUtil.java @@ -21,9 +21,8 @@ package org.apache.kylin.query.util; import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.apache.calcite.sql.SqlBasicCall; import org.apache.calcite.sql.SqlCall; @@ -39,16 +38,11 @@ import org.apache.calcite.sql.SqlOrderBy; import org.apache.calcite.sql.SqlSelect; import org.apache.calcite.sql.parser.SqlParseException; import org.apache.calcite.sql.util.SqlVisitor; -import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.text.StrBuilder; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.apache.commons.lang3.tuple.Pair; -import org.apache.commons.lang3.tuple.Triple; import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.util.ClassUtil; -import org.apache.kylin.metadata.MetadataManager; -import org.apache.kylin.metadata.model.ComputedColumnDesc; -import org.apache.kylin.metadata.model.DataModelDesc; +import org.apache.kylin.common.util.Pair; import org.apache.kylin.metadata.model.tool.CalciteParser; import org.apache.kylin.metadata.querymeta.SelectedColumnMeta; import org.apache.kylin.query.routing.NoRealizationFoundException; @@ -57,9 +51,6 @@ import org.apache.kylin.source.adhocquery.IPushDownRunner; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; - public class PushDownUtil { private static final Logger logger = LoggerFactory.getLogger(PushDownUtil.class); @@ -98,7 +89,7 @@ public class PushDownUtil { for (String converterName : kylinConfig.getPushDownConverterClassNames()) { IPushDownConverter converter = (IPushDownConverter) ClassUtil.newInstance(converterName); - String converted = converter.convert(sql); + String converted = converter.convert(sql, project, defaultSchema); if (!sql.equals(converted)) { logger.info("the query is converted to {} after applying converter {}", converted, converterName); sql = converted; @@ -133,106 +124,22 @@ public class PushDownUtil { } // make the behind position in the front of the list, so that the front position will not be affected when replaced - Collections.sort(tablesPos); - Collections.reverse(tablesPos); + Collections.sort(tablesPos, new Comparator<Pair<Integer, Integer>>() { + @Override + public int compare(Pair<Integer, Integer> o1, Pair<Integer, Integer> o2) { + int r = o2.getFirst() - o1.getFirst(); + return r == 0 ? o2.getSecond() - o1.getSecond() : r; + } + }); StrBuilder afterConvert = new StrBuilder(inputSql); for (Pair<Integer, Integer> pos : tablesPos) { - String tableWithSchema = schema + "." + inputSql.substring(pos.getLeft(), pos.getRight()); - afterConvert.replace(pos.getLeft(), pos.getRight(), tableWithSchema); + String tableWithSchema = schema + "." + inputSql.substring(pos.getFirst(), pos.getSecond()); + afterConvert.replace(pos.getFirst(), pos.getSecond(), tableWithSchema); } return afterConvert.toString(); } - private final static Pattern identifierInSqlPattern = Pattern.compile( - //find pattern like "table"."column" or "column" - "((?<![\\p{L}_0-9\\.\\\"])(\\\"[\\p{L}_0-9]+\\\"\\.)?(\\\"[\\p{L}_0-9]+\\\")(?![\\p{L}_0-9\\.\\\"]))" + "|" - //find pattern like table.column or column - + "((?<![\\p{L}_0-9\\.\\\"])([\\p{L}_0-9]+\\.)?([\\p{L}_0-9]+)(?![\\p{L}_0-9\\.\\\"]))"); - - private final static Pattern endWithAsPattern = Pattern.compile("\\s+as\\s+$", Pattern.CASE_INSENSITIVE); - - public static String restoreComputedColumnToExpr(String beforeSql, String project) { - final MetadataManager metadataManager = MetadataManager.getInstance(KylinConfig.getInstanceFromEnv()); - List<DataModelDesc> dataModelDescs = metadataManager.getModels(project); - - String afterSql = beforeSql; - for (DataModelDesc dataModelDesc : dataModelDescs) { - for (ComputedColumnDesc computedColumnDesc : dataModelDesc.getComputedColumnDescs()) { - afterSql = restoreComputedColumnToExpr(afterSql, computedColumnDesc); - } - } - return afterSql; - } - - static String restoreComputedColumnToExpr(String sql, ComputedColumnDesc computedColumnDesc) { - - String ccName = computedColumnDesc.getColumnName(); - List<Triple<Integer, Integer, String>> replacements = Lists.newArrayList(); - Matcher matcher = identifierInSqlPattern.matcher(sql); - - while (matcher.find()) { - if (matcher.group(1) != null) { //with quote case: "TABLE"."COLUMN" - - String quotedColumnName = matcher.group(3); - Preconditions.checkNotNull(quotedColumnName); - String columnName = StringUtils.strip(quotedColumnName, "\""); - if (!columnName.equalsIgnoreCase(ccName)) { - continue; - } - - if (matcher.group(2) != null) { // table name exist - String quotedTableAlias = StringUtils.strip(matcher.group(2), "."); - String tableAlias = StringUtils.strip(quotedTableAlias, "\""); - replacements.add(Triple.of(matcher.start(1), matcher.end(1), - replaceIdentifierInExpr(computedColumnDesc.getExpression(), tableAlias, true))); - } else { //only column - if (endWithAsPattern.matcher(sql.substring(0, matcher.start(1))).find()) { - //select DEAL_AMOUNT as "deal_amount" case - continue; - } - replacements.add(Triple.of(matcher.start(1), matcher.end(1), - replaceIdentifierInExpr(computedColumnDesc.getExpression(), null, true))); - } - } else if (matcher.group(4) != null) { //without quote case: table.column or simply column - String columnName = matcher.group(6); - Preconditions.checkNotNull(columnName); - if (!columnName.equalsIgnoreCase(ccName)) { - continue; - } - - if (matcher.group(5) != null) { //table name exist - String tableAlias = StringUtils.strip(matcher.group(5), "."); - replacements.add(Triple.of(matcher.start(4), matcher.end(4), - replaceIdentifierInExpr(computedColumnDesc.getExpression(), tableAlias, false))); - - } else { //only column - if (endWithAsPattern.matcher(sql.substring(0, matcher.start(4))).find()) { - //select DEAL_AMOUNT as deal_amount case - continue; - } - replacements.add(Triple.of(matcher.start(4), matcher.end(4), - replaceIdentifierInExpr(computedColumnDesc.getExpression(), null, false))); - } - } - } - - Collections.reverse(replacements); - for (Triple<Integer, Integer, String> triple : replacements) { - sql = sql.substring(0, triple.getLeft()) + "(" + triple.getRight() + ")" - + sql.substring(triple.getMiddle()); - } - return sql; - } - - static String replaceIdentifierInExpr(String expr, String tableAlias, boolean quoted) { - if (tableAlias == null) { - return expr; - } - - return CalciteParser.insertAliasInExpr(expr, tableAlias); - } - /** * Get all the tables from "FROM clause" that without schema * subquery is only considered in "from clause" @@ -312,4 +219,4 @@ public class PushDownUtil { return null; } } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/query/src/test/java/org/apache/kylin/query/util/PushDownUtilTest.java ---------------------------------------------------------------------- diff --git a/query/src/test/java/org/apache/kylin/query/util/PushDownUtilTest.java b/query/src/test/java/org/apache/kylin/query/util/PushDownUtilTest.java index afae2f2..63594bf 100644 --- a/query/src/test/java/org/apache/kylin/query/util/PushDownUtilTest.java +++ b/query/src/test/java/org/apache/kylin/query/util/PushDownUtilTest.java @@ -18,86 +18,57 @@ package org.apache.kylin.query.util; import org.apache.calcite.sql.parser.SqlParseException; -import org.apache.kylin.metadata.model.ComputedColumnDesc; import org.junit.Assert; import org.junit.Test; -import org.mockito.Mockito; public class PushDownUtilTest { @Test public void testSchemaCompletion() throws SqlParseException { - String sql1 = "SELECT a \n" + - "FROM a.KYLIN_SALES as KYLIN_SALES\n" + - "INNER JOIN \"A\".KYLIN_ACCOUNT as BUYER_ACCOUNT\n" + - "ON KYLIN_SALES.BUYER_ID = BUYER_ACCOUNT.ACCOUNT_ID\n" + - "INNER JOIN \"KYLIN_COUNTRY\" as BUYER_COUNTRY\n" + - "ON BUYER_ACCOUNT.ACCOUNT_COUNTRY = BUYER_COUNTRY.COUNTRY LIMIT 5"; + String sql1 = "SELECT a \n" + "FROM a.KYLIN_SALES as KYLIN_SALES\n" + + "INNER JOIN \"A\".KYLIN_ACCOUNT as BUYER_ACCOUNT\n" + + "ON KYLIN_SALES.BUYER_ID = BUYER_ACCOUNT.ACCOUNT_ID\n" + + "INNER JOIN \"KYLIN_COUNTRY\" as BUYER_COUNTRY\n" + + "ON BUYER_ACCOUNT.ACCOUNT_COUNTRY = BUYER_COUNTRY.COUNTRY LIMIT 5"; String sql2 = "select * from DB2.t,DB2.tt,ttt"; - String sql3 = "SELECT t1.week_beg_dt, t1.sum_price, t2.cnt\n" + - "FROM (\n" + - " select test_cal_dt.week_beg_dt, sum(price) as sum_price\n" + - " from DB1.\"test_kylin_fact\"\n" + - " inner JOIN test_cal_dt as test_cal_dt\n" + - " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + - " inner JOIN test_category_groupings\n" + - " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + - " inner JOIN test_sites as test_sites\n" + - " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + - " where price > 100\n" + - " group by test_cal_dt.week_beg_dt\n" + - " having sum(price) > 1000\n" + - " order by sum(price)\n" + - ") t1\n" + - "inner join (\n" + - " select test_cal_dt.week_beg_dt, count(*) as cnt\n" + - " from DB1.test_kylin_fact\n" + - " inner JOIN test_cal_dt as test_cal_dt\n" + - " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + - " inner JOIN test_category_groupings\n" + - " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + - " inner JOIN test_sites as test_sites\n" + - " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + - " group by test_cal_dt.week_beg_dt\n" + - ") t2\n" + - "on t1.week_beg_dt=t2.week_beg_dt limit 5"; + String sql3 = "SELECT t1.week_beg_dt, t1.sum_price, t2.cnt\n" + "FROM (\n" + + " select test_cal_dt.week_beg_dt, sum(price) as sum_price\n" + " from DB1.\"test_kylin_fact\"\n" + + " inner JOIN test_cal_dt as test_cal_dt\n" + " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + + " inner JOIN test_category_groupings\n" + + " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + + " inner JOIN test_sites as test_sites\n" + " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + + " where price > 100\n" + " group by test_cal_dt.week_beg_dt\n" + " having sum(price) > 1000\n" + + " order by sum(price)\n" + ") t1\n" + "inner join (\n" + + " select test_cal_dt.week_beg_dt, count(*) as cnt\n" + " from DB1.test_kylin_fact\n" + + " inner JOIN test_cal_dt as test_cal_dt\n" + " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + + " inner JOIN test_category_groupings\n" + + " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + + " inner JOIN test_sites as test_sites\n" + " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + + " group by test_cal_dt.week_beg_dt\n" + ") t2\n" + "on t1.week_beg_dt=t2.week_beg_dt limit 5"; - String exceptSQL1 = "SELECT a \n" + - "FROM a.KYLIN_SALES as KYLIN_SALES\n" + - "INNER JOIN \"A\".KYLIN_ACCOUNT as BUYER_ACCOUNT\n" + - "ON KYLIN_SALES.BUYER_ID = BUYER_ACCOUNT.ACCOUNT_ID\n" + - "INNER JOIN EDW.\"KYLIN_COUNTRY\" as BUYER_COUNTRY\n" + - "ON BUYER_ACCOUNT.ACCOUNT_COUNTRY = BUYER_COUNTRY.COUNTRY LIMIT 5"; + String exceptSQL1 = "SELECT a \n" + "FROM a.KYLIN_SALES as KYLIN_SALES\n" + + "INNER JOIN \"A\".KYLIN_ACCOUNT as BUYER_ACCOUNT\n" + + "ON KYLIN_SALES.BUYER_ID = BUYER_ACCOUNT.ACCOUNT_ID\n" + + "INNER JOIN EDW.\"KYLIN_COUNTRY\" as BUYER_COUNTRY\n" + + "ON BUYER_ACCOUNT.ACCOUNT_COUNTRY = BUYER_COUNTRY.COUNTRY LIMIT 5"; String exceptSQL2 = "select * from DB2.t,DB2.tt,EDW.ttt"; - String exceptSQL3 = "SELECT t1.week_beg_dt, t1.sum_price, t2.cnt\n" + - "FROM (\n" + - " select test_cal_dt.week_beg_dt, sum(price) as sum_price\n" + - " from DB1.\"test_kylin_fact\"\n" + - " inner JOIN EDW.test_cal_dt as test_cal_dt\n" + - " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + - " inner JOIN EDW.test_category_groupings\n" + - " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + - " inner JOIN EDW.test_sites as test_sites\n" + - " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + - " where price > 100\n" + - " group by test_cal_dt.week_beg_dt\n" + - " having sum(price) > 1000\n" + - " order by sum(price)\n" + - ") t1\n" + - "inner join (\n" + - " select test_cal_dt.week_beg_dt, count(*) as cnt\n" + - " from DB1.test_kylin_fact\n" + - " inner JOIN EDW.test_cal_dt as test_cal_dt\n" + - " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + - " inner JOIN EDW.test_category_groupings\n" + - " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + - " inner JOIN EDW.test_sites as test_sites\n" + - " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + - " group by test_cal_dt.week_beg_dt\n" + - ") t2\n" + - "on t1.week_beg_dt=t2.week_beg_dt limit 5"; + String exceptSQL3 = "SELECT t1.week_beg_dt, t1.sum_price, t2.cnt\n" + "FROM (\n" + + " select test_cal_dt.week_beg_dt, sum(price) as sum_price\n" + " from DB1.\"test_kylin_fact\"\n" + + " inner JOIN EDW.test_cal_dt as test_cal_dt\n" + " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + + " inner JOIN EDW.test_category_groupings\n" + + " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + + " inner JOIN EDW.test_sites as test_sites\n" + + " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + " where price > 100\n" + + " group by test_cal_dt.week_beg_dt\n" + " having sum(price) > 1000\n" + " order by sum(price)\n" + + ") t1\n" + "inner join (\n" + " select test_cal_dt.week_beg_dt, count(*) as cnt\n" + + " from DB1.test_kylin_fact\n" + " inner JOIN EDW.test_cal_dt as test_cal_dt\n" + + " ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt\n" + " inner JOIN EDW.test_category_groupings\n" + + " ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id\n" + + " inner JOIN EDW.test_sites as test_sites\n" + + " ON test_kylin_fact.lstg_site_id = test_sites.site_id\n" + " group by test_cal_dt.week_beg_dt\n" + + ") t2\n" + "on t1.week_beg_dt=t2.week_beg_dt limit 5"; Assert.assertEquals(exceptSQL1, PushDownUtil.schemaCompletion(sql1, "EDW")); Assert.assertEquals(exceptSQL2, PushDownUtil.schemaCompletion(sql2, "EDW")); Assert.assertEquals(exceptSQL3, PushDownUtil.schemaCompletion(sql3, "EDW")); @@ -105,100 +76,14 @@ public class PushDownUtilTest { @Test public void testSchemaCompletionWithComplexSubquery() throws SqlParseException { - String sql = - "SELECT a, b " + - "FROM (" + - " SELECT c, d, sum(p) " + - " FROM table1 t1, DB.table2 t2 " + - " WHERE t1.c > t2.d " + - " GROUP BY t.e" + - " HAVING sum(p) > 100" + - " ORDER BY t2.f" + - ") at1 " + - "INNER JOIN table3 t3 " + - "ON at1.c = t3.c " + - "WHERE t3.d > 0 " + - "ORDER BY t3.e"; + String sql = "SELECT a, b " + "FROM (" + " SELECT c, d, sum(p) " + " FROM table1 t1, DB.table2 t2 " + + " WHERE t1.c > t2.d " + " GROUP BY t.e" + " HAVING sum(p) > 100" + " ORDER BY t2.f" + ") at1 " + + "INNER JOIN table3 t3 " + "ON at1.c = t3.c " + "WHERE t3.d > 0 " + "ORDER BY t3.e"; - String exceptSQL = - "SELECT a, b " + - "FROM (" + - " SELECT c, d, sum(p) " + - " FROM EDW.table1 t1, DB.table2 t2 " + - " WHERE t1.c > t2.d " + - " GROUP BY t.e" + - " HAVING sum(p) > 100" + - " ORDER BY t2.f" + - ") at1 " + - "INNER JOIN EDW.table3 t3 " + - "ON at1.c = t3.c " + - "WHERE t3.d > 0 " + - "ORDER BY t3.e"; + String exceptSQL = "SELECT a, b " + "FROM (" + " SELECT c, d, sum(p) " + + " FROM EDW.table1 t1, DB.table2 t2 " + " WHERE t1.c > t2.d " + " GROUP BY t.e" + + " HAVING sum(p) > 100" + " ORDER BY t2.f" + ") at1 " + "INNER JOIN EDW.table3 t3 " + + "ON at1.c = t3.c " + "WHERE t3.d > 0 " + "ORDER BY t3.e"; Assert.assertEquals(exceptSQL, PushDownUtil.schemaCompletion(sql, "EDW")); } - - @Test - public void testReplaceIdentifierInExpr() { - { - String ret = PushDownUtil.replaceIdentifierInExpr("x * y", null, false); - Assert.assertEquals("x * y", ret); - } - { - String ret = PushDownUtil.replaceIdentifierInExpr("x_3 * y_3", "b_2", false); - Assert.assertEquals("b_2.x_3 * b_2.y_3", ret); - } - { - String ret = PushDownUtil.replaceIdentifierInExpr("substr(x,1,3)>y", "c", true); - Assert.assertEquals("substr(c.x,1,3)>c.y", ret); - } - { - String ret = PushDownUtil.replaceIdentifierInExpr("strcmp(substr(x,1,3),y)", "c", true); - Assert.assertEquals("strcmp(substr(c.x,1,3),c.y)", ret); - } - { - String ret = PushDownUtil.replaceIdentifierInExpr("strcmp(substr(x,1,3),y)", null, true); - Assert.assertEquals("strcmp(substr(x,1,3),y)", ret); - } - { - String ret = PushDownUtil.replaceIdentifierInExpr("strcmp(substr(x,1,3),y)", null, false); - Assert.assertEquals("strcmp(substr(x,1,3),y)", ret); - } - } - - @Test - public void testRestoreComputedColumnToExpr() { - - ComputedColumnDesc computedColumnDesc = Mockito.mock(ComputedColumnDesc.class); - Mockito.when(computedColumnDesc.getColumnName()).thenReturn("DEAL_AMOUNT"); - Mockito.when(computedColumnDesc.getExpression()).thenReturn("price * number"); - { - String ret = PushDownUtil.restoreComputedColumnToExpr( - "select DEAL_AMOUNT from DB.TABLE group by date order by DEAL_AMOUNT", computedColumnDesc); - Assert.assertEquals("select (price * number) from DB.TABLE group by date order by (price * number)", ret); - } - { - String ret = PushDownUtil.restoreComputedColumnToExpr( - "select DEAL_AMOUNT as DEAL_AMOUNT from DB.TABLE group by date order by DEAL_AMOUNT", - computedColumnDesc); - Assert.assertEquals( - "select (price * number) as DEAL_AMOUNT from DB.TABLE group by date order by (price * number)", - ret); - } - { - String ret = PushDownUtil.restoreComputedColumnToExpr( - "select \"DEAL_AMOUNT\" AS deal_amount from DB.TABLE group by date order by DEAL_AMOUNT", - computedColumnDesc); - Assert.assertEquals( - "select (price * number) AS deal_amount from DB.TABLE group by date order by (price * number)", - ret); - } - { - String ret = PushDownUtil.restoreComputedColumnToExpr( - "select x.DEAL_AMOUNT AS deal_amount from DB.TABLE x group by date order by x.DEAL_AMOUNT", - computedColumnDesc); - Assert.assertEquals( - "select (x.price * x.number) AS deal_amount from DB.TABLE x group by date order by (x.price * x.number)", - ret); - } - } } http://git-wip-us.apache.org/repos/asf/kylin/blob/6e28a020/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java index 6373cb8..14b2280 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/ModelService.java @@ -43,7 +43,6 @@ import org.apache.kylin.metadata.model.JoinsTree; import org.apache.kylin.metadata.model.ModelDimensionDesc; import org.apache.kylin.metadata.model.TableDesc; import org.apache.kylin.metadata.model.TblColRef; -import org.apache.kylin.metadata.model.tool.CalciteParser; import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.rest.exception.BadRequestException; import org.apache.kylin.rest.exception.ForbiddenException; @@ -228,7 +227,7 @@ public class ModelService extends BasicService { for (ComputedColumnDesc cc : dataModelDesc.getComputedColumnDescs()) { //check by calcite parser - CalciteParser.ensureAliasInExpr(cc.getExpression(), dataModelDesc.getAliasMap().keySet()); + cc.simpleParserCheck(cc.getExpression(), dataModelDesc.getAliasMap().keySet()); //check by hive cli, this could be slow StringBuilder sb = new StringBuilder();
