KYLIN-2162 Improve the cube validation error message
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/d5d2b9c2 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/d5d2b9c2 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/d5d2b9c2 Branch: refs/heads/KYLIN-2006 Commit: d5d2b9c28d625da54f7cbb72459382f73f5d8f9e Parents: 9615b4e Author: shaofengshi <shaofeng...@apache.org> Authored: Tue Nov 8 17:11:34 2016 +0800 Committer: shaofengshi <shaofeng...@apache.org> Committed: Tue Nov 8 17:11:50 2016 +0800 ---------------------------------------------------------------------- .../org/apache/kylin/cube/model/CubeDesc.java | 64 ++++++++++++-------- .../validation/rule/AggregationGroupRule.java | 52 +++++++++++++--- .../kylin/cube/AggregationGroupRuleTest.java | 6 +- .../org/apache/kylin/cube/CubeDescTest.java | 14 ++--- 4 files changed, 92 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/d5d2b9c2/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java ---------------------------------------------------------------------- diff --git a/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java b/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java index 34650f4..94b41a3 100644 --- a/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java +++ b/core-cube/src/main/java/org/apache/kylin/cube/model/CubeDesc.java @@ -29,6 +29,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -39,6 +40,7 @@ import java.util.Map.Entry; import javax.annotation.Nullable; +import com.google.common.collect.Iterables; import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.ArrayUtils; @@ -50,6 +52,7 @@ import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.RootPersistentEntity; import org.apache.kylin.common.util.Array; import org.apache.kylin.common.util.JsonUtil; +import org.apache.kylin.common.util.Pair; import org.apache.kylin.measure.MeasureType; import org.apache.kylin.measure.extendedcolumn.ExtendedColumnMeasureType; import org.apache.kylin.metadata.MetadataConstants; @@ -597,8 +600,15 @@ public class CubeDesc extends RootPersistentEntity implements IEngineAware { } if (!includeDims.containsAll(mandatoryDims) || !includeDims.containsAll(hierarchyDims) || !includeDims.containsAll(jointDims)) { - logger.error("Aggregation group " + index + " Include dims not containing all the used dims"); - throw new IllegalStateException("Aggregation group " + index + " Include dims not containing all the used dims"); + List<String> notIncluded = Lists.newArrayList(); + final Iterable<String> all = Iterables.unmodifiableIterable(Iterables.concat(mandatoryDims, hierarchyDims, jointDims)); + for (String dim : all) { + if (includeDims.contains(dim) == false) { + notIncluded.add(dim); + } + } + logger.error("Aggregation group " + index + " Include dimensions not containing all the used dimensions"); + throw new IllegalStateException("Aggregation group " + index + " 'includes' dimensions not include all the dimensions:" + notIncluded.toString()); } Set<String> normalDims = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); @@ -617,33 +627,36 @@ public class CubeDesc extends RootPersistentEntity implements IEngineAware { } if (CollectionUtils.containsAny(mandatoryDims, hierarchyDims)) { - logger.warn("Aggregation group " + index + " mandatory dims overlap with hierarchy dims"); + logger.warn("Aggregation group " + index + " mandatory dimensions overlap with hierarchy dimensions: " + CollectionUtils.intersection(mandatoryDims, hierarchyDims)); } if (CollectionUtils.containsAny(mandatoryDims, jointDims)) { - logger.warn("Aggregation group " + index + " mandatory dims overlap with joint dims"); + logger.warn("Aggregation group " + index + " mandatory dimensions overlap with joint dimensions: " + CollectionUtils.intersection(mandatoryDims, jointDims)); } if (CollectionUtils.containsAny(hierarchyDims, jointDims)) { - logger.error("Aggregation group " + index + " hierarchy dims overlap with joint dims"); - throw new IllegalStateException("Aggregation group " + index + " hierarchy dims overlap with joint dims"); + logger.error("Aggregation group " + index + " hierarchy dimensions overlap with joint dimensions"); + throw new IllegalStateException("Aggregation group " + index + " hierarchy dimensions overlap with joint dimensions: " + CollectionUtils.intersection(hierarchyDims, jointDims)); } if (hasSingle(hierarchyDimsList)) { - logger.error("Aggregation group " + index + " require at least 2 dims in a hierarchy"); - throw new IllegalStateException("Aggregation group " + index + " require at least 2 dims in a hierarchy"); + logger.error("Aggregation group " + index + " require at least 2 dimensions in a hierarchy"); + throw new IllegalStateException("Aggregation group " + index + " require at least 2 dimensions in a hierarchy."); } if (hasSingle(jointDimsList)) { - logger.error("Aggregation group " + index + " require at least 2 dims in a joint"); - throw new IllegalStateException("Aggregation group " + index + " require at least 2 dims in a joint"); + logger.error("Aggregation group " + index + " require at least 2 dimensions in a joint"); + throw new IllegalStateException("Aggregation group " + index + " require at least 2 dimensions in a joint"); } - if (hasOverlap(hierarchyDimsList, hierarchyDims)) { - logger.error("Aggregation group " + index + " a dim exist in more than one hierarchy"); - throw new IllegalStateException("Aggregation group " + index + " a dim exist in more than one hierarchy"); + Pair<Boolean, Set<String>> overlap = hasOverlap(hierarchyDimsList, hierarchyDims); + if (overlap.getFirst() == true) { + logger.error("Aggregation group " + index + " a dimension exist in more than one hierarchy: " + overlap.getSecond()); + throw new IllegalStateException("Aggregation group " + index + " a dimension exist in more than one hierarchy: " + overlap.getSecond()); } - if (hasOverlap(jointDimsList, jointDims)) { - logger.error("Aggregation group " + index + " a dim exist in more than one joint"); - throw new IllegalStateException("Aggregation group " + index + " a dim exist in more than one joint"); + + overlap = hasOverlap(jointDimsList, jointDims); + if (overlap.getFirst() == true) { + logger.error("Aggregation group " + index + " a dimension exist in more than one joint: " + overlap.getSecond()); + throw new IllegalStateException("Aggregation group " + index + " a dimension exist in more than one joint: " + overlap.getSecond()); } index++; @@ -674,21 +687,24 @@ public class CubeDesc extends RootPersistentEntity implements IEngineAware { private boolean hasSingle(ArrayList<Set<String>> dimsList) { boolean hasSingle = false; for (Set<String> dims : dimsList) { - if (dims.size() == 1) + if (dims.size() == 1) { hasSingle = true; + break; + } } return hasSingle; } - private boolean hasOverlap(ArrayList<Set<String>> dimsList, Set<String> Dims) { - boolean hasOverlap = false; - int dimSize = 0; + private Pair<Boolean, Set<String>> hasOverlap(ArrayList<Set<String>> dimsList, Set<String> Dims) { + Set<String> existing = new HashSet<>(); + Set<String> overlap = new HashSet<>(); for (Set<String> dims : dimsList) { - dimSize += dims.size(); + if (CollectionUtils.containsAny(existing, dims)) { + overlap.addAll(CollectionUtils.intersection(existing, dims)); + } + existing.addAll(dims); } - if (dimSize != Dims.size()) - hasOverlap = true; - return hasOverlap; + return new Pair<>(overlap.size() > 0, overlap); } private void initDimensionColumns() { http://git-wip-us.apache.org/repos/asf/kylin/blob/d5d2b9c2/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java ---------------------------------------------------------------------- diff --git a/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java b/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java index 7b90782..c1a062a 100644 --- a/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java +++ b/core-cube/src/main/java/org/apache/kylin/cube/model/validation/rule/AggregationGroupRule.java @@ -19,6 +19,8 @@ package org.apache.kylin.cube.model.validation.rule; import java.util.Arrays; +import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -29,6 +31,9 @@ import org.apache.kylin.cube.model.validation.IValidatorRule; import org.apache.kylin.cube.model.validation.ResultLevel; import org.apache.kylin.cube.model.validation.ValidateContext; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; + /** * find forbid overlaps in each AggregationGroup * the include dims in AggregationGroup must contain all mandatory, hierarchy and joint @@ -40,17 +45,20 @@ public class AggregationGroupRule implements IValidatorRule<CubeDesc> { inner(cube, context); } + public AggregationGroupRule() { + } + private void inner(CubeDesc cube, ValidateContext context) { int index = 0; for (AggregationGroup agg : cube.getAggregationGroups()) { if (agg.getIncludes() == null) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " includes field not set"); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " 'includes' field not set"); continue; } if (agg.getSelectRule() == null) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " select rule field not set"); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " 'select rule' field not set"); continue; } @@ -90,7 +98,14 @@ public class AggregationGroupRule implements IValidatorRule<CubeDesc> { } if (!includeDims.containsAll(mandatoryDims) || !includeDims.containsAll(hierarchyDims) || !includeDims.containsAll(jointDims)) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " Include dims not containing all the used dims"); + List<String> notIncluded = Lists.newArrayList(); + final Iterable<String> all = Iterables.unmodifiableIterable(Iterables.concat(mandatoryDims, hierarchyDims, jointDims)); + for (String dim : all) { + if (includeDims.contains(dim) == false) { + notIncluded.add(dim); + } + } + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " 'includes' dimensions not include all the dimensions:" + notIncluded.toString()); continue; } @@ -103,11 +118,15 @@ public class AggregationGroupRule implements IValidatorRule<CubeDesc> { combination = combination * (1 << normalDims.size()); if (CollectionUtils.containsAny(mandatoryDims, hierarchyDims)) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " mandatory dims overlap with hierarchy dims"); + Set<String> intersection = new HashSet<>(mandatoryDims); + intersection.retainAll(hierarchyDims); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " mandatory dimension has overlap with hierarchy dimension: " + intersection.toString()); continue; } if (CollectionUtils.containsAny(mandatoryDims, jointDims)) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " mandatory dims overlap with joint dims"); + Set<String> intersection = new HashSet<>(mandatoryDims); + intersection.retainAll(jointDims); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " mandatory dimension has overlap with joint dimension: " + intersection.toString()); continue; } @@ -121,7 +140,7 @@ public class AggregationGroupRule implements IValidatorRule<CubeDesc> { } if (oneJoint.size() < 2) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " require at least 2 dims in a joint"); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " require at least 2 dimensions in a joint: " + oneJoint.toString()); continue; } jointDimNum += oneJoint.size(); @@ -136,26 +155,39 @@ public class AggregationGroupRule implements IValidatorRule<CubeDesc> { overlapHierarchies++; } if (share.size() > 1) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " joint columns overlap with more than 1 dim in same hierarchy"); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " joint dimensions has overlap with more than 1 dimensions in same hierarchy: " + share.toString()); continue; } } if (overlapHierarchies > 1) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " joint columns overlap with more than 1 hierarchies"); + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " joint dimensions has overlap with more than 1 hierarchies"); continue; } } } if (jointDimNum != jointDims.size()) { - context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " a dim exist in more than one joint"); + + Set<String> existing = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + Set<String> overlap = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + for (String[] joints : agg.getSelectRule().joint_dims) { + Set<String> oneJoint = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + for (String s : joints) { + oneJoint.add(s); + } + if (CollectionUtils.containsAny(existing, oneJoint)) { + overlap.addAll(CollectionUtils.intersection(existing, oneJoint)); + } + existing.addAll(oneJoint); + } + context.addResult(ResultLevel.ERROR, "Aggregation group " + index + " a dimension exists in more than one joint: " + overlap.toString()); continue; } } if (combination > getMaxCombinations(cube)) { - String msg = "Aggregation group " + index + " has too many combinations, use 'mandatory'/'hierarchy'/'joint' to optimize; or update 'kylin.cube.aggrgroup.max.combination' to a bigger value."; + String msg = "Aggregation group " + index + " has too many combinations, current combination is " + combination + ", max allowed combination is " + getMaxCombinations(cube) + "; use 'mandatory'/'hierarchy'/'joint' to optimize; or update 'kylin.cube.aggrgroup.max.combination' to a bigger value."; context.addResult(ResultLevel.ERROR, msg); continue; } http://git-wip-us.apache.org/repos/asf/kylin/blob/d5d2b9c2/core-cube/src/test/java/org/apache/kylin/cube/AggregationGroupRuleTest.java ---------------------------------------------------------------------- diff --git a/core-cube/src/test/java/org/apache/kylin/cube/AggregationGroupRuleTest.java b/core-cube/src/test/java/org/apache/kylin/cube/AggregationGroupRuleTest.java index a06ce1b..0ebb249 100644 --- a/core-cube/src/test/java/org/apache/kylin/cube/AggregationGroupRuleTest.java +++ b/core-cube/src/test/java/org/apache/kylin/cube/AggregationGroupRuleTest.java @@ -65,7 +65,7 @@ public class AggregationGroupRuleTest { rule.validate(desc, vContext); vContext.print(System.out); assertTrue(vContext.getResults().length > 0); - assertEquals("Aggregation group 0 has too many combinations, use 'mandatory'/'hierarchy'/'joint' to optimize; or update 'kylin.cube.aggrgroup.max.combination' to a bigger value.", (vContext.getResults()[0].getMessage())); + assertTrue(vContext.getResults()[0].getMessage().startsWith("Aggregation group 0 has too many combinations")); } } @@ -97,7 +97,7 @@ public class AggregationGroupRuleTest { // System.out.println(vContext.getResults().length); // System.out.println(vContext.getResults()[0].getMessage()); assertEquals(1, vContext.getResults().length); - assertEquals("Aggregation group 0 Include dims not containing all the used dims", (vContext.getResults()[0].getMessage())); + assertEquals("Aggregation group 0 'includes' dimensions not include all the dimensions:[seller_id, META_CATEG_NAME, lstg_format_name, lstg_site_id, slr_segment_cd]", (vContext.getResults()[0].getMessage())); } @Test @@ -112,7 +112,7 @@ public class AggregationGroupRuleTest { rule.validate(desc, vContext); vContext.print(System.out); assertEquals(1, vContext.getResults().length); - assertEquals("Aggregation group 0 joint columns overlap with more than 1 dim in same hierarchy", (vContext.getResults()[0].getMessage())); + assertEquals("Aggregation group 0 joint dimensions has overlap with more than 1 dimensions in same hierarchy: [CATEG_LVL2_NAME, META_CATEG_NAME]", (vContext.getResults()[0].getMessage())); } public AggregationGroupRule getAggregationGroupRule() { http://git-wip-us.apache.org/repos/asf/kylin/blob/d5d2b9c2/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java ---------------------------------------------------------------------- diff --git a/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java b/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java index db80025..417ad46 100644 --- a/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java +++ b/core-cube/src/test/java/org/apache/kylin/cube/CubeDescTest.java @@ -86,7 +86,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit3() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 Include dims not containing all the used dims"); + thrown.expectMessage("Aggregation group 0 'includes' dimensions not include all the dimensions:[SELLER_ID, META_CATEG_NAME, LSTG_FORMAT_NAME, LSTG_SITE_ID, SLR_SEGMENT_CD]"); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); String[] temp = Arrays.asList(cubeDesc.getAggregationGroups().get(0).getIncludes()).subList(0, 3).toArray(new String[3]); @@ -128,7 +128,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit7() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 require at least 2 dims in a joint"); + thrown.expectMessage("Aggregation group 0 require at least 2 dimensions in a joint"); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); cubeDesc.getAggregationGroups().get(0).getSelectRule().joint_dims = new String[][] { new String[] { "lstg_format_name" } }; @@ -139,7 +139,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit8() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 hierarchy dims overlap with joint dims"); + thrown.expectMessage("Aggregation group 0 hierarchy dimensions overlap with joint dimensions: [CATEG_LVL2_NAME, META_CATEG_NAME]"); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); cubeDesc.getAggregationGroups().get(0).getSelectRule().joint_dims = new String[][] { new String[] { "META_CATEG_NAME", "CATEG_LVL2_NAME" } }; @@ -150,7 +150,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit9() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 hierarchy dims overlap with joint dims"); + thrown.expectMessage("Aggregation group 0 hierarchy dimensions overlap with joint dimensions: [lstg_format_name, META_CATEG_NAME]"); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); cubeDesc.getAggregationGroups().get(0).getSelectRule().hierarchy_dims = new String[][] { new String[] { "META_CATEG_NAME", "CATEG_LVL2_NAME", "CATEG_LVL3_NAME" }, new String[] { "lstg_format_name", "lstg_site_id" } }; @@ -162,7 +162,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit10() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 a dim exist in more than one joint"); + thrown.expectMessage("Aggregation group 0 a dimension exist in more than one joint: [lstg_format_name, lstg_site_id]"); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); cubeDesc.getAggregationGroups().get(0).getSelectRule().joint_dims = new String[][] { new String[] { "lstg_format_name", "lstg_site_id", "slr_segment_cd" }, new String[] { "lstg_format_name", "lstg_site_id", "leaf_categ_id" } }; @@ -173,7 +173,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit11() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 require at least 2 dims in a hierarchy"); + thrown.expectMessage("Aggregation group 0 require at least 2 dimensions in a hierarchy."); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); cubeDesc.getAggregationGroups().get(0).getSelectRule().hierarchy_dims = new String[][] { new String[] { "META_CATEG_NAME" } }; @@ -184,7 +184,7 @@ public class CubeDescTest extends LocalFileMetadataTestCase { @Test public void testBadInit12() throws Exception { thrown.expect(IllegalStateException.class); - thrown.expectMessage("Aggregation group 0 a dim exist in more than one hierarchy"); + thrown.expectMessage("Aggregation group 0 a dimension exist in more than one hierarchy: [CATEG_LVL2_NAME, META_CATEG_NAME]"); CubeDesc cubeDesc = CubeDescManager.getInstance(getTestConfig()).getCubeDesc("test_kylin_cube_with_slr_desc"); cubeDesc.getAggregationGroups().get(0).getSelectRule().hierarchy_dims = new String[][] { new String[] { "META_CATEG_NAME", "CATEG_LVL2_NAME", "CATEG_LVL3_NAME" }, new String[] { "META_CATEG_NAME", "CATEG_LVL2_NAME" } };
