Repository: lens Updated Branches: refs/heads/master b58749e20 -> a899577ec
LENS-1416 : Union query order by should work on column alias Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/a899577e Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/a899577e Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/a899577e Branch: refs/heads/master Commit: a899577ecaca09a3c237c072fd94550cc80711c6 Parents: b58749e Author: Sushil Mohanty <sushil.k.moha...@gmail.com> Authored: Wed May 3 16:23:33 2017 +0530 Committer: Rajat Khandelwal <rajatgupt...@gmail.com> Committed: Wed May 3 16:23:33 2017 +0530 ---------------------------------------------------------------------- lens-api/src/main/resources/lens-errors.conf | 5 ++++ .../lens/cube/error/LensCubeErrorCode.java | 1 + .../apache/lens/cube/parse/CandidateUtil.java | 27 ++++++++++++++++++++ .../cube/parse/StorageCandidateHQLContext.java | 23 +---------------- .../lens/cube/parse/UnionQueryWriter.java | 1 + .../lens/cube/parse/TestUnionQueries.java | 21 ++++++++++++--- 6 files changed, 52 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/a899577e/lens-api/src/main/resources/lens-errors.conf ---------------------------------------------------------------------- diff --git a/lens-api/src/main/resources/lens-errors.conf b/lens-api/src/main/resources/lens-errors.conf index e5536bb..43de1e9 100644 --- a/lens-api/src/main/resources/lens-errors.conf +++ b/lens-api/src/main/resources/lens-errors.conf @@ -345,6 +345,11 @@ lensCubeErrorsForQuery = [ errorMsg = "%s does not have any facts that can cover the queried measure set : %s" } + { + errorCode = 3036 + httpStatusCode = ${BAD_REQUEST} + errorMsg = "Order by column alias : %s shouldn't contain white space " + } ] http://git-wip-us.apache.org/repos/asf/lens/blob/a899577e/lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java b/lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java index babe3de..32b9db3 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java @@ -39,6 +39,7 @@ public enum LensCubeErrorCode { INVALID_TIME_RANGE(3014, 0), FROM_AFTER_TO(3015, 0), JOIN_TARGET_NOT_CUBE_TABLE(3016, 0), + ORDERBY_ALIAS_CONTAINING_WHITESPACE(3036, 0), // Error codes different for drivers CANNOT_USE_TIMERANGE_WRITER(3017, 100), NO_DEFAULT_AGGREGATE(3018, 200), http://git-wip-us.apache.org/repos/asf/lens/blob/a899577e/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java index 467ca0a..6ba46d6 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java @@ -23,6 +23,10 @@ import static org.apache.hadoop.hive.ql.parse.HiveParser.Identifier; import java.util.*; import java.util.stream.Collectors; +import org.apache.lens.cube.error.LensCubeErrorCode; +import org.apache.lens.server.api.error.LensException; + +import org.apache.hadoop.hive.ql.lib.Node; import org.apache.hadoop.hive.ql.parse.ASTNode; import org.apache.hadoop.hive.ql.parse.HiveParser; @@ -160,4 +164,27 @@ public final class CandidateUtil { static Set<String> getColumnsFromCandidates(Collection<? extends Candidate> scSet) { return scSet.stream().map(Candidate::getColumns).flatMap(Collection::stream).collect(Collectors.toSet()); } + + static void updateOrderByWithFinalAlias(ASTNode orderby, ASTNode select) throws LensException{ + if (orderby == null) { + return; + } + for (Node orderbyNode : orderby.getChildren()) { + ASTNode orderBychild = (ASTNode) orderbyNode; + for (Node selectNode : select.getChildren()) { + ASTNode selectChild = (ASTNode) selectNode; + if (selectChild.getChildCount() == 2) { + if (HQLParser.getString((ASTNode) selectChild.getChild(0)) + .equals(HQLParser.getString((ASTNode) orderBychild.getChild(0)))) { + ASTNode alias = new ASTNode((ASTNode) selectChild.getChild(1)); + if (!alias.toString().matches("\\S+")) { + throw new LensException(LensCubeErrorCode.ORDERBY_ALIAS_CONTAINING_WHITESPACE.getLensErrorInfo(), alias); + } + orderBychild.replaceChildren(0, 0, alias); + break; + } + } + } + } + } } http://git-wip-us.apache.org/repos/asf/lens/blob/a899577e/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 f5f468f..730b802 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 @@ -27,7 +27,6 @@ import org.apache.lens.cube.metadata.CubeInterface; import org.apache.lens.cube.metadata.Dimension; import org.apache.lens.server.api.error.LensException; -import org.apache.hadoop.hive.ql.lib.Node; import org.apache.hadoop.hive.ql.parse.ASTNode; import org.apache.hadoop.hive.ql.parse.HiveParser; @@ -136,29 +135,9 @@ public class StorageCandidateHQLContext extends DimHQLContext { // Check if the picked candidate is a StorageCandidate and in that case // update the selectAST with final alias. CandidateUtil.updateFinalAlias(queryAst.getSelectAST(), getCubeQueryContext()); - updateOrderByWithFinalAlias(queryAst.getOrderByAST(), queryAst.getSelectAST()); + CandidateUtil.updateOrderByWithFinalAlias(queryAst.getOrderByAST(), queryAst.getSelectAST()); setPrefix(getCubeQueryContext().getInsertClause()); } } } - - private void updateOrderByWithFinalAlias(ASTNode orderby, ASTNode select) { - if (orderby == null) { - return; - } - for (Node orderbyNode : orderby.getChildren()) { - ASTNode orderBychild = (ASTNode) orderbyNode; - for (Node selectNode : select.getChildren()) { - ASTNode selectChild = (ASTNode) selectNode; - if (selectChild.getChildCount() == 2) { - if (HQLParser.getString((ASTNode) selectChild.getChild(0)) - .equals(HQLParser.getString((ASTNode) orderBychild.getChild(0)))) { - ASTNode alias = new ASTNode((ASTNode) selectChild.getChild(1)); - orderBychild.replaceChildren(0, 0, alias); - break; - } - } - } - } - } } http://git-wip-us.apache.org/repos/asf/lens/blob/a899577e/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 1b4cc10..01b39e9 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 @@ -82,6 +82,7 @@ public class UnionQueryWriter extends SimpleHQLContext { CandidateUtil.updateFinalAlias(queryAst.getSelectAST(), cubeql); setPrefix(cubeql.getInsertClause()); setFrom(getFromString()); + CandidateUtil.updateOrderByWithFinalAlias(queryAst.getOrderByAST(), queryAst.getSelectAST()); } /** http://git-wip-us.apache.org/repos/asf/lens/blob/a899577e/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 8e65139..7ec3324 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 @@ -30,8 +30,10 @@ import static org.testng.Assert.*; import java.util.*; import java.util.stream.Collectors; +import org.apache.lens.cube.error.LensCubeErrorCode; import org.apache.lens.cube.error.NoCandidateFactAvailableException; import org.apache.lens.server.api.LensServerAPITestUtil; +import org.apache.lens.server.api.error.LensException; import org.apache.hadoop.conf.Configuration; @@ -165,7 +167,7 @@ public class TestUnionQueries extends TestQueryRewrite { "SELECT max(testcube.msr3) as `alias0`, sum(testcube.msr2) as `alias1`", null, null); compareQueries(hqlQuery, expected); - hqlQuery = rewrite("select zipcode, cityid as `City ID`, msr3 as `Measure 3`, msr4, " + hqlQuery = rewrite("select zipcode, cityid as `CityID`, msr3 as `Measure 3`, msr4, " + "SUM(msr2) as `Measure 2` from testCube where " + THREE_MONTHS_RANGE_UPTO_MONTH + " having msr4 > 10 order by cityid desc limit 5", conf); @@ -173,11 +175,22 @@ public class TestUnionQueries extends TestQueryRewrite { "SELECT (testcube.alias0) as `zipcode`, (testcube.alias1) as `City ID`, max((testcube.alias2)) " + "as `Measure 3`, count((testcube.alias3)) as `msr4`, sum((testcube.alias4)) as `Measure 2`", null, "group by testcube.alias0, testcube.alias1 " - + " having count(testcube.alias3) > 10 order by testcube.alias1 desc limit 5", + + " having count(testcube.alias3) > 10 order by cityid desc limit 5", "SELECT (testcube.zipcode) as `alias0`, (testcube.cityid) as `alias1`, max((testcube.msr3)) as `alias2`, " + "count((testcube.msr4)) as `alias3`, sum((testcube.msr2)) as `alias4`", null, "group by testcube.zipcode, testcube.cityid "); compareQueries(hqlQuery, expected); + + // Order by column with whitespace in alias should fail + try { + hqlQuery = rewrite("select zipcode, cityid as `City ID`, msr3 as `Measure 3`, msr4, " + + "SUM(msr2) as `Measure 2` from testCube where " + + THREE_MONTHS_RANGE_UPTO_MONTH + " having msr4 > 10 order by cityid desc limit 5", conf); + } catch (LensException e) { + assertEquals(e.getErrorCode(), + LensCubeErrorCode.ORDERBY_ALIAS_CONTAINING_WHITESPACE.getLensErrorInfo().getErrorCode()); + } + } finally { getStorageToUpdatePeriodMap().clear(); } @@ -274,13 +287,13 @@ public class TestUnionQueries extends TestQueryRewrite { } }; - String hqlQuery = rewrite("select cityid as `City ID`, msr3 as `Measure 3`, " + String hqlQuery = rewrite("select cityid as `CityID`, msr3 as `Measure 3`, " + "round(SUM(msr2)) as `Measure 2` from testCube" + " where " + THREE_MONTHS_RANGE_UPTO_MONTH + " group by cityid having msr3 > 10 order by cityid desc 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.alias0) HAVING (max((testcube.alias1)) > 10) " - + "ORDER BY testcube.alias0 desc LIMIT 5", + + "ORDER BY cityid desc LIMIT 5", "SELECT (testcube.cityid) as `alias0`, max((testcube.msr3)) as `alias1`, " + "sum((testcube.msr2)) as `alias2` FROM ", null, "GROUP BY testcube.cityid");
