Repository: lens Updated Branches: refs/heads/master 67adf5b84 -> e2d6cbb94
LENS-1448: Having clause expressions are not resolved correctly for JoinCandidates Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/e2d6cbb9 Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/e2d6cbb9 Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/e2d6cbb9 Branch: refs/heads/master Commit: e2d6cbb947441e7c3c2a425cc9cf25ffadccf08b Parents: 67adf5b Author: Sushil Mohanty <sushil.k.moha...@gmail.com> Authored: Wed Jun 28 13:10:55 2017 +0530 Committer: Rajat Khandelwal <rajatgupt...@gmail.com> Committed: Wed Jun 28 13:10:55 2017 +0530 ---------------------------------------------------------------------- .../lens/cube/parse/ExpressionResolver.java | 11 +++++------ .../lens/cube/parse/UnionQueryWriter.java | 19 +++++++++++-------- .../lens/cube/parse/TestBaseCubeQueries.java | 20 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/e2d6cbb9/lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java index 2403576..66b043e 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java @@ -424,20 +424,19 @@ class ExpressionResolver implements ContextRewriter { } replaceAST(cubeql, queryAST.getJoinAST()); replaceAST(cubeql, queryAST.getGroupByAST()); - // Having AST is resolved by each fact, so that all facts can expand their expressions. - // Having ast is not copied now, it's maintained in cubeQueryContext, each fact processes that serially. + // Resolve having expression for StorageCandidate if (queryAST.getHavingAST() != null) { replaceAST(cubeql, queryAST.getHavingAST()); - } else if (cubeql.getHavingAST() != null && nonPickedExpressionsForCandidate.isEmpty()) { - replaceAST(cubeql, cubeql.getHavingAST()); - queryAST.setHavingAST(MetastoreUtil.copyAST(cubeql.getHavingAST())); + } else if (cubeql.getHavingAST() != null) { + ASTNode havingCopy = MetastoreUtil.copyAST(cubeql.getHavingAST()); + replaceAST(cubeql, havingCopy); + queryAST.setHavingAST(havingCopy); } replaceAST(cubeql, queryAST.getOrderByAST()); } private void replaceAST(final CubeQueryContext cubeql, ASTNode node) throws LensException { if (node == null) { - return; } // Traverse the tree and resolve expression columns http://git-wip-us.apache.org/repos/asf/lens/blob/e2d6cbb9/lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java index f6c9ce1..9dc7ee6 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java @@ -96,7 +96,6 @@ public class UnionQueryWriter extends SimpleHQLContext { if (sc.getQueryAst().getHavingAST() != null) { cubeql.setHavingAST(sc.getQueryAst().getHavingAST()); } - sc.getQueryAst().setHavingAST(null); sc.getQueryAst().setOrderByAST(null); sc.getQueryAst().setLimitValue(null); } @@ -120,8 +119,12 @@ public class UnionQueryWriter extends SimpleHQLContext { */ private ASTNode processHavingAST(ASTNode innerAst, AliasDecider aliasDecider, StorageCandidateHQLContext sc) throws LensException { - if (cubeql.getHavingAST() != null) { - ASTNode havingCopy = MetastoreUtil.copyAST(cubeql.getHavingAST()); + if (sc.getQueryAst().getHavingAST() == null + && cubeql.getHavingAST() != null) { + sc.getQueryAst().setHavingAST(cubeql.getHavingAST()); + } + if (sc.getQueryAst().getHavingAST() != null) { + ASTNode havingCopy = MetastoreUtil.copyAST(sc.getQueryAst().getHavingAST()); Set<ASTNode> havingAggChildrenASTs = new LinkedHashSet<>(); getAggregateChildrenInNode(havingCopy, havingAggChildrenASTs); processHavingExpression(innerAst, havingAggChildrenASTs, aliasDecider, sc); @@ -283,8 +286,9 @@ public class UnionQueryWriter extends SimpleHQLContext { } private boolean isNodeDefault(ASTNode node) { - if (HQLParser.isAggregateAST((ASTNode) node.getChild(0))) { - if (HQLParser.getString((ASTNode) node.getChild(0).getChild(1)).equals(DEFAULT_MEASURE)) { + if (HQLParser.isAggregateAST((ASTNode) node.getChild(0)) || HQLParser.isAggregateAST(node)) { + if (HQLParser.getString((ASTNode) node.getChild(0).getChild(1)).equals(DEFAULT_MEASURE) + || HQLParser.getString((ASTNode) node.getChild(1)).equals(DEFAULT_MEASURE)) { return true; } } @@ -640,9 +644,7 @@ public class UnionQueryWriter extends SimpleHQLContext { AliasDecider aliasDecider, StorageCandidateHQLContext sc) throws LensException { // iterate over all children of the ast and get outer ast corresponding to it. for (ASTNode child : havingAggASTs) { - if (!innerToOuterSelectASTs.containsKey(new HQLParser.HashableASTNode(child))) { - getOuterAST(child, innerSelectAst, aliasDecider, sc, false, cubeql.getBaseCube().getDimAttributeNames()); - } + getOuterAST(child, innerSelectAst, aliasDecider, sc, false, cubeql.getBaseCube().getDimAttributeNames()); } } @@ -726,6 +728,7 @@ public class UnionQueryWriter extends SimpleHQLContext { List<String> hqlQueries = new ArrayList<>(); for (StorageCandidateHQLContext sc : storageCandidates) { removeAggreagateFromDefaultColumns(sc.getQueryAst().getSelectAST()); + sc.getQueryAst().setHavingAST(null); hqlQueries.add(sc.toHQL()); } return hqlQueries.stream().collect(joining(" UNION ALL ", "(", ") as " + cubeql.getBaseCube())); http://git-wip-us.apache.org/repos/asf/lens/blob/e2d6cbb9/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java index f87158c..cf29dff 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java @@ -882,6 +882,26 @@ public class TestBaseCubeQueries extends TestQueryRewrite { assertTrue(hqlQuery.endsWith("HAVING ((sum((basecube.alias2)) > 2) " + "and (round((sum((basecube.alias3)) / 1000)) > 0))")); + // Having clause with expression answerable from any one fact and not projected + hqlQuery = rewrite("select dim1, dim11, msr12 from basecube where " + TWO_DAYS_RANGE + + "having msr12 > 2 and roundedmsr2 > 0", conf); + expected1 = getExpectedQuery(cubeName, + "SELECT (basecube.dim1) as `alias0`, (basecube.dim11) as `alias1`, sum((basecube.msr12)) " + + "as `alias2`, 0.0 as `alias4` FROM ", null, " group by basecube.dim1, basecube.dim11", + getWhereForDailyAndHourly2days(cubeName, "C1_testFact2_BASE")); + expected2 = getExpectedQuery(cubeName, + "SELECT (basecube.dim1) as `alias0`, (basecube.dim11) as `alias1`, 0.0 as `alias2`, " + + "sum((basecube.msr2)) as `alias4` FROM ", null, " group by basecube.dim1, basecube.dim11", + getWhereForDailyAndHourly2days(cubeName, "C1_testFact1_BASE")); + + compareContains(expected1, hqlQuery); + compareContains(expected2, hqlQuery); + assertTrue(hqlQuery.toLowerCase().startsWith("select (basecube.alias0) as `dim1`, (basecube.alias1) as `dim11`, " + + "sum((basecube.alias2)) as `msr12` from"), + hqlQuery); + assertTrue(hqlQuery.endsWith("HAVING ((sum((basecube.alias2)) > 2) and " + + "(round((sum((basecube.alias4)) / 1000)) > 0))")); + hqlQuery = rewrite("select dim1, dim11, msr12, roundedmsr2 from basecube where " + TWO_DAYS_RANGE + "having msr12+roundedmsr2 <= 1000 and msr12 > 2 and roundedmsr2 > 0", conf); expected1 = getExpectedQuery(cubeName,