Repository: lens
Updated Branches:
  refs/heads/master 62cafa448 -> a4b4b85d8


LENS-1149 : Union queries with dim-attribute expressions are not written 
properly


Project: http://git-wip-us.apache.org/repos/asf/lens/repo
Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/a4b4b85d
Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/a4b4b85d
Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/a4b4b85d

Branch: refs/heads/master
Commit: a4b4b85d8179f1d7c7f0773f47b7aeebe2a0234d
Parents: 62cafa4
Author: Amareshwari Sriramadasu <amareshw...@apache.org>
Authored: Wed May 25 16:47:03 2016 +0530
Committer: sushilmohanty <sushilmoha...@apache.org>
Committed: Wed May 25 16:47:03 2016 +0530

----------------------------------------------------------------------
 .../parse/SingleFactMultiStorageHQLContext.java | 169 ++++++++++++-------
 .../apache/lens/cube/parse/CubeTestSetup.java   |   2 +
 .../lens/cube/parse/TestUnionQueries.java       |  68 ++++++++
 3 files changed, 180 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lens/blob/a4b4b85d/lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java
----------------------------------------------------------------------
diff --git 
a/lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java
 
b/lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java
index 1af031a..9b48213 100644
--- 
a/lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java
+++ 
b/lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java
@@ -66,7 +66,7 @@ public class SingleFactMultiStorageHQLContext extends 
UnionHQLContext {
   private void processSelectAST() {
     ASTNode originalSelectAST = MetastoreUtil.copyAST(ast.getSelectAST());
     ast.setSelectAST(new ASTNode(originalSelectAST.getToken()));
-    ASTNode outerSelectAST = processExpression(originalSelectAST);
+    ASTNode outerSelectAST = processSelectExpression(originalSelectAST);
     setSelect(getString(outerSelectAST));
   }
 
@@ -86,7 +86,7 @@ public class SingleFactMultiStorageHQLContext extends 
UnionHQLContext {
 
   private void processOrderByAST() {
     if (ast.getOrderByAST() != null) {
-      setOrderby(getString(processExpression(ast.getOrderByAST())));
+      setOrderby(getString(processOrderbyExpression(ast.getOrderByAST())));
       ast.setOrderByAST(null);
     }
   }
@@ -96,51 +96,117 @@ public class SingleFactMultiStorageHQLContext extends 
UnionHQLContext {
     ast.setLimitValue(null);
   }
 
+  private ASTNode processExpression(ASTNode astNode) {
+    if (astNode == null) {
+      return null;
+    }
+    ASTNode outerExpression = new ASTNode(astNode);
+    // iterate over all children of the ast and get outer ast corresponding to 
it.
+    for (Node child : astNode.getChildren()) {
+      outerExpression.addChild(getOuterAST((ASTNode)child));
+    }
+    return outerExpression;
+  }
+
+  private ASTNode processSelectExpression(ASTNode astNode) {
+    if (astNode == null) {
+      return null;
+    }
+    ASTNode outerExpression = new ASTNode(astNode);
+    // iterate over all children of the ast and get outer ast corresponding to 
it.
+    for (Node node : astNode.getChildren()) {
+      ASTNode child = (ASTNode)node;
+      ASTNode outerSelect = new ASTNode(child);
+      ASTNode selectExprAST = (ASTNode)child.getChild(0);
+      ASTNode outerAST = getOuterAST(selectExprAST);
+      outerSelect.addChild(outerAST);
+
+      // has an alias? add it
+      if (child.getChildCount() > 1) {
+        outerSelect.addChild(child.getChild(1));
+      }
+      outerExpression.addChild(outerSelect);
+    }
+    return outerExpression;
+  }
+
+  private ASTNode processOrderbyExpression(ASTNode astNode) {
+    if (astNode == null) {
+      return null;
+    }
+    ASTNode outerExpression = new ASTNode(astNode);
+    // sample orderby AST looks the following :
+    /*
+    TOK_ORDERBY
+   TOK_TABSORTCOLNAMEDESC
+      TOK_NULLS_LAST
+         .
+            TOK_TABLE_OR_COL
+               testcube
+            cityid
+   TOK_TABSORTCOLNAMEASC
+      TOK_NULLS_FIRST
+         .
+            TOK_TABLE_OR_COL
+               testcube
+            stateid
+   TOK_TABSORTCOLNAMEASC
+      TOK_NULLS_FIRST
+         .
+            TOK_TABLE_OR_COL
+               testcube
+            zipcode
+     */
+    for (Node node : astNode.getChildren()) {
+      ASTNode child = (ASTNode)node;
+      ASTNode outerOrderby = new ASTNode(child);
+      ASTNode tokNullsChild = (ASTNode) child.getChild(0);
+      ASTNode outerTokNullsChild = new ASTNode(tokNullsChild);
+      
outerTokNullsChild.addChild(getOuterAST((ASTNode)tokNullsChild.getChild(0)));
+      outerOrderby.addChild(outerTokNullsChild);
+      outerExpression.addChild(outerOrderby);
+    }
+    return outerExpression;
+  }
   /*
+
   Perform a DFS on the provided AST, and Create an AST of similar structure 
with changes specific to the
   inner query - outer query dynamics. The resultant AST is supposed to be used 
in outer query.
 
   Base cases:
    1. ast is null => null
-   2. ast is table.column => add this to inner select expressions, generate 
alias, return cube.alias. Memoize the
-            mapping table.column => cube.alias
-   3. ast is aggregate_function(table.column) => add 
aggregate_function(table.column) to inner select expressions,
+   2. ast is aggregate_function(table.column) => add 
aggregate_function(table.column) to inner select expressions,
             generate alias, return aggregate_function(cube.alias). Memoize the 
mapping
             aggregate_function(table.column) => aggregate_function(cube.alias)
             Assumption is aggregate_function is transitive i.e. f(a,b,c,d) = 
f(f(a,b), f(c,d)). SUM, MAX, MIN etc
             are transitive, while AVG, COUNT etc are not. For non-transitive 
aggregate functions, the re-written
             query will be incorrect.
-   4. If given ast is memoized as mentioned in the above cases, return the 
mapping.
-
-   Recursive case:
-     Copy the root node, process children recursively and add as children to 
the copied node. Return the copied node.
+   3. ast has aggregates - iterate over children and add the non aggregate 
nodes as is and recursively get outer ast
+   for aggregate.
+   4. If no aggregates, simply select its alias in outer ast.
+   5. If given ast is memorized as mentioned in the above cases, return the 
mapping.
    */
-  private ASTNode processExpression(ASTNode astNode) {
+  private ASTNode getOuterAST(ASTNode astNode) {
     if (astNode == null) {
       return null;
     }
+    if (innerToOuterASTs.containsKey(new HashableASTNode(astNode))) {
+      return innerToOuterASTs.get(new HashableASTNode(astNode));
+    }
     if (isAggregateAST(astNode)) {
-      if (innerToOuterASTs.containsKey(new HashableASTNode(astNode))) {
-        return innerToOuterASTs.get(new HashableASTNode(astNode));
+      return processAggregate(astNode);
+    } else if (hasAggregate(astNode)) {
+      ASTNode outerAST = new ASTNode(astNode);
+      for (Node child : astNode.getChildren()) {
+        ASTNode childAST = (ASTNode) child;
+        if (hasAggregate(childAST)) {
+          outerAST.addChild(getOuterAST(childAST));
+        } else {
+          outerAST.addChild(childAST);
+        }
       }
-      ASTNode innerSelectASTWithoutAlias = MetastoreUtil.copyAST(astNode);
-      ASTNode innerSelectExprAST = new ASTNode(new 
CommonToken(HiveParser.TOK_SELEXPR));
-      innerSelectExprAST.addChild(innerSelectASTWithoutAlias);
-      String alias = aliasDecider.decideAlias(astNode);
-      ASTNode aliasNode = new ASTNode(new CommonToken(Identifier, alias));
-      innerSelectExprAST.addChild(aliasNode);
-      addToInnerSelectAST(innerSelectExprAST);
-      ASTNode dotAST = getDotAST(query.getCube().getName(), alias);
-      ASTNode outerAST = new ASTNode(new CommonToken(TOK_FUNCTION));
-      //TODO: take care or non-transitive aggregate functions
-      outerAST.addChild(new ASTNode(new CommonToken(Identifier, 
astNode.getChild(0).getText())));
-      outerAST.addChild(dotAST);
-      innerToOuterASTs.put(new HashableASTNode(innerSelectASTWithoutAlias), 
outerAST);
       return outerAST;
-    } else if (isTableColumnAST(astNode) || 
isNonAggregateFunctionAST(astNode)) {
-      if (innerToOuterASTs.containsKey(new HashableASTNode(astNode))) {
-        return innerToOuterASTs.get(new HashableASTNode(astNode));
-      }
+    } else {
       ASTNode innerSelectASTWithoutAlias = MetastoreUtil.copyAST(astNode);
       ASTNode innerSelectExprAST = new ASTNode(new 
CommonToken(HiveParser.TOK_SELEXPR));
       innerSelectExprAST.addChild(innerSelectASTWithoutAlias);
@@ -151,39 +217,24 @@ public class SingleFactMultiStorageHQLContext extends 
UnionHQLContext {
       ASTNode outerAST = getDotAST(query.getCube().getName(), alias);
       innerToOuterASTs.put(new HashableASTNode(innerSelectASTWithoutAlias), 
outerAST);
       return outerAST;
-    } else {
-      ASTNode outerHavingExpression = new ASTNode(astNode);
-      if (astNode.getChildren() != null) {
-        for (Node child : astNode.getChildren()) {
-          outerHavingExpression.addChild(processExpression((ASTNode) child));
-        }
-      }
-      return outerHavingExpression;
     }
   }
 
-  /**
-   * Transforms the inner query's AST so that aliases are used now instead of 
column names.
-   * Does so in-place, without creating new ASTNode instances.
-   * @param astNode inner query's AST Node to transform
-   * @return Transformed AST Node.
-   */
-  private ASTNode replaceAST(ASTNode astNode) {
-    if (astNode == null) {
-      return null;
-    }
-    if (isAggregateAST(astNode) || isTableColumnAST(astNode) || 
isNonAggregateFunctionAST(astNode)) {
-      if (innerToOuterASTs.containsKey(new HashableASTNode(astNode))) {
-        ASTNode ret = innerToOuterASTs.get(new HashableASTNode(astNode));
-        // Set parent null for quicker GC
-        astNode.setParent(null);
-        return ret;
-      }
-    }
-    for (int i = 0; i < astNode.getChildCount(); i++) {
-      astNode.setChild(i, replaceAST((ASTNode) astNode.getChild(i)));
-    }
-    return astNode;
+  private ASTNode processAggregate(ASTNode astNode) {
+    ASTNode innerSelectASTWithoutAlias = MetastoreUtil.copyAST(astNode);
+    ASTNode innerSelectExprAST = new ASTNode(new 
CommonToken(HiveParser.TOK_SELEXPR));
+    innerSelectExprAST.addChild(innerSelectASTWithoutAlias);
+    String alias = aliasDecider.decideAlias(astNode);
+    ASTNode aliasNode = new ASTNode(new CommonToken(Identifier, alias));
+    innerSelectExprAST.addChild(aliasNode);
+    addToInnerSelectAST(innerSelectExprAST);
+    ASTNode dotAST = getDotAST(query.getCube().getName(), alias);
+    ASTNode outerAST = new ASTNode(new CommonToken(TOK_FUNCTION));
+    //TODO: take care or non-transitive aggregate functions
+    outerAST.addChild(new ASTNode(new CommonToken(Identifier, 
astNode.getChild(0).getText())));
+    outerAST.addChild(dotAST);
+    innerToOuterASTs.put(new HashableASTNode(innerSelectASTWithoutAlias), 
outerAST);
+    return outerAST;
   }
 
   private void addToInnerSelectAST(ASTNode selectExprAST) {

http://git-wip-us.apache.org/repos/asf/lens/blob/a4b4b85d/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 2ab3fd0..86db011 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
@@ -651,6 +651,8 @@ public class CubeTestSetup {
       "concat(cubecity.name, \":\", cubestate.name)"));
     exprs.add(new ExprColumn(new FieldSchema("cityStateName", "String", "city 
state"), "City State",
       "concat('CityState:', cubecity.statename)"));
+    exprs.add(new ExprColumn(new FieldSchema("isIndia", "String", "is indian 
city/state"), "Is Indian City/state",
+      "cubecity.name == 'DELHI' OR cubestate.name == 'KARNATAKA' OR 
cubestate.name == 'MAHARASHTRA'"));
     exprs.add(new ExprColumn(new FieldSchema("cubeStateName", "String", 
"statename from cubestate"), "CubeState Name",
       "substr(cubestate.name, 5)"));
     exprs.add(new ExprColumn(new FieldSchema("substrdim2big1", "String", 
"substr of dim2big1"), "dim2big1 substr",

http://git-wip-us.apache.org/repos/asf/lens/blob/a4b4b85d/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionQueries.java
----------------------------------------------------------------------
diff --git 
a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionQueries.java 
b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionQueries.java
index 5eea123..f5657e5 100644
--- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionQueries.java
+++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestUnionQueries.java
@@ -209,7 +209,75 @@ public class TestUnionQueries extends TestQueryRewrite {
     } finally {
       getStorageToUpdatePeriodMap().clear();
     }
+  }
 
+  @Test
+  public void testDimAttrExpressionQuery() throws Exception {
+    Configuration conf = getConf();
+    conf.set(getValidStorageTablesKey("testfact"), "C1_testFact,C2_testFact");
+    conf.set(getValidUpdatePeriodsKey("testfact", "C1"), "DAILY,HOURLY");
+    conf.set(getValidUpdatePeriodsKey("testfact2", "C1"), "YEARLY");
+    conf.set(getValidUpdatePeriodsKey("testfact", "C2"), "MONTHLY,DAILY");
+    ArrayList<String> storages = Lists.newArrayList("c1_testfact", 
"c2_testfact");
+    getStorageToUpdatePeriodMap().put("c1_testfact", 
Lists.newArrayList(HOURLY, DAILY));
+    getStorageToUpdatePeriodMap().put("c2_testfact", 
Lists.newArrayList(MONTHLY));
+    StoragePartitionProvider provider = new StoragePartitionProvider() {
+      @Override
+      public Map<String, String> providePartitionsForStorage(String storage) {
+        return getWhereForMonthlyDailyAndHourly2monthsUnionQuery(storage);
+      }
+    };
+    String hqlQuery = rewrite("select asciicity as `City Name`, cityAndState 
as citystate, isIndia as isIndia,"
+      + " msr8, msr7 as `Third measure` "
+      + "from testCube where asciicity = 'c' and cityname = 'a' and zipcode = 
'b' and "
+      + TWO_MONTHS_RANGE_UPTO_HOURS, conf);
+    String expected = getExpectedUnionQuery(TEST_CUBE_NAME, storages, provider,
+      "SELECT testcube.alias0 as `City Name`, testcube.alias1 as citystate, 
testcube.alias2 as isIndia, "
+        + "sum(testcube.alias3) + max(testcube.alias4), "
+        + "case when sum(testcube.alias3) = 0 then 0 else 
sum(testcube.alias5)/sum(testcube.alias3) end "
+        + "as `Third Measure`",
+      null, " group by testcube.alias0, testcube.alias1, testcube.alias2",
+      "select ascii(cubecity.name) as `alias0`, concat(cubecity.name, \":\", 
cubestate.name) as alias1,"
+        + "cubecity.name == 'DELHI' OR cubestate.name == 'KARNATAKA' OR 
cubestate.name == 'MAHARASHTRA' as alias2,"
+        + "sum(testcube.msr2) as `alias3`, max(testcube.msr3) as `alias4`, "
+        + "sum(case when testcube.cityid = 'x' then testcube.msr21 else 
testcube.msr22 end) as `alias5`", " join "
+        + getDbName() + "c1_statetable cubestate on testcube.stateid = 
cubestate.id and (cubestate.dt = 'latest') join"
+        + getDbName() + "c1_citytable cubecity on testcube.cityid = 
cubecity.id and (cubecity.dt = 'latest')",
+      "ascii(cubecity.name) = 'c' and cubecity.name = 'a' and testcube.zipcode 
= 'b'",
+      " group by ascii(cubecity.name)), concat(cubecity.name, \":\", 
cubestate.name),"
+        + "cubecity.name == 'DELHI' OR cubestate.name == 'KARNATAKA' OR 
cubestate.name == 'MAHARASHTRA'");
+    compareQueries(hqlQuery, expected);
+  }
+
+  @Test
+  public void testNonAggregateOverAggregateFunction() throws Exception {
+    Configuration conf = getConf();
+    conf.set(getValidStorageTablesKey("testfact"), "C1_testFact,C2_testFact");
+    conf.set(getValidUpdatePeriodsKey("testfact", "C1"), "DAILY,HOURLY");
+    conf.set(getValidUpdatePeriodsKey("testfact2", "C1"), "YEARLY");
+    conf.set(getValidUpdatePeriodsKey("testfact", "C2"), "MONTHLY,DAILY");
+    ArrayList<String> storages = Lists.newArrayList("c1_testfact", 
"c2_testfact");
+    getStorageToUpdatePeriodMap().put("c1_testfact", 
Lists.newArrayList(HOURLY, DAILY));
+    getStorageToUpdatePeriodMap().put("c2_testfact", 
Lists.newArrayList(MONTHLY));
+    StoragePartitionProvider provider = new StoragePartitionProvider() {
+      @Override
+      public Map<String, String> providePartitionsForStorage(String storage) {
+        return getWhereForMonthlyDailyAndHourly2monthsUnionQuery(storage);
+      }
+    };
+    String hqlQuery = rewrite("select cityid as `City ID`, msr3 as `Measure 
3`, "
+      + "round(SUM(msr2)) as `Measure 2` from testCube" + " where "
+      + TWO_MONTHS_RANGE_UPTO_HOURS + " group by zipcode having msr4 > 10 
order by cityid desc, stateid asc, zipcode "
+      + "asc limit 5",
+      conf);
+    String expected = getExpectedUnionQuery(TEST_CUBE_NAME, storages, provider,
+      "SELECT testcube.alias0 as `City ID`,max(testcube.alias1) as `Measure 
3`,round(sum(testcube.alias2)) as "
+        + "`Measure 2` ", null, "group by testcube.alias3 having 
count(testcube.alias4) > 10 "
+        + "order by testcube.alias0 desc, testcube.alias5 asc, testcube.alias3 
asc limit 5",
+        "SELECT testcube.cityid as `alias0`, max(testcube.msr3) as `alias1`, 
sum(testcube.msr2) as `alias2`, "
+          + "testcube.zipcode as `alias3`, count(testcube .msr4) as `alias4`, 
(testcube.stateid) as `alias5` FROM ",
+      null, "GROUP BY testcube.zipcode");
+    compareQueries(hqlQuery, expected);
   }
 
   @Test

Reply via email to