You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lens.apache.org by pr...@apache.org on 2017/07/13 09:13:58 UTC
[05/12] lens git commit: LENS-1445: Expression having reference
column ends up rewriting wrong query
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 <su...@gmail.com>
Authored: Fri Jun 23 16:40:25 2017 +0530
Committer: Rajat Khandelwal <ra...@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>