LENS-1445: Expression having reference column ends up rewriting wrong query
Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/d49f45a0 Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/d49f45a0 Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/d49f45a0 Branch: refs/heads/current-release-line Commit: d49f45a0f8c6665784a3770a534d6495c21fd1bc Parents: a7f407b Author: Sushil Mohanty <sushil.k.moha...@gmail.com> Authored: Fri Jun 23 16:40:25 2017 +0530 Committer: Rajat Khandelwal <rajatgupt...@gmail.com> Committed: Thu Jul 13 14:42:53 2017 +0530 ---------------------------------------------------------------------- .../lens/cube/parse/UnionQueryWriter.java | 79 +++++++++++++------- .../parse/TestCubeSegmentationRewriter.java | 3 +- .../cube/parse/TestUnionAndJoinCandidates.java | 26 +++++++ .../resources/schema/cubes/base/basecube.xml | 11 +++ .../resources/schema/cubes/base/testcube.xml | 1 + 5 files changed, 92 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/d49f45a0/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 cc0a2e5..f6c9ce1 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 @@ -28,7 +28,8 @@ import static org.apache.hadoop.hive.ql.parse.HiveParser.*; import java.util.*; -import org.apache.lens.cube.metadata.MetastoreUtil; +import org.apache.lens.cube.metadata.*; +import org.apache.lens.cube.metadata.join.JoinPath; import org.apache.lens.server.api.error.LensException; import org.apache.hadoop.hive.ql.lib.Node; @@ -117,7 +118,7 @@ public class UnionQueryWriter extends SimpleHQLContext { * @return ASTNode * @throws LensException */ - private ASTNode processHavingAST(ASTNode innerAst, AliasDecider aliasDecider, StorageCandidate sc) + private ASTNode processHavingAST(ASTNode innerAst, AliasDecider aliasDecider, StorageCandidateHQLContext sc) throws LensException { if (cubeql.getHavingAST() != null) { ASTNode havingCopy = MetastoreUtil.copyAST(cubeql.getHavingAST()); @@ -251,25 +252,15 @@ public class UnionQueryWriter extends SimpleHQLContext { for (StorageCandidateHQLContext sc : storageCandidates) { node = (ASTNode) sc.getQueryAst().getSelectAST().getChild(position).getChild(0); if (HQLParser.isAggregateAST(node) || HQLParser.hasAggregate(node)) { - return MetastoreUtil.copyAST(node); + if (!node.getChild(1).toString().equals(DEFAULT_MEASURE)) { + return MetastoreUtil.copyAST(node); + } } } return MetastoreUtil.copyAST(node); } /** - * Check if ASTNode is answerable by StorageCandidate - * @param sc - * @param node - * @return - */ - private boolean isNodeNotAnswerableForStorageCandidate(StorageCandidate sc, ASTNode node) { - Set<String> cols = new LinkedHashSet<>(); - getAllColumnsOfNode(node, cols); - return !sc.getColumns().containsAll(cols); - } - - /** * Set the default value "0.0" in the non answerable aggreagte expressions. * @param node * @param sc @@ -467,7 +458,7 @@ public class UnionQueryWriter extends SimpleHQLContext { // Iterate over the StorageCandidates and add non projected having columns in inner select ASTs for (StorageCandidateHQLContext sc : storageCandidates) { aliasDecider.setCounter(selectAliasCounter); - processHavingAST(sc.getQueryAst().getSelectAST(), aliasDecider, sc.getStorageCandidate()); + processHavingAST(sc.getQueryAst().getSelectAST(), aliasDecider, sc); } removeRedundantProjectedPhrases(); } @@ -493,7 +484,7 @@ public class UnionQueryWriter extends SimpleHQLContext { ASTNode child = (ASTNode) selectAST.getChild(i); ASTNode outerSelect = new ASTNode(child); ASTNode selectExprAST = (ASTNode) child.getChild(0); - ASTNode outerAST = getOuterAST(selectExprAST, innerSelectAST, aliasDecider, sc.getStorageCandidate(), true, + ASTNode outerAST = getOuterAST(selectExprAST, innerSelectAST, aliasDecider, sc, true, cubeql.getBaseCube().getDimAttributeNames()); outerSelect.addChild(outerAST); // has an alias? add it @@ -529,13 +520,14 @@ public class UnionQueryWriter extends SimpleHQLContext { 5. If given ast is memorized as mentioned in the above cases, return the mapping. */ private ASTNode getOuterAST(ASTNode astNode, ASTNode innerSelectAST, - AliasDecider aliasDecider, StorageCandidate sc, boolean isSelectAst, Set<String> dimensionSet) + AliasDecider aliasDecider, StorageCandidateHQLContext scContext, boolean isSelectAst, Set<String> dimensionSet) throws LensException { + StorageCandidate sc = scContext.getStorageCandidate(); if (astNode == null) { return null; } Set<String> msrCols = new HashSet<>(); - getAllColumnsOfNode(astNode, msrCols); + getAllColumnsOfNode(astNode, msrCols, scContext); msrCols.removeAll(dimensionSet); if (isAggregateAST(astNode) && sc.getColumns().containsAll(msrCols)) { return processAggregate(astNode, innerSelectAST, aliasDecider, isSelectAst); @@ -544,7 +536,7 @@ public class UnionQueryWriter extends SimpleHQLContext { ASTNode exprCopy = MetastoreUtil.copyAST(astNode); setDefaultValueInExprForAggregateNodes(exprCopy, sc); outerAST.addChild(getOuterAST(getSelectExpr(exprCopy, null, true), - innerSelectAST, aliasDecider, sc, isSelectAst, dimensionSet)); + innerSelectAST, aliasDecider, scContext, isSelectAst, dimensionSet)); return outerAST; } else { if (hasAggregate(astNode)) { @@ -552,10 +544,12 @@ public class UnionQueryWriter extends SimpleHQLContext { for (Node child : astNode.getChildren()) { ASTNode childAST = (ASTNode) child; if (hasAggregate(childAST) && sc.getColumns().containsAll(msrCols)) { - outerAST.addChild(getOuterAST(childAST, innerSelectAST, aliasDecider, sc, isSelectAst, dimensionSet)); + outerAST.addChild(getOuterAST(childAST, innerSelectAST, aliasDecider, + scContext, isSelectAst, dimensionSet)); } else if (hasAggregate(childAST) && !sc.getColumns().containsAll(msrCols)) { childAST.replaceChildren(1, 1, getSelectExpr(null, null, true)); - outerAST.addChild(getOuterAST(childAST, innerSelectAST, aliasDecider, sc, isSelectAst, dimensionSet)); + outerAST.addChild(getOuterAST(childAST, innerSelectAST, aliasDecider, + scContext, isSelectAst, dimensionSet)); } else { outerAST.addChild(childAST); } @@ -643,7 +637,7 @@ public class UnionQueryWriter extends SimpleHQLContext { */ private void processHavingExpression(ASTNode innerSelectAst, Set<ASTNode> havingAggASTs, - AliasDecider aliasDecider, StorageCandidate sc) throws LensException { + 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))) { @@ -677,18 +671,51 @@ public class UnionQueryWriter extends SimpleHQLContext { * @param msrs * @return */ - private Set<String> getAllColumnsOfNode(ASTNode node, Set<String> msrs) { + private Set<String> getAllColumnsOfNode(ASTNode node, Set<String> msrs, StorageCandidateHQLContext sc) { if (node.getToken().getType() == HiveParser.DOT) { - msrs.add(node.getChild(1).toString()); + String col = node.getChild(1).toString(); + msrs.addAll(getSourceColumnOfRefColumn(col, sc)); } for (int i = 0; i < node.getChildCount(); i++) { ASTNode child = (ASTNode) node.getChild(i); - getAllColumnsOfNode(child, msrs); + getAllColumnsOfNode(child, msrs, sc); } return msrs; } /** + * Returns the source column of the ref column + * + * @param refCol + * @return + */ + private Set<String> getSourceColumnOfRefColumn(String refCol, StorageCandidateHQLContext sc) { + Set<String> sourceColumns = new HashSet<String>(); + for (Map.Entry<String, Set<String>> entry : sc.getCubeQueryContext().getTblAliasToColumns().entrySet()) { + if (entry.getValue().contains(refCol)) { + String table = entry.getKey(); + + if (sc.getCubeQueryContext().getAutoJoinCtx() != null) { + for (Map.Entry<Aliased<Dimension>, List<JoinPath>> dimPaths + : sc.getCubeQueryContext().getAutoJoinCtx().getAllPaths().entrySet()) { + + if (dimPaths.getKey().alias.equals(table)) { + List<JoinPath> joinPaths = dimPaths.getValue(); + for (JoinPath path : joinPaths) { + sourceColumns.addAll(path.getColumnsForTable(sc.getCubeQueryContext().getBaseCube())); + } + } + } + } + } + } + if (sourceColumns.isEmpty()) { + sourceColumns.add(refCol); + } + return sourceColumns; + } + + /** * Gets from string of the ouer query, this is a union query of all * StorageCandidates participated. * http://git-wip-us.apache.org/repos/asf/lens/blob/d49f45a0/lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java index 9fa31dc..7e1714b 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java @@ -234,8 +234,7 @@ public class TestCubeSegmentationRewriter extends TestQueryRewrite { userQuery = "select cityid, segmsr1 from testcube where cityname='blah' and " + TWO_DAYS_RANGE + " having citysegmsr1 > 20"; String rewrittenQuery = rewrite(userQuery, getConf()); - assertTrue(rewrittenQuery.toLowerCase().endsWith("sum(case when ((cubecity.name) = 'foo') " - + "then (testcube.segmsr1) end) > 20)")); + assertTrue(rewrittenQuery.toLowerCase().endsWith("(sum((testcube.alias2)) > 20)")); // Order by on alias userQuery = "select cityid as `city_id_alias`, segmsr1 from testcube where cityname='blah' and " http://git-wip-us.apache.org/repos/asf/lens/blob/d49f45a0/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionAndJoinCandidates.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionAndJoinCandidates.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionAndJoinCandidates.java index dc06ead..ccc3bf4 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionAndJoinCandidates.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionAndJoinCandidates.java @@ -92,6 +92,32 @@ public class TestUnionAndJoinCandidates extends TestQueryRewrite { } @Test + public void testExpressionHavingRefcol() throws ParseException, LensException { + String colsSelected = " union_join_ctx_cityid, union_join_ctx_cityname_msr1_expr, " + + "union_join_ctx_cityname_msr2_expr "; + String whereCond = "(" + TWO_MONTHS_RANGE_UPTO_DAYS + ")"; + String rewrittenQuery = rewrite("select " + colsSelected + " from basecube where " + whereCond, conf); + assertTrue(rewrittenQuery.contains("UNION ALL")); + String expectedInnerSelect1 = "SELECT (basecube.union_join_ctx_cityid) as `alias0`, sum(case " + + "when ((cubecityjoinunionctx.name) = 'blr') then (basecube.union_join_ctx_msr1) else 0 end) " + + "as `alias1`, 0.0 as `alias2` FROM TestQueryRewrite.c1_union_join_ctx_fact1 basecube "; + String expectedInnerSelect2 = "SELECT (basecube.union_join_ctx_cityid) as `alias0`, " + + "sum(case when ((cubecityjoinunionctx.name) = 'blr') then (basecube.union_join_ctx_msr1) else 0 end) " + + "as `alias1`, 0.0 as `alias2` FROM TestQueryRewrite.c1_union_join_ctx_fact2 basecube"; + String expectedInnerSelect3 = "SELECT (basecube.union_join_ctx_cityid) as `alias0`, 0.0 as `alias1`, " + + "sum(case when ((cubecityjoinunionctx.name) = 'blr') then (basecube.union_join_ctx_msr2) else 0 end) " + + "as `alias2` FROM TestQueryRewrite.c1_union_join_ctx_fact3 basecube"; + String outerSelect = "SELECT (basecube.alias0) as `union_join_ctx_cityid`, sum((basecube.alias1)) " + + "as `union_join_ctx_cityname_msr1_expr`, sum((basecube.alias2)) as `union_join_ctx_cityname_msr2_expr` FROM"; + String outerGroupBy = "GROUP BY (basecube.alias0)"; + compareContains(expectedInnerSelect1, rewrittenQuery); + compareContains(expectedInnerSelect2, rewrittenQuery); + compareContains(expectedInnerSelect3, rewrittenQuery); + compareContains(outerSelect, rewrittenQuery); + compareContains(outerGroupBy, rewrittenQuery); + } + + @Test public void testCustomExpressionForJoinCandidate() throws ParseException, LensException { // Expr : (case when union_join_ctx_msr2_expr = 0 then 0 else // union_join_ctx_msr4_expr * 100 / union_join_ctx_msr2_expr end) is completely answered by http://git-wip-us.apache.org/repos/asf/lens/blob/d49f45a0/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 bcea938..6bb5eb9 100644 --- a/lens-cube/src/test/resources/schema/cubes/base/basecube.xml +++ b/lens-cube/src/test/resources/schema/cubes/base/basecube.xml @@ -360,6 +360,17 @@ description="union_join_ctx_Not null cityid"> <expr_spec expr="case when union_join_ctx_cityid is null then 0 else union_join_ctx_cityid end"/> </expression> + + <expression _type="int" name="union_join_ctx_cityname_msr1_expr" display_string="union_join_ctx_cityname_msr1_expr" + description="union_join_ctx_cityname_msr1_expr"> + <expr_spec expr="sum(case when union_join_ctx_cityname = 'blr' then union_join_ctx_msr1 else 0 end)"/> + </expression> + + <expression _type="int" name="union_join_ctx_cityname_msr2_expr" display_string="union_join_ctx_cityname_msr2_expr" + description="union_join_ctx_cityname_msr2_expr"> + <expr_spec expr="sum(case when union_join_ctx_cityname = 'blr' then union_join_ctx_msr2 else 0 end)"/> + </expression> + <expression _type="String" name="cityandstatenew" display_string="City and State" description="city and state together"> <expr_spec expr="concat(cityname, ":", statename_cube)" end_time="$gregorian{now.month-2months}"/> http://git-wip-us.apache.org/repos/asf/lens/blob/d49f45a0/lens-cube/src/test/resources/schema/cubes/base/testcube.xml ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/resources/schema/cubes/base/testcube.xml b/lens-cube/src/test/resources/schema/cubes/base/testcube.xml index 088c8de..9d2bb02 100644 --- a/lens-cube/src/test/resources/schema/cubes/base/testcube.xml +++ b/lens-cube/src/test/resources/schema/cubes/base/testcube.xml @@ -206,6 +206,7 @@ <expression _type="int" name="notnullcityid" display_string="Not null cityid Expr" description="Not null cityid"> <expr_spec expr="case when cityid is null then 0 else cityid end"/> </expression> + <expression _type="double" name="roundedmsr1" display_string="Rounded msr1" description="rounded measure1"> <expr_spec expr="round(msr1/1000)"/> </expression>