LENS-596: Fix hasAggregates in GroupByResolver to look at expressions
Project: http://git-wip-us.apache.org/repos/asf/incubator-lens/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-lens/commit/022b589e Tree: http://git-wip-us.apache.org/repos/asf/incubator-lens/tree/022b589e Diff: http://git-wip-us.apache.org/repos/asf/incubator-lens/diff/022b589e Branch: refs/heads/current-release-line Commit: 022b589ee7c0ba4a33c3054b7b1ac116625316c4 Parents: b8995ed Author: Amareshwari Sriramadasu <[email protected]> Authored: Fri Jun 12 11:45:25 2015 +0530 Committer: Rajat Khandelwal <[email protected]> Committed: Fri Jun 12 11:45:25 2015 +0530 ---------------------------------------------------------------------- .../lens/cube/parse/ExpressionResolver.java | 16 ++++++-- .../apache/lens/cube/parse/GroupbyResolver.java | 43 +++++++++++++++++++- .../apache/lens/cube/parse/CubeTestSetup.java | 4 ++ .../lens/cube/parse/TestExpressionResolver.java | 23 +++++++++++ 4 files changed, 80 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/022b589e/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 539badb..8e199ea 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 @@ -186,6 +186,7 @@ class ExpressionResolver implements ContextRewriter { } evalSet.add(esc); } + Set<ASTNode> getAllASTNodes() { Set<ASTNode> allAST = new HashSet<ASTNode>(); for (ExprSpecContext esc : allExprs) { @@ -193,6 +194,15 @@ class ExpressionResolver implements ContextRewriter { } return allAST; } + + boolean hasAggregates() { + for (ExprSpecContext esc : allExprs) { + if (HQLParser.hasAggregate(esc.finalAST)) { + return true; + } + } + return false; + } } static class ExprSpecContext implements TrackQueriedColumns { @@ -311,10 +321,8 @@ class ExpressionResolver implements ContextRewriter { boolean hasAggregates() { for (Set<ExpressionContext> ecSet : allExprsQueried.values()) { for (ExpressionContext ec : ecSet) { - for (ExprSpecContext esc : ec.allExprs) { - if (HQLParser.isAggregateAST(esc.finalAST)) { - return true; - } + if (ec.hasAggregates()) { + return true; } } } http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/022b589e/lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java index 6a2a897..3201c09 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java @@ -24,6 +24,8 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import org.apache.lens.cube.metadata.AbstractBaseTable; + import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -244,7 +246,7 @@ class GroupbyResolver implements ContextRewriter { for (int i = 0; i < selectASTNode.getChildCount(); i++) { ASTNode childNode = (ASTNode) selectASTNode.getChild(i); - if (hasMeasure(childNode, cubeQueryCtx) || HQLParser.hasAggregate(childNode)) { + if (hasMeasure(childNode, cubeQueryCtx) || hasAggregate(childNode, cubeQueryCtx)) { continue; } nonMsrNonAggSelASTChildren.add(childNode); @@ -266,7 +268,7 @@ class GroupbyResolver implements ContextRewriter { if (hasMeasure(child, cubeql)) { continue; } - if (HQLParser.hasAggregate(child)) { + if (hasAggregate(child, cubeql)) { continue; } list.add(HQLParser.getString((ASTNode) node.getChild(i))); @@ -275,6 +277,43 @@ class GroupbyResolver implements ContextRewriter { return list; } + boolean hasAggregate(ASTNode node, CubeQueryContext cubeql) { + int nodeType = node.getToken().getType(); + if (nodeType == TOK_TABLE_OR_COL || nodeType == DOT) { + String colname; + String alias = ""; + + if (node.getToken().getType() == TOK_TABLE_OR_COL) { + colname = ((ASTNode) node.getChild(0)).getText().toLowerCase(); + } else { + // node in 'alias.column' format + ASTNode tabident = HQLParser.findNodeByPath(node, TOK_TABLE_OR_COL, Identifier); + ASTNode colIdent = (ASTNode) node.getChild(1); + + colname = colIdent.getText().toLowerCase(); + alias = tabident.getText().toLowerCase(); + } + // by the time Groupby resolver is looking for aggregate, all columns should be aliased with correct + // alias name. + if (cubeql.getCubeTableForAlias(alias) instanceof AbstractBaseTable) { + if (((AbstractBaseTable)cubeql.getCubeTableForAlias(alias)).getExpressionByName(colname) != null) { + return cubeql.getExprCtx().getExpressionContext(colname, alias).hasAggregates(); + } + } + } else { + if (HQLParser.isAggregateAST(node)) { + return true; + } + + for (int i = 0; i < node.getChildCount(); i++) { + if (hasAggregate((ASTNode) node.getChild(i), cubeql)) { + return true; + } + } + } + return false; + } + boolean hasMeasure(ASTNode node, CubeQueryContext cubeql) { int nodeType = node.getToken().getType(); if (nodeType == TOK_TABLE_OR_COL || nodeType == DOT) { http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/022b589e/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java index 914fe1b..554e709 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java @@ -662,6 +662,8 @@ public class CubeTestSetup { exprs = new HashSet<ExprColumn>(); exprs.add(new ExprColumn(new FieldSchema("avgmsr", "double", "avg measure"), "Avg Msr", "avg(msr1 + msr2)")); + exprs.add(new ExprColumn(new FieldSchema("summsrs", "double", "sum measures"), "Sum Msrs", + "(1000 + sum(msr1) + sum(msr2))/100")); exprs.add(new ExprColumn(new FieldSchema("msr5", "double", "materialized in some facts"), "Fifth Msr", "msr2 + msr3")); exprs.add(new ExprColumn(new FieldSchema("equalsums", "double", "sums are equals"), "equalsums", @@ -1468,6 +1470,8 @@ public class CubeTestSetup { null), new ExprSpec("concat(citydim.name, \":\", statedim.name)", null, null))); exprs.add(new ExprColumn(new FieldSchema("CityState", "string", "city's state"), "City State", new ExprSpec("concat(citydim.name, \":\", citydim.statename)", null, null))); + exprs.add(new ExprColumn(new FieldSchema("AggrExpr", "int", "count(name)"), "city count", + new ExprSpec("count(name)", null, null))); Dimension cityDim = new Dimension("citydim", cityAttrs, exprs, dimProps, 0L); client.createDimension(cityDim); http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/022b589e/lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java index 5cb7b0a..7f872e9 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java @@ -167,6 +167,20 @@ public class TestExpressionResolver extends TestQueryRewrite { TestCubeRewriter.compareQueries(hqlQuery, expected); } + + @Test + public void testExpressionInSelectToGroupbyWithComplexExpression() throws Exception { + String hqlQuery = + rewrite("select booleancut, summsrs from testCube" + " where " + TWO_DAYS_RANGE + " and substrexpr != 'XYZ'", + conf); + String expected = + getExpectedQuery(cubeName, "select testCube.dim1 != 'x' AND testCube.dim2 != 10 ," + + " ((1000 + sum(testCube.msr1) + sum(testCube.msr2))/100) FROM ", null, + " and substr(testCube.dim1, 3) != 'XYZ' group by testCube.dim1 != 'x' AND testCube.dim2 != 10", + getWhereForHourly2days("C1_testfact2_raw")); + TestCubeRewriter.compareQueries(hqlQuery, expected); + } + @Test public void testExpressionToJoin() throws Exception { // expression which results in join @@ -398,6 +412,15 @@ public class TestExpressionResolver extends TestQueryRewrite { } @Test + public void testDimensionQueryExpressionInSelectToGroupby() throws Exception { + String hqlQuery = rewrite("select id, AggrExpr from citydim", conf); + String expected = getExpectedQuery("citydim", "select citydim.id, count(citydim.name) FROM ", null, null, + " group by citydim.id", "c1_citytable", true); + TestCubeRewriter.compareQueries(hqlQuery, expected); + + } + + @Test public void testDimensionQueryWithTableAliasColumnAlias() throws Exception { String hqlQuery = rewrite("select ct.name cname, ct.cityaddress caddr from" + " citydim ct", conf);
