minor, fix computed column and adhoc issues
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/cf1ba956 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/cf1ba956 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/cf1ba956 Branch: refs/heads/kylin-2.1.x Commit: cf1ba9566c2b8ae8aa0f69dc8c27293bee156ab5 Parents: 5fb4f35 Author: Hongbin Ma <mahong...@apache.org> Authored: Fri Jun 30 15:50:09 2017 +0800 Committer: Hongbin Ma <m...@kyligence.io> Committed: Fri Jun 30 16:00:37 2017 +0800 ---------------------------------------------------------------------- .../apache/kylin/metadata/MetadataManager.java | 32 +------- .../kylin/metadata/model/DataModelDesc.java | 80 +++++++++----------- .../metadata/model/ModelDimensionDesc.java | 12 ++- .../metadata/model/tool/CalciteParser.java | 9 ++- .../kylin/model/tool/CalciteParserTest.java | 26 +++++-- .../query/util/ConvertToComputedColumn.java | 36 +++------ .../apache/kylin/rest/service/AclService.java | 4 +- .../apache/kylin/rest/service/ModelService.java | 2 +- .../org/apache/kylin/rest/util/AdHocUtil.java | 58 +++++--------- .../apache/kylin/rest/util/AdHocUtilTest.java | 23 +++--- .../kylin/rest/service/ModelServiceTest.java | 4 +- 11 files changed, 116 insertions(+), 170 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/core-metadata/src/main/java/org/apache/kylin/metadata/MetadataManager.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/MetadataManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/MetadataManager.java index 96ae294..6c4faa3 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/MetadataManager.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/MetadataManager.java @@ -25,7 +25,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -40,7 +39,6 @@ import org.apache.kylin.metadata.cachesync.Broadcaster; import org.apache.kylin.metadata.cachesync.Broadcaster.Event; import org.apache.kylin.metadata.cachesync.CaseInsensitiveStringCache; import org.apache.kylin.metadata.model.ColumnDesc; -import org.apache.kylin.metadata.model.ComputedColumnDesc; import org.apache.kylin.metadata.model.DataModelDesc; import org.apache.kylin.metadata.model.ExternalFilterDesc; import org.apache.kylin.metadata.model.TableDesc; @@ -120,26 +118,6 @@ public class MetadataManager { // name => External Filter Desc private CaseInsensitiveStringCache<ExternalFilterDesc> extFilterMap; - public static class CCInfo { - private ComputedColumnDesc computedColumnDesc; - private Set<DataModelDesc> dataModelDescs; - - public CCInfo(ComputedColumnDesc computedColumnDesc, Set<DataModelDesc> dataModelDescs) { - this.computedColumnDesc = computedColumnDesc; - this.dataModelDescs = dataModelDescs; - } - - public ComputedColumnDesc getComputedColumnDesc() { - return computedColumnDesc; - } - - public Set<DataModelDesc> getDataModelDescs() { - return dataModelDescs; - } - } - - private Map<String, CCInfo> ccInfoMap = Maps.newHashMap();// this is to check any two models won't conflict computed columns - private MetadataManager(KylinConfig config) throws IOException { init(config); } @@ -181,9 +159,6 @@ public class MetadataManager { return Collections.unmodifiableMap(srcTableMap.getMap()); } - public Map<String, CCInfo> getCcInfoMap() { - return ccInfoMap; - } public Map<String, TableExtDesc> listAllTableExdMap() { return srcTableExdMap.getMap(); @@ -373,7 +348,6 @@ public class MetadataManager { @Override public void onProjectSchemaChange(Broadcaster broadcaster, String project) throws IOException { - ccInfoMap.clear(); for (String model : ProjectManager.getInstance(config).getProject(project).getModels()) { reloadDataModelDescAt(DataModelDesc.concatResourcePath(model)); } @@ -546,7 +520,7 @@ public class MetadataManager { return new ArrayList<>(dataModelDescMap.values()); } - public List<DataModelDesc> getModels(String projectName) throws IOException { + public List<DataModelDesc> getModels(String projectName) { ProjectInstance projectInstance = ProjectManager.getInstance(config).getProject(projectName); ArrayList<DataModelDesc> ret = new ArrayList<>(); @@ -618,7 +592,7 @@ public class MetadataManager { DataModelDesc dataModelDesc = store.getResource(path, DataModelDesc.class, MODELDESC_SERIALIZER); if (!dataModelDesc.isDraft()) - dataModelDesc.init(config, this.getAllTablesMap(), this.ccInfoMap); + dataModelDesc.init(config, this.getAllTablesMap(), listDataModels()); dataModelDescMap.putLocal(dataModelDesc.getName(), dataModelDesc); return dataModelDesc; @@ -669,7 +643,7 @@ public class MetadataManager { private DataModelDesc saveDataModelDesc(DataModelDesc dataModelDesc) throws IOException { if (!dataModelDesc.isDraft()) - dataModelDesc.init(config, this.getAllTablesMap(), this.ccInfoMap); + dataModelDesc.init(config, this.getAllTablesMap(), listDataModels()); String path = dataModelDesc.getResourcePath(); getStore().putResource(path, dataModelDesc, MODELDESC_SERIALIZER); http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/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 341f36e..bc35e2a 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 @@ -18,8 +18,6 @@ package org.apache.kylin.metadata.model; -import static org.apache.kylin.metadata.MetadataManager.CCInfo; - import java.io.Serializable; import java.util.ArrayDeque; import java.util.ArrayList; @@ -38,6 +36,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.RootPersistentEntity; +import org.apache.kylin.common.util.Pair; import org.apache.kylin.common.util.StringUtil; import org.apache.kylin.metadata.MetadataConstants; import org.apache.kylin.metadata.model.JoinsTree.Chain; @@ -49,6 +48,7 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; 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.Lists; @@ -234,6 +234,7 @@ public class DataModelDesc extends RootPersistentEntity { return factTableRefs.contains(t); } + //TODO: different from isFactTable(TableRef t) public boolean isFactTable(String fullTableName) { for (TableRef t : factTableRefs) { if (t.getTableIdentity().equals(fullTableName)) @@ -332,7 +333,7 @@ public class DataModelDesc extends RootPersistentEntity { throw new IllegalArgumentException("Table not found by " + tableIdentity + " in model " + name); } - public void init(KylinConfig config, Map<String, TableDesc> originalTables, Map<String, CCInfo> ccInfoMap) { + public void init(KylinConfig config, Map<String, TableDesc> originalTables, List<DataModelDesc> dataModelDescs) { //tweak the tables according to Computed Columns defined in model Map<String, TableDesc> tables = Maps.newHashMap(); for (Map.Entry<String, TableDesc> entry : originalTables.entrySet()) { @@ -351,12 +352,12 @@ public class DataModelDesc extends RootPersistentEntity { initJoinsTree(); initDimensionsAndMetrics(); initPartitionDesc(); - initComputedColumns(ccInfoMap); + initComputedColumns(dataModelDescs); initFilterCondition(); boolean reinit = validate(); if (reinit) { // model slightly changed by validate() and must init() again - init(config, tables, ccInfoMap); + init(config, tables, dataModelDescs); } } @@ -471,38 +472,43 @@ public class DataModelDesc extends RootPersistentEntity { this.partitionDesc.init(this); } - private void initComputedColumns(Map<String, CCInfo> ccInfoMap) { - if (ccInfoMap == null) { - logger.error("cc info map is null"); + private void initComputedColumns(List<DataModelDesc> allDataModelDescs) { + Preconditions.checkNotNull(allDataModelDescs); + + List<Pair<ComputedColumnDesc, DataModelDesc>> existingCCs = Lists.newArrayList(); + + for (DataModelDesc dataModelDesc : allDataModelDescs) { + if (!StringUtils.equals(dataModelDesc.getName(), this.getName())) { + for (ComputedColumnDesc cc : dataModelDesc.getComputedColumnDescs()) { + existingCCs.add(Pair.newPair(cc, dataModelDesc)); + } + } } - Set<String> ccSet = Sets.newHashSet();//make sure cc name does not duplicate within this model + for (ComputedColumnDesc newCC : this.computedColumnDescs) { - for (ComputedColumnDesc thisCCDesc : this.computedColumnDescs) { - thisCCDesc.init(); - String thisCCName = thisCCDesc.getFullName(); + newCC.init(); + final String newCCName = newCC.getFullName(); + final String newCCColumnName = newCC.getColumnName(); - if (ccSet.contains(thisCCName)) { - throw new IllegalArgumentException(String.format( - "More than one computed column named %s exist in model %s", thisCCName, this.getName())); - } else { - ccSet.add(thisCCName); - } + for (Pair<ComputedColumnDesc, DataModelDesc> pair : existingCCs) { + DataModelDesc dataModelDesc = pair.getSecond(); + ComputedColumnDesc cc = pair.getFirst(); - CCInfo other = ccInfoMap.get(thisCCName); + if (StringUtils.equals(cc.getFullName(), newCCName) && !(cc.equals(newCC))) { + throw new IllegalArgumentException( + String.format("Computed column named %s is defined differently in model %s", newCCName, + dataModelDesc.getName())); + } - 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); - } else { - throw new IllegalStateException(String.format( - "Computed column named %s is already defined in other models: %s. Please change another name, or try to keep consistent definition", // - thisCCName, other.getDataModelDescs())); + if (isTwoCCDefinitionEquals(cc.getExpression(), newCC.getExpression()) + && !StringUtils.equals(cc.getColumnName(), newCCColumnName)) { + throw new IllegalArgumentException(String.format( + "Duplicate expression of %s with computed column %s in model %s, keep same computed column name could suppress this", + newCCColumnName, cc.getColumnName(), dataModelDesc.getName())); + } } + existingCCs.add(Pair.newPair(newCC, this)); } } @@ -548,20 +554,6 @@ 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/cf1ba956/core-metadata/src/main/java/org/apache/kylin/metadata/model/ModelDimensionDesc.java ---------------------------------------------------------------------- diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ModelDimensionDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ModelDimensionDesc.java index c0ddbad..283c39c 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ModelDimensionDesc.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ModelDimensionDesc.java @@ -31,7 +31,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE, getterVisibility = JsonAutoDetect.Visibility.NONE, isGetterVisibility = JsonAutoDetect.Visibility.NONE, setterVisibility = JsonAutoDetect.Visibility.NONE) public class ModelDimensionDesc implements Serializable { private static final long serialVersionUID = 1L; - + @JsonProperty("table") private String table; @JsonProperty("columns") @@ -58,12 +58,18 @@ public class ModelDimensionDesc implements Serializable { if (columns != null) { StringUtil.toUpperCaseArray(columns, columns); } - + if (model != null) { table = model.findTable(table).getAlias(); if (columns != null) { for (int i = 0; i < columns.length; i++) { - columns[i] = model.findColumn(table, columns[i]).getName(); + TblColRef column = model.findColumn(table, columns[i]); + + if (column.getColumnDesc().isComputedColumnn() && !model.isFactTable(column.getTableRef())) { + throw new RuntimeException("Computed Column on lookup table is not allowed"); + } + + columns[i] = column.getName(); } } } http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/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 47a90bf..9b80969 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 @@ -22,7 +22,9 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Objects; +import java.util.Set; +import com.google.common.collect.Lists; import org.apache.calcite.sql.SqlCall; import org.apache.calcite.sql.SqlIdentifier; import org.apache.calcite.sql.SqlLiteral; @@ -37,7 +39,7 @@ 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; public class CalciteParser { public static SqlNode parse(String sql) throws SqlParseException { @@ -151,19 +153,20 @@ public class CalciteParser { String sql = prefix + expr + suffix; SqlNode sqlNode = getOnlySelectNode(sql); - final List<SqlIdentifier> sqlIdentifiers = Lists.newArrayList(); + final Set<SqlIdentifier> s = Sets.newHashSet(); 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); + s.add(id); return null; } }; sqlNode.accept(sqlVisitor); + List<SqlIdentifier> sqlIdentifiers = Lists.newArrayList(s); Collections.sort(sqlIdentifiers, new Comparator<SqlIdentifier>() { @Override http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/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 acf7b5a..81c1c97 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 @@ -18,12 +18,13 @@ package org.apache.kylin.model.tool; +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.metadata.model.tool.CalciteParser; -import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -38,13 +39,13 @@ public class CalciteParserTest { @Test public void testNoTableNameExists() throws SqlParseException { String expr1 = "a + b"; - Assert.assertEquals("x.a + x.b", CalciteParser.insertAliasInExpr(expr1, "x")); + 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")); + 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")); + assertEquals("x.a + hiveudf(x.b)", CalciteParser.insertAliasInExpr(expr3, "x")); } @Test @@ -75,6 +76,17 @@ public class CalciteParserTest { } @Test + public void testCasewhen() { + String expr = "(CASE LSTG_FORMAT_NAME WHEN 'Auction' THEN 'x' WHEN 'y' THEN '222' ELSE 'z' END)"; + String alias = "TEST_KYLIN_FACT"; + String s = CalciteParser.insertAliasInExpr(expr, alias); + System.out.println(s); + assertEquals( + "(CASE TEST_KYLIN_FACT.LSTG_FORMAT_NAME WHEN 'Auction' THEN 'x' WHEN 'y' THEN '222' ELSE 'z' END)", + s); + } + + @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", // @@ -104,8 +116,8 @@ public class CalciteParserTest { 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)); + assertEquals(true, CalciteParser.isNodeEqual(sn0, sn1)); + assertEquals(true, CalciteParser.isNodeEqual(sn0, sn2)); + assertEquals(false, CalciteParser.isNodeEqual(sn0, sn3)); } } http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/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 7f188c1..a9ff360 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 @@ -24,8 +24,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import javax.annotation.Nullable; - import org.apache.calcite.sql.SqlCall; import org.apache.calcite.sql.SqlDataTypeSpec; import org.apache.calcite.sql.SqlDynamicParam; @@ -40,18 +38,15 @@ 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.ComputedColumnDesc; 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; import org.slf4j.LoggerFactory; import com.google.common.base.Functions; -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { @@ -65,7 +60,7 @@ public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { ImmutableSortedMap<String, String> computedColumns = getSortedComputedColumnWithProject(project); String s = replaceComputedColumn(sql, computedColumns); if (!StringUtils.equals(sql, s)) { - logger.debug("change sql to " + s); + logger.debug("sql changed"); } return s; } @@ -162,28 +157,15 @@ public class ConvertToComputedColumn implements QueryUtil.IQueryTransformer { } private ImmutableSortedMap<String, String> getSortedComputedColumnWithProject(String project) { - MetadataManager metadataManager = MetadataManager.getInstance(KylinConfig.getInstanceFromEnv()); - Map<String, MetadataManager.CCInfo> ccInfoMap = metadataManager.getCcInfoMap(); - final ProjectInstance projectInstance = ProjectManager.getInstance(KylinConfig.getInstanceFromEnv()) - .getProject(project); - - Iterable<MetadataManager.CCInfo> projectCCInfo = Iterables.filter(ccInfoMap.values(), - new Predicate<MetadataManager.CCInfo>() { - @Override - public boolean apply(@Nullable MetadataManager.CCInfo ccInfo) { - return Iterables.any(ccInfo.getDataModelDescs(), new Predicate<DataModelDesc>() { - @Override - public boolean apply(@Nullable DataModelDesc model) { - return projectInstance.containsModel(model.getName()); - } - }); - } - }); Map<String, String> computedColumns = new HashMap<>(); - for (MetadataManager.CCInfo ccInfo : projectCCInfo) { - computedColumns.put(ccInfo.getComputedColumnDesc().getColumnName(), - ccInfo.getComputedColumnDesc().getExpression()); + + MetadataManager metadataManager = MetadataManager.getInstance(KylinConfig.getInstanceFromEnv()); + List<DataModelDesc> dataModelDescs = metadataManager.getModels(project); + for (DataModelDesc dataModelDesc : dataModelDescs) { + for (ComputedColumnDesc computedColumnDesc : dataModelDesc.getComputedColumnDescs()) { + computedColumns.put(computedColumnDesc.getColumnName(), computedColumnDesc.getExpression()); + } } return getMapSortedByValue(computedColumns); http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java index 02b8e9f..79ed4aa 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java @@ -281,9 +281,7 @@ public class AclService implements MutableAclService { } else { aceInfos.addAll(allAceInfos.values()); } - } else { - logger.warn("Get null AllAceInfos from AclRecord"); - } + } List<AccessControlEntry> newAces = new ArrayList<AccessControlEntry>(); for (int i = 0; i < aceInfos.size(); i++) { http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/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 fa54eaa..89aee26 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 @@ -222,7 +222,7 @@ public class ModelService extends BasicService { List<CubeInstance> cubes = cubeService.listAllCubes(null, null, modelName, true); if (cubes != null && cubes.size() != 0) { dataModelDesc.init(getConfig(), getMetadataManager().getAllTablesMap(), - getMetadataManager().getCcInfoMap()); + getMetadataManager().listDataModels()); List<String> dimCols = new ArrayList<String>(); List<String> dimAndMCols = new ArrayList<String>(); http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/server-base/src/main/java/org/apache/kylin/rest/util/AdHocUtil.java ---------------------------------------------------------------------- diff --git a/server-base/src/main/java/org/apache/kylin/rest/util/AdHocUtil.java b/server-base/src/main/java/org/apache/kylin/rest/util/AdHocUtil.java index 9694a21..0eff508 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/util/AdHocUtil.java +++ b/server-base/src/main/java/org/apache/kylin/rest/util/AdHocUtil.java @@ -21,23 +21,18 @@ package org.apache.kylin.rest.util; import java.sql.SQLException; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.annotation.Nullable; - import org.apache.commons.lang.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; 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.MetadataManager.CCInfo; +import org.apache.kylin.metadata.model.ComputedColumnDesc; 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.apache.kylin.metadata.querymeta.SelectedColumnMeta; import org.apache.kylin.query.routing.NoRealizationFoundException; import org.apache.kylin.source.adhocquery.IAdHocConverter; @@ -46,8 +41,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.base.Preconditions; -import com.google.common.base.Predicate; -import com.google.common.collect.Iterables; import com.google.common.collect.Lists; public class AdHocUtil { @@ -72,9 +65,13 @@ public class AdHocUtil { logger.debug("Ad-hoc query runner {}", runner); String expandCC = restoreComputedColumnToExpr(sql, project); + if (!StringUtils.equals(expandCC, sql)) { + logger.info("computed column in sql is expanded to: " + expandCC); + } String adhocSql = converter.convert(expandCC); - if (!adhocSql.equals(sql)) { - logger.info("before delegating to adhoc, the query is converted to {} ", adhocSql); + if (!adhocSql.equals(expandCC)) { + logger.info("the query is converted to {} according to kylin.query.ad-hoc.converter-class-name", + adhocSql); } runner.executeQuery(adhocSql, results, columnMetas); @@ -98,37 +95,22 @@ public class AdHocUtil { private final static Pattern endWithAsPattern = Pattern.compile("\\s+as\\s+$", Pattern.CASE_INSENSITIVE); public static String restoreComputedColumnToExpr(String beforeSql, String project) { - MetadataManager metadataManager = MetadataManager.getInstance(KylinConfig.getInstanceFromEnv()); - Map<String, CCInfo> ccInfoMap = metadataManager.getCcInfoMap(); - final ProjectInstance projectInstance = ProjectManager.getInstance(KylinConfig.getInstanceFromEnv()) - .getProject(project); - - Iterable<CCInfo> projectCCInfo = Iterables.filter(ccInfoMap.values(), new Predicate<CCInfo>() { - @Override - public boolean apply(@Nullable CCInfo ccInfo) { - return Iterables.any(ccInfo.getDataModelDescs(), new Predicate<DataModelDesc>() { - @Override - public boolean apply(@Nullable DataModelDesc model) { - return projectInstance.containsModel(model.getName()); - } - }); - } - }); + final MetadataManager metadataManager = MetadataManager.getInstance(KylinConfig.getInstanceFromEnv()); + List<DataModelDesc> dataModelDescs = metadataManager.getModels(project); String afterSql = beforeSql; - for (CCInfo ccInfo : projectCCInfo) { - afterSql = restoreComputedColumnToExpr(afterSql, ccInfo); + for (DataModelDesc dataModelDesc : dataModelDescs) { + for (ComputedColumnDesc computedColumnDesc : dataModelDesc.getComputedColumnDescs()) { + afterSql = restoreComputedColumnToExpr(afterSql, computedColumnDesc); + } } - if (!StringUtils.equals(beforeSql, afterSql)) { - logger.info("computed column in sql is expanded before sending to adhoc engine: " + afterSql); - } return afterSql; } - static String restoreComputedColumnToExpr(String sql, CCInfo ccInfo) { + static String restoreComputedColumnToExpr(String sql, ComputedColumnDesc computedColumnDesc) { - String ccName = ccInfo.getComputedColumnDesc().getColumnName(); + String ccName = computedColumnDesc.getColumnName(); List<Triple<Integer, Integer, String>> replacements = Lists.newArrayList(); Matcher matcher = identifierInSqlPattern.matcher(sql); @@ -146,14 +128,14 @@ public class AdHocUtil { String quotedTableAlias = StringUtils.strip(matcher.group(2), "."); String tableAlias = StringUtils.strip(quotedTableAlias, "\""); replacements.add(Triple.of(matcher.start(1), matcher.end(1), - replaceIdentifierInExpr(ccInfo.getComputedColumnDesc().getExpression(), tableAlias, true))); + 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(ccInfo.getComputedColumnDesc().getExpression(), null, true))); + replaceIdentifierInExpr(computedColumnDesc.getExpression(), null, true))); } } else if (matcher.group(4) != null) { //without quote case: table.column or simply column String columnName = matcher.group(6); @@ -164,8 +146,8 @@ public class AdHocUtil { 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( - ccInfo.getComputedColumnDesc().getExpression(), tableAlias, false))); + 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()) { @@ -173,7 +155,7 @@ public class AdHocUtil { continue; } replacements.add(Triple.of(matcher.start(4), matcher.end(4), - replaceIdentifierInExpr(ccInfo.getComputedColumnDesc().getExpression(), null, false))); + replaceIdentifierInExpr(computedColumnDesc.getExpression(), null, false))); } } } http://git-wip-us.apache.org/repos/asf/kylin/blob/cf1ba956/server-base/src/test/java/org/apache/kylin/rest/util/AdHocUtilTest.java ---------------------------------------------------------------------- diff --git a/server-base/src/test/java/org/apache/kylin/rest/util/AdHocUtilTest.java b/server-base/src/test/java/org/apache/kylin/rest/util/AdHocUtilTest.java index 724844c..05b135a 100644 --- a/server-base/src/test/java/org/apache/kylin/rest/util/AdHocUtilTest.java +++ b/server-base/src/test/java/org/apache/kylin/rest/util/AdHocUtilTest.java @@ -17,7 +17,6 @@ */ package org.apache.kylin.rest.util; -import static org.apache.kylin.metadata.MetadataManager.CCInfo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -62,36 +61,34 @@ public class AdHocUtilTest { when(computedColumnDesc.getColumnName()).thenReturn("DEAL_AMOUNT"); when(computedColumnDesc.getExpression()).thenReturn("price * number"); - CCInfo ccInfo = mock(CCInfo.class); - when(ccInfo.getComputedColumnDesc()).thenReturn(computedColumnDesc); - { String ret = AdHocUtil.restoreComputedColumnToExpr( - "select DEAL_AMOUNT from DB.TABLE group by date order by DEAL_AMOUNT", ccInfo); - Assert.assertEquals( - "select (price * number) from DB.TABLE group by date order by (price * number)", - ret); + "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 = AdHocUtil.restoreComputedColumnToExpr( - "select DEAL_AMOUNT as DEAL_AMOUNT from DB.TABLE group by date order by DEAL_AMOUNT", ccInfo); + "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 = AdHocUtil.restoreComputedColumnToExpr( - "select \"DEAL_AMOUNT\" AS deal_amount from DB.TABLE group by date order by DEAL_AMOUNT", ccInfo); + "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 = AdHocUtil.restoreComputedColumnToExpr( - "select x.DEAL_AMOUNT AS deal_amount from DB.TABLE x group by date order by x.DEAL_AMOUNT", ccInfo); + "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); + "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/cf1ba956/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 b78b18e..b8633fb 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 @@ -80,8 +80,8 @@ public class ModelServiceTest extends ServiceTestBase { @Test public void testFailureModelUpdateDueToComputedColumnConflict() throws IOException, JobException, NoSuchFieldException, IllegalAccessException { - expectedEx.expect(IllegalStateException.class); - expectedEx.expectMessage("Computed column named DEFAULT.TEST_KYLIN_FACT.DEAL_AMOUNT is already defined in other models: [DataModelDesc [name=ci_left_join_model], DataModelDesc [name=ci_inner_join_model]]. Please change another name, or try to keep consistent definition"); + expectedEx.expect(IllegalArgumentException.class); + expectedEx.expectMessage("Computed column named DEFAULT.TEST_KYLIN_FACT.DEAL_AMOUNT is defined differently in model ci_inner_join_model"); List<DataModelDesc> dataModelDescs = modelService.listAllModels("ci_left_join_model", "default", true); Assert.assertTrue(dataModelDescs.size() == 1);
