Repository: lens Updated Branches: refs/heads/current-release-line 051412bec -> 6dca44661
LENS-1441: CandidateTableResolver should not add StorageCandidate if storage is not supported Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/c2a9c931 Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/c2a9c931 Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/c2a9c931 Branch: refs/heads/current-release-line Commit: c2a9c9316cc5d3a06b02a157722fa58f2757d47a Parents: 051412b Author: Rajat Khandelwal <pro...@apache.org> Authored: Tue Jun 20 16:08:00 2017 +0530 Committer: Rajat Khandelwal <rajatgupt...@gmail.com> Committed: Thu Jul 13 14:42:48 2017 +0530 ---------------------------------------------------------------------- .../lens/cube/parse/CandidateTableResolver.java | 24 +++++++++++++++++++- .../lens/cube/parse/CubeQueryRewriter.java | 2 +- .../lens/cube/parse/ExpressionResolver.java | 5 +++- .../lens/cube/parse/StorageTableResolver.java | 6 ----- .../cube/parse/TestDenormalizationResolver.java | 10 ++------ .../lens/cube/parse/TestTimeRangeResolver.java | 20 +++++++--------- .../server/query/QueryAPIErrorResponseTest.java | 1 + 7 files changed, 39 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java index f530650..be3b474 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java @@ -29,6 +29,7 @@ import org.apache.lens.cube.parse.ExpressionResolver.ExpressionContext; import org.apache.lens.server.api.error.LensException; import org.apache.commons.lang.StringUtils; +import org.apache.hadoop.conf.Configuration; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -48,8 +49,15 @@ import lombok.extern.slf4j.Slf4j; @Slf4j class CandidateTableResolver implements ContextRewriter { + private final List<String> supportedStorages; + private final boolean allStoragesSupported; private boolean checkForQueriedColumns = true; + public CandidateTableResolver(Configuration conf) { + this.supportedStorages = getSupportedStorages(conf); + this.allStoragesSupported = (supportedStorages == null); + } + @Override public void rewriteContext(CubeQueryContext cubeql) throws LensException { if (checkForQueriedColumns) { @@ -81,6 +89,18 @@ class CandidateTableResolver implements ContextRewriter { checkForQueriedColumns = true; } } + private List<String> getSupportedStorages(Configuration conf) { + String[] storages = conf.getStrings(CubeQueryConfUtil.DRIVER_SUPPORTED_STORAGES); + if (storages != null) { + return Arrays.asList(storages); + } + return null; + } + + private boolean isStorageSupportedOnDriver(String storage) { + return allStoragesSupported || supportedStorages.contains(storage); + } + private void populateCandidateTables(CubeQueryContext cubeql) throws LensException { if (cubeql.getCube() != null) { @@ -95,7 +115,9 @@ class CandidateTableResolver implements ContextRewriter { } else { for (String s : fact.getStorages()) { StorageCandidate sc = new StorageCandidate(cubeql.getCube(), fact, s, cubeql); - cubeql.getCandidates().add(sc); + if (isStorageSupportedOnDriver(sc.getStorageName())) { + cubeql.getCandidates().add(sc); + } } } } http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java index 0ef41f3..143b266 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java @@ -147,7 +147,7 @@ public class CubeQueryRewriter { rewriters.add(new AliasReplacer()); ExpressionResolver exprResolver = new ExpressionResolver(); DenormalizationResolver denormResolver = new DenormalizationResolver(); - CandidateTableResolver candidateTblResolver = new CandidateTableResolver(); + CandidateTableResolver candidateTblResolver = new CandidateTableResolver(conf); StorageTableResolver storageTableResolver = new StorageTableResolver(conf); LightestFactResolver lightestFactResolver = new LightestFactResolver(); http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/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 b1654d1..2403576 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 @@ -556,7 +556,10 @@ class ExpressionResolver implements ContextRewriter { log.info("Removing expression {} as all tables have non reachable fields", esc); iterator.remove(); removedEsc.add(esc); - break; + removed = true; + } + if (removed) { + continue; } //remove expressions which are not valid in the timerange queried // If an expression is defined as http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java index 3acd754..d7da8cb 100644 --- a/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java +++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java @@ -252,12 +252,6 @@ class StorageTableResolver implements ContextRewriter { if (c instanceof StorageCandidate) { StorageCandidate sc = (StorageCandidate) c; // first check: if the storage is supported on driver - if (!isStorageSupportedOnDriver(sc.getStorageName())) { - log.info("Skipping storage: {} as it is not supported", sc.getStorageName()); - cubeql.addStoragePruningMsg(sc, new CandidateTablePruneCause(CandidateTablePruneCode.UNSUPPORTED_STORAGE)); - it.remove(); - continue; - } String str = conf.get(CubeQueryConfUtil.getValidStorageTablesKey(sc.getFact().getName())); List<String> validFactStorageTables = StringUtils.isBlank(str) ? null : Arrays.asList(StringUtils.split(str.toLowerCase(), ",")); http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java index ffd0dec..9b217ae 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java @@ -200,16 +200,10 @@ public class TestDenormalizationResolver extends TestQueryRewrite { Map<Set<String>, List<CandidateTablePruneCause>> expected = Maps.newHashMap(); expected.put(newHashSet("c1_summary1", "c1_testfact", "c1_testfact2"), newArrayList(columnNotFound("dim2big2"))); - expected.put(newHashSet("c2_summary2", "c2_summary3", "c1_testfact2_raw", "" - + "c3_testfact2_raw", "c1_summary3", "c1_summary2"), + expected.put(newHashSet("c1_testfact2_raw", "c1_summary3", "c1_summary2"), newArrayList(new CandidateTablePruneCause(CandidateTablePruneCode.INVALID_DENORM_TABLE))); - expected.put(newHashSet("c0_b1b2fact1", "c0_testfact_continuous", "SEG[b1cube; b2cube]"), + expected.put(newHashSet("SEG[b1cube; b2cube]"), newArrayList(columnNotFound("msr2", "msr3"))); - expected.put(newHashSet("c2_summary2", "c2_summary3", "c2_summary4", "c4_testfact", "c2_summary1", - "c3_testfact", "c3_testfact2_raw", "c6_testfact", "c4_testfact2", "c5_testfact", "c99_cheapfact", - "c2_testfact", "c0_cheapfact", "c2_testfactmonthly", "c0_testfact"), - newArrayList(new CandidateTablePruneCause(CandidateTablePruneCode.UNSUPPORTED_STORAGE))); - Assert.assertEquals(enhanced, expected); } http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java ---------------------------------------------------------------------- diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java index 929fb46..181608f 100644 --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -21,9 +21,9 @@ package org.apache.lens.cube.parse; import static org.apache.lens.cube.metadata.DateFactory.*; import static org.apache.lens.cube.parse.CandidateTablePruneCause.CandidateTablePruneCode.TIME_RANGE_NOT_ANSWERABLE; -import static org.apache.lens.cube.parse.CandidateTablePruneCause.CandidateTablePruneCode.UNSUPPORTED_STORAGE; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; import java.util.*; @@ -41,8 +41,6 @@ import org.testng.annotations.Test; public class TestTimeRangeResolver extends TestQueryRewrite { - private final String cubeName = CubeTestSetup.TEST_CUBE_NAME; - private Configuration conf; @BeforeTest @@ -62,7 +60,7 @@ public class TestTimeRangeResolver extends TestQueryRewrite { @Test public void testFactValidity() throws ParseException, LensException, HiveException, ClassNotFoundException { - String query = "select msr2 from " + cubeName + " where " + LAST_YEAR_RANGE; + String query = "select msr2 from " + CubeTestSetup.TEST_CUBE_NAME + " where " + LAST_YEAR_RANGE; LensException e = getLensExceptionInRewrite(query, getConf()); assertEquals(e.getErrorInfo().getErrorName(), "NO_UNION_CANDIDATE_AVAILABLE"); } @@ -74,13 +72,11 @@ public class TestTimeRangeResolver extends TestQueryRewrite { getConf()); List<CandidateTablePruneCause> causes = findPruningMessagesForStorage("c3_testfact_deprecated", ctx.getStoragePruningMsgs()); - assertEquals(causes.size(), 1); - assertEquals(causes.get(0).getCause(), UNSUPPORTED_STORAGE); + assertTrue(causes.isEmpty()); causes = findPruningMessagesForStorage("c4_testfact_deprecated", ctx.getStoragePruningMsgs()); - assertEquals(causes.size(), 1); - assertEquals(causes.get(0).getCause(), UNSUPPORTED_STORAGE); + assertTrue(causes.isEmpty()); // testfact_deprecated's validity should be in between of both ranges. So both ranges should be in the invalid list // That would prove that parsing of properties has gone through successfully @@ -109,8 +105,8 @@ public class TestTimeRangeResolver extends TestQueryRewrite { /** * * @param stoargeName storageName_factName - * @param allStoragePruningMsgs - * @return + * @param allStoragePruningMsgs all pruning messages + * @return pruning messages for storagetable */ private static List<CandidateTablePruneCause> findPruningMessagesForStorage(String stoargeName, PruneCauses<Candidate> allStoragePruningMsgs) { @@ -121,6 +117,6 @@ public class TestTimeRangeResolver extends TestQueryRewrite { } } } - return new ArrayList<CandidateTablePruneCause>(); + return new ArrayList<>(); } } http://git-wip-us.apache.org/repos/asf/lens/blob/c2a9c931/lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java ---------------------------------------------------------------------- diff --git a/lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java b/lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java index 5409d21..82bb505 100644 --- a/lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java +++ b/lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java @@ -284,6 +284,7 @@ public class QueryAPIErrorResponseTest extends LensJerseyTest { //Create a StorageTable XStorageTables tables = new XStorageTables(); tables.getStorageTable().add(createStorageTblElement(testStorage, "DAILY")); + tables.getStorageTable().add(createStorageTblElement("mydb", "DAILY")); // for jdbc xFactTable.setStorageTables(tables); createFactFailFast(target, sessionId, xFactTable, mt);