LENS-1468: Expressions in having clauses are not getting rewritten properly for join queries
Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/2772efb2 Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/2772efb2 Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/2772efb2 Branch: refs/heads/current-release-line Commit: 2772efb275742eb4c03da5e7635bdbcfab63cebc Parents: 9a678d8 Author: Rajat Khandelwal <pro...@apache.org> Authored: Sun Sep 3 18:27:59 2017 +0530 Committer: rajub <raju.bairishe...@lazada.com> Committed: Thu Oct 5 11:13:31 2017 +0800 ---------------------------------------------------------------------- .../lens/cube/parse/ExpressionResolver.java | 55 +++++++++++++++----- .../cube/parse/StorageCandidateHQLContext.java | 4 ++ .../lens/cube/parse/UnionQueryWriter.java | 8 +++ .../lens/cube/parse/TestBaseCubeQueries.java | 32 +++++++++++- .../resources/schema/cubes/base/basecube.xml | 6 +++ .../resources/schema/cubes/derived/der2.xml | 2 + 6 files changed, 92 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/2772efb2/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 822e25e..553468f 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 @@ -34,6 +34,7 @@ import org.apache.hadoop.hive.ql.parse.HiveParser; import org.antlr.runtime.CommonToken; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import lombok.*; import lombok.extern.slf4j.Slf4j; @@ -262,7 +263,7 @@ class ExpressionResolver implements ContextRewriter { @RequiredArgsConstructor @ToString - private static class PickedExpression { + static class PickedExpression { private final String srcAlias; private final ExprSpecContext pickedCtx; private transient ASTNode reWrittenAST = null; @@ -286,11 +287,13 @@ class ExpressionResolver implements ContextRewriter { static class ExpressionResolverContext { @Getter - private Map<String, Set<ExpressionContext>> allExprsQueried = new HashMap<String, Set<ExpressionContext>>(); - private Map<String, Set<PickedExpression>> pickedExpressions = new HashMap<String, Set<PickedExpression>>(); + private Map<String, Set<ExpressionContext>> allExprsQueried = new HashMap<>(); + private Map<String, Set<PickedExpression>> pickedExpressions = new HashMap<>(); + @Getter + private Map<DimHQLContext, Map<String, Set<PickedExpression>>> pickedExpressionsPerCandidate = new HashMap<>(); private Map<String, ASTNode> nonPickedExpressionsForCandidate = new HashMap<String, ASTNode>(); private final CubeQueryContext cubeql; - + private boolean replacedHavingExpressions = false; ExpressionResolverContext(CubeQueryContext cubeql) { this.cubeql = cubeql; } @@ -411,7 +414,7 @@ class ExpressionResolver implements ContextRewriter { // Replace picked expressions in all the base trees replacePickedExpressions(sc); } - + pickedExpressionsPerCandidate.put(sc, Maps.newHashMap(pickedExpressions)); pickedExpressions.clear(); nonPickedExpressionsForCandidate.clear(); @@ -430,16 +433,42 @@ class ExpressionResolver implements ContextRewriter { replaceAST(cubeql, queryAST.getJoinAST()); replaceAST(cubeql, queryAST.getGroupByAST()); // Resolve having expression for StorageCandidate - if (queryAST.getHavingAST() != null) { - replaceAST(cubeql, queryAST.getHavingAST()); - } else if (cubeql.getHavingAST() != null) { - ASTNode havingCopy = MetastoreUtil.copyAST(cubeql.getHavingAST()); - replaceAST(cubeql, havingCopy); - queryAST.setHavingAST(havingCopy); - } replaceAST(cubeql, queryAST.getOrderByAST()); } - + public void replaceHavingExpressions() throws LensException { + replaceHavingExpressions(pickedExpressionsPerCandidate); + } + public void replaceHavingExpressions( + Map<DimHQLContext, Map<String, Set<PickedExpression>>> pickedExpressionsPerCandidate) throws LensException { + if (cubeql.getHavingAST() != null && !replacedHavingExpressions) { + HQLParser.bft(cubeql.getHavingAST(), visited -> { + ASTNode node1 = visited.getNode(); + int childcount = node1.getChildCount(); + for (int i = 0; i < childcount; i++) { + ASTNode current = (ASTNode) node1.getChild(i); + if (current.getToken().getType() == DOT) { + // This is for the case where column name is prefixed by table name + // or table alias + // For example 'select fact.id, dim2.id ...' + // Right child is the column name, left child.ident is table name + ASTNode tabident = HQLParser.findNodeByPath(current, TOK_TABLE_OR_COL, Identifier); + ASTNode colIdent = (ASTNode) current.getChild(1); + String column = colIdent.getText().toLowerCase(); + + Optional<PickedExpression> exprOptional = pickedExpressionsPerCandidate.values().stream() + .filter(x -> x.containsKey(column)).map(x -> x.get(column)).flatMap(Collection::stream) + .filter(x -> x.srcAlias.equals(tabident.getText().toLowerCase())).findFirst(); + + if (exprOptional.isPresent()) { + PickedExpression expr = exprOptional.get(); + node1.setChild(i, replaceAlias(expr.getRewrittenAST(), cubeql)); + } + } + } + }); + replacedHavingExpressions = true; + } + } private void replaceAST(final CubeQueryContext cubeql, ASTNode node) throws LensException { if (node == null) { return; http://git-wip-us.apache.org/repos/asf/lens/blob/2772efb2/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java index 993aa4c..21cdb61 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java @@ -129,6 +129,10 @@ public class StorageCandidateHQLContext extends DimHQLContext { @Override protected void setMissingExpressions() throws LensException { setFrom(getFromTable()); + if (getQueryAst().getHavingAST() != null) { + getStorageCandidate().getCubeQueryContext().getExprCtx().replaceHavingExpressions(); + getQueryAst().setHavingAST(getCubeQueryContext().getHavingAST()); + } String whereString = genWhereClauseWithDimPartitions(getWhere()); StringBuilder whereStringBuilder = (whereString != null) ? new StringBuilder(whereString) : new StringBuilder(); http://git-wip-us.apache.org/repos/asf/lens/blob/2772efb2/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 9dc7ee6..4eb086b 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 @@ -21,6 +21,7 @@ package org.apache.lens.cube.parse; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; import static org.apache.lens.cube.parse.HQLParser.*; @@ -30,6 +31,7 @@ import java.util.*; import org.apache.lens.cube.metadata.*; import org.apache.lens.cube.metadata.join.JoinPath; +import org.apache.lens.cube.parse.ExpressionResolver.PickedExpression; import org.apache.lens.server.api.error.LensException; import org.apache.hadoop.hive.ql.lib.Node; @@ -460,6 +462,12 @@ public class UnionQueryWriter extends SimpleHQLContext { queryAst.setSelectAST(outerSelectAst); // Iterate over the StorageCandidates and add non projected having columns in inner select ASTs + Map<DimHQLContext, Map<String, Set<PickedExpression>>> pickedExpressionsPerCandidate = new HashMap<>(); + for (CubeQueryContext cubeQueryContext : storageCandidates.stream() + .map(StorageCandidateHQLContext::getCubeQueryContext).collect(toSet())) { + pickedExpressionsPerCandidate.putAll(cubeQueryContext.getExprCtx().getPickedExpressionsPerCandidate()); + } + cubeql.getExprCtx().replaceHavingExpressions(pickedExpressionsPerCandidate); for (StorageCandidateHQLContext sc : storageCandidates) { aliasDecider.setCounter(selectAliasCounter); processHavingAST(sc.getQueryAst().getSelectAST(), aliasDecider, sc); http://git-wip-us.apache.org/repos/asf/lens/blob/2772efb2/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 cf29dff..35cb2b5 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 @@ -835,13 +835,41 @@ public class TestBaseCubeQueries extends TestQueryRewrite { } } } + @Test + public void testHavingOnTwoExpressions() throws Exception { + CubeQueryContext ctx1 = rewriteCtx("select dim1, msr2 from basecube where " + TWO_DAYS_RANGE + + "having effectivemsr2 > 0 and complexmsr12 > 10", conf); + CubeQueryContext ctx2 = rewriteCtx("select dim1, msr2 from basecube where " + TWO_DAYS_RANGE + + "having effectivemsr2 > 0 and complexmsr12 > 10", conf); + // shuffle join candidate order in ctx2 + for (Candidate candidate : ctx2.getCandidates()) { + if (candidate instanceof JoinCandidate) { + JoinCandidate jc = (JoinCandidate) candidate; + List<Candidate> children = jc.getChildren(); + Collections.reverse(children); + } + } + // toHQL outputs are tested in other functions, not testing here. + + // test having clauses are same in both + String having1 = ctx1.toHQL().substring(ctx1.toHQL().indexOf("HAVING")); + String having2 = ctx2.toHQL().substring(ctx2.toHQL().indexOf("HAVING")); + assertEquals(having1, having2, "having1: " + having1 + "\nhaving2: " + having2); + + // assert order of facts is differnet in to hqls + int ind11 = ctx1.toHQL().indexOf("c1_testfact1_base"); + int ind21 = ctx2.toHQL().indexOf("c1_testfact1_base"); + + int ind12 = ctx1.toHQL().indexOf("c1_testfact2_base"); + int ind22 = ctx2.toHQL().indexOf("c1_testfact2_base"); + + assertTrue((ind11 < ind21 && ind12 > ind22) || (ind11 > ind21 && ind12 < ind22)); + } @Test public void testMultiFactQueryWithHaving() throws Exception { String hqlQuery, expected1, expected2; - String endSubString = "mq2 on mq1.dim1 <=> mq2.dim1 AND mq1.dim11 <=> mq2.dim11"; - String joinSubString = "mq1 full outer join "; // only One having clause, that too answerable from one fact hqlQuery = rewrite("select dim1, dim11, msr12 from basecube where " + TWO_DAYS_RANGE http://git-wip-us.apache.org/repos/asf/lens/blob/2772efb2/lens-cube/src/test/resources/schema/cubes/base/basecube.xml ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/resources/schema/cubes/base/basecube.xml b/lens-cube/src/test/resources/schema/cubes/base/basecube.xml index 6bb5eb9..6cc3201 100644 --- a/lens-cube/src/test/resources/schema/cubes/base/basecube.xml +++ b/lens-cube/src/test/resources/schema/cubes/base/basecube.xml @@ -269,6 +269,12 @@ <expression _type="double" name="flooredmsr12" display_string="Floored msr12" description="floored measure12"> <expr_spec expr="floor(msr12)"/> </expression> + <expression _type="double" name="complexmsr12" display_string="Floored msr12" description="floored measure12"> + <expr_spec expr="floor(msr12)+0"/> + </expression> + <expression _type="double" name="effectivemsr2" display_string="effective msr12" description="effective measure2"> + <expr_spec expr="msr2 + msr21 + msr22"/> + </expression> <expression _type="String" name="cityandstate" display_string="City and State" description="city and state together"> <expr_spec expr="concat(cityname, ":", statename_cube)"/> http://git-wip-us.apache.org/repos/asf/lens/blob/2772efb2/lens-cube/src/test/resources/schema/cubes/derived/der2.xml ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/resources/schema/cubes/derived/der2.xml b/lens-cube/src/test/resources/schema/cubes/derived/der2.xml index 337e7f4..213fe07 100644 --- a/lens-cube/src/test/resources/schema/cubes/derived/der2.xml +++ b/lens-cube/src/test/resources/schema/cubes/derived/der2.xml @@ -33,6 +33,8 @@ <measure_names> <measure_name>directmsr</measure_name> <measure_name>msr2</measure_name> + <measure_name>msr21</measure_name> + <measure_name>msr22</measure_name> <measure_name>msr12</measure_name> <measure_name>msr14</measure_name> <measure_name>msr13</measure_name>
