LENS-1467: CubeQueryContext.getAllFilters is returning incorrect list of filters in case there is an "OR" in the filters
Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/ef6e59c6 Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/ef6e59c6 Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/ef6e59c6 Branch: refs/heads/master Commit: ef6e59c61d9b281332911a5d282723d29cd700c3 Parents: f4fdab0 Author: Rajat Khandelwal <pro...@apache.org> Authored: Wed Aug 30 16:10:54 2017 +0530 Committer: Rajat Khandelwal <rajatgupt...@gmail.com> Committed: Wed Aug 30 16:10:55 2017 +0530 ---------------------------------------------------------------------- .../lens/cube/parse/CubeQueryContext.java | 67 ++++++------------- .../cube/parse/StorageCandidateHQLContext.java | 3 +- .../apache/lens/cube/parse/join/JoinClause.java | 9 +++ .../lens/cube/parse/CubeQueryContextTest.java | 70 ++++++++++++++++++++ 4 files changed, 100 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/ef6e59c6/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java index 8b9583a..bff5c47 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java @@ -49,7 +49,6 @@ import org.apache.lens.cube.metadata.*; import org.apache.lens.cube.metadata.join.TableRelationship; import org.apache.lens.cube.parse.join.AutoJoinContext; import org.apache.lens.cube.parse.join.JoinClause; -import org.apache.lens.cube.parse.join.JoinTree; import org.apache.lens.cube.parse.join.JoinUtils; import org.apache.lens.server.api.error.LensException; @@ -1137,14 +1136,13 @@ public class CubeQueryContext extends TracksQueriedColumns implements QueryAST, return ImmutableSet.copyOf(this.queriedTimeDimCols); } - String getWhere(StorageCandidateHQLContext sc, AutoJoinContext autoJoinCtx, - ASTNode node, String cubeAlias, - boolean shouldReplaceDimFilter, String storageTable, - Map<Dimension, CandidateDim> dimToQuery) throws LensException { + String getWhere(StorageCandidateHQLContext sc, AutoJoinContext autoJoinCtx, String cubeAlias, + boolean shouldReplaceDimFilter, Map<Dimension, CandidateDim> dimToQuery) throws LensException { String whereString; if (autoJoinCtx != null && shouldReplaceDimFilter) { List<String> allfilters = new ArrayList<>(); - getAllFilters(node, cubeAlias, allfilters, autoJoinCtx.getJoinClause(sc.getStorageCandidate()), dimToQuery); + getAllFilters(sc.getQueryAst().getWhereAST(), cubeAlias, allfilters, + autoJoinCtx.getJoinClause(sc.getStorageCandidate()), dimToQuery); whereString = StringUtils.join(allfilters, " and "); } else { whereString = HQLParser.getString(sc.getQueryAst().getWhereAST()); @@ -1152,58 +1150,33 @@ public class CubeQueryContext extends TracksQueriedColumns implements QueryAST, return whereString; } - private void getAllFilters(ASTNode node, String cubeAlias, List<String> allFilters, - JoinClause joinClause, Map<Dimension, CandidateDim> dimToQuery) - throws LensException { - - if (node.getToken().getType() == HiveParser.KW_AND) { - // left child is and - if (node.getChild(0).getType() == HiveParser.KW_AND) { - // take right corresponding to right - String table = getTableFromFilterAST((ASTNode) node.getChild(1)); - allFilters.add(getFilter(table, cubeAlias, node, joinClause, 1, dimToQuery)); - } else if (node.getChildCount() > 1) { - for (int i = 0; i < node.getChildCount(); i++) { - String table = getTableFromFilterAST((ASTNode) node.getChild(i)); - allFilters.add(getFilter(table, cubeAlias, node, joinClause, i, dimToQuery)); - } + protected static void getAllFilters(ASTNode node, String cubeAlias, List<String> allFilters, JoinClause joinClause, + Map<Dimension, CandidateDim> dimToQuery) throws LensException { + if (node.getToken().getType() == HiveParser.KW_AND || node.getToken().getType() == HiveParser.TOK_WHERE) { + for (int i = 0; i < node.getChildCount(); i++) { + ASTNode child = (ASTNode) node.getChild(i); + getAllFilters(child, cubeAlias, allFilters, joinClause, dimToQuery); } - } else if (node.getParent() == null - && node.getToken().getType() != HiveParser.KW_AND - && node.getChild(0).getType() != HiveParser.KW_AND) { - // if node is the only child - allFilters.add(HQLParser.getString((ASTNode) node)); - } - for (int i = 0; i < node.getChildCount(); i++) { - ASTNode child = (ASTNode) node.getChild(i); - getAllFilters(child, cubeAlias, allFilters, joinClause, dimToQuery); + } else { + String table = getTableFromFilterAST(node); + allFilters.add(getFilter(table, cubeAlias, node, joinClause, dimToQuery)); } } - private String getFilter(String table, String cubeAlias, ASTNode node, JoinClause joinClause, - int index, Map<Dimension, CandidateDim> dimToQuery) + private static String getFilter(String table, String cubeAlias, ASTNode node, JoinClause joinClause, + Map<Dimension, CandidateDim> dimToQuery) throws LensException{ String filter; - if (table != null && !table.equals(cubeAlias) && getStarJoin(joinClause, table) != null) { + if (table != null && !table.equals(cubeAlias) && joinClause.getStarJoin(table) != null) { //rewrite dim filter to fact filter if its a star join with fact - filter = buildFactSubqueryFromDimFilter(getStarJoin(joinClause, table), - (ASTNode) node.getChild(index), table, dimToQuery, cubeAlias); + filter = buildFactSubqueryFromDimFilter(joinClause.getStarJoin(table), node, table, dimToQuery, cubeAlias); } else { - filter = HQLParser.getString((ASTNode) node.getChild(index)); + filter = HQLParser.getString(node); } return filter; } - private TableRelationship getStarJoin(JoinClause joinClause, String table) { - for (Map.Entry<TableRelationship, JoinTree> entry : joinClause.getJoinTree().getSubtrees().entrySet()) { - if (entry.getValue().getDepthFromRoot() == 1 && table.equals(entry.getValue().getAlias())) { - return entry.getKey(); - } - } - return null; - } - - private String getTableFromFilterAST(ASTNode node) { + private static String getTableFromFilterAST(ASTNode node) { if (node.getToken().getType() == HiveParser.DOT) { ASTNode n = HQLParser.findNodeByPath(node, TOK_TABLE_OR_COL, Identifier); @@ -1222,7 +1195,7 @@ public class CubeQueryContext extends TracksQueriedColumns implements QueryAST, return null; } - private String buildFactSubqueryFromDimFilter(TableRelationship tabRelation, ASTNode dimFilter, + private static String buildFactSubqueryFromDimFilter(TableRelationship tabRelation, ASTNode dimFilter, String dimAlias, Map<Dimension, CandidateDim> dimToQuery, String cubeAlias) throws LensException { http://git-wip-us.apache.org/repos/asf/lens/blob/ef6e59c6/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 494b08e..993aa4c 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 @@ -90,9 +90,8 @@ public class StorageCandidateHQLContext extends DimHQLContext { String qualifiedStorageTable = getStorageCandidate().getStorageName(); String storageTable = qualifiedStorageTable.substring(qualifiedStorageTable.indexOf(".") + 1); String where = getCubeQueryContext().getWhere(this, getCubeQueryContext().getAutoJoinCtx(), - getQueryAst().getWhereAST(), getCubeQueryContext().getAliasForTableName(getStorageCandidate().getBaseTable().getName()), - getCubeQueryContext().shouldReplaceDimFilterWithFactFilter(), storageTable, getDimsToQuery()); + getCubeQueryContext().shouldReplaceDimFilterWithFactFilter(), getDimsToQuery()); setWhere(where); } } http://git-wip-us.apache.org/repos/asf/lens/blob/ef6e59c6/lens-cube/src/main/java/org/apache/lens/cube/parse/join/JoinClause.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/join/JoinClause.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/join/JoinClause.java index 8661496..9e8f9bc 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/join/JoinClause.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/join/JoinClause.java @@ -50,6 +50,15 @@ public class JoinClause implements Comparable<JoinClause> { this.dimsInPath = dimsInPath; } + public TableRelationship getStarJoin(String table) { + for (Map.Entry<TableRelationship, JoinTree> entry : getJoinTree().getSubtrees().entrySet()) { + if (entry.getValue().getDepthFromRoot() == 1 && table.equals(entry.getValue().getAlias())) { + return entry.getKey(); + } + } + return null; + } + void initChainColumns() { for (List<TableRelationship> path : chain.values()) { for (TableRelationship edge : path) { http://git-wip-us.apache.org/repos/asf/lens/blob/ef6e59c6/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeQueryContextTest.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeQueryContextTest.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeQueryContextTest.java new file mode 100644 index 0000000..19e4f44 --- /dev/null +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeQueryContextTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.lens.cube.parse; + +import static org.testng.Assert.assertEquals; + +import static com.google.common.collect.Lists.newArrayList; + +import java.util.List; + +import org.apache.lens.cube.parse.join.JoinClause; + +import org.mockito.Mockito; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import com.google.common.collect.Maps; + +/** + * Created on 28/08/17. + */ +public class CubeQueryContextTest { + private JoinClause jc; + + @BeforeClass + public void intClass() { + jc = Mockito.mock(JoinClause.class); + Mockito.when(jc.getStarJoin(Mockito.anyString())).thenReturn(null); + } + + @DataProvider + public Object[][] testCases() { + return new Object[][]{ + {"testcube.x=1 and (testcube.dt=yesterday or (testcube.dt=today and testcube.pt=yesterday))", + newArrayList("((testcube.x) = 1)", + "(((testcube.dt) = yesterday) or (((testcube.dt) = today) and ((testcube.pt) = yesterday)))"), }, + {"testcube.x=1 and (testcube.dt=yesterday and (testcube.dt=today and testcube.pt=yesterday))", + newArrayList("((testcube.x) = 1)", "((testcube.dt) = yesterday)", + "((testcube.dt) = today)", "((testcube.pt) = yesterday)"), }, + {"testcube.x=1 and (testcube.dt = yesterday or " + + "(case when true and false then 1 else 0 end))", + newArrayList("((testcube.x) = 1)", + "(((testcube.dt) = yesterday) or case when ( true and false ) then 1 else 0 end)"), }, + }; + } + + @Test(dataProvider = "testCases") + public void testGetAllFilters(String expr, List<String> expected) throws Exception { + List<String> allFilters = newArrayList(); + CubeQueryContext.getAllFilters(HQLParser.parseExpr(expr), "testcube", allFilters, jc, Maps.newHashMap()); + assertEquals(allFilters, expected); + } +}
