Repository: incubator-lens
Updated Branches:
  refs/heads/master 2e5748a8c -> c8d38f7cf


LENS-648 : Fix NPE in CandidateTableResolver


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

Branch: refs/heads/master
Commit: c8d38f7cfdb630558339377e8b427c7fbee8b3cf
Parents: 2e5748a
Author: Amareshwari Sriramadasu <[email protected]>
Authored: Fri Jul 10 11:20:41 2015 +0530
Committer: Amareshwari Sriramadasu <[email protected]>
Committed: Fri Jul 10 11:20:41 2015 +0530

----------------------------------------------------------------------
 .../lens/cube/parse/CandidateTableResolver.java | 79 +++++++++++---------
 .../apache/lens/cube/parse/CubeTestSetup.java   | 16 +++-
 .../cube/parse/TestDenormalizationResolver.java | 14 ++++
 lens-cube/src/test/resources/log4j.properties   |  2 +-
 4 files changed, 73 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/c8d38f7c/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 1b5c09a..f861ec7 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
@@ -544,23 +544,28 @@ class CandidateTableResolver implements ContextRewriter {
       Set<Dimension> dimSet = dimColEntry.getValue();
       for (Dimension dim : dimSet) {
         OptionalDimCtx optdim = cubeql.getOptionalDimensionMap().get(dim);
-        candidatesReachableThroughRefs.addAll(optdim.requiredForCandidates);
+        if (optdim != null) {
+          candidatesReachableThroughRefs.addAll(optdim.requiredForCandidates);
+        }
       }
       for (Dimension dim : dimSet) {
-        for (CandidateTable candidate : removedCandidates.get(dim)) {
-          if (!candidatesReachableThroughRefs.contains(candidate)) {
-            if (candidate instanceof CandidateFact) {
-              if (cubeql.getCandidateFacts().contains(candidate)) {
-                LOG.info("Not considering fact:" + candidate + " as its 
required optional dims are not reachable");
-                cubeql.getCandidateFacts().remove(candidate);
-                cubeql.addFactPruningMsgs(((CandidateFact) candidate).fact,
+        if (removedCandidates.get(dim) != null) {
+          for (CandidateTable candidate : removedCandidates.get(dim)) {
+            if (!candidatesReachableThroughRefs.contains(candidate)) {
+              if (candidate instanceof CandidateFact) {
+                if (cubeql.getCandidateFacts().contains(candidate)) {
+                  LOG.info("Not considering fact:" + candidate + " as its 
required optional dims are not reachable");
+                  cubeql.getCandidateFacts().remove(candidate);
+                  cubeql.addFactPruningMsgs(((CandidateFact) candidate).fact,
+                    CandidateTablePruneCause.columnNotFound(col));
+                }
+              } else if 
(cubeql.getCandidateDimTables().containsKey(((CandidateDim) 
candidate).getBaseTable())) {
+                LOG.info("Not considering dimtable:" + candidate + " as its 
required optional dims are not reachable");
+                cubeql.getCandidateDimTables().get(((CandidateDim) 
candidate).getBaseTable()).remove(candidate);
+                cubeql.addDimPruningMsgs((Dimension) candidate.getBaseTable(),
+                  (CubeDimensionTable) candidate.getTable(),
                   CandidateTablePruneCause.columnNotFound(col));
               }
-            } else if 
(cubeql.getCandidateDimTables().containsKey(((CandidateDim) 
candidate).getBaseTable())) {
-              LOG.info("Not considering dimtable:" + candidate + " as its 
required optional dims are not reachable");
-              cubeql.getCandidateDimTables().get(((CandidateDim) 
candidate).getBaseTable()).remove(candidate);
-              cubeql.addDimPruningMsgs((Dimension) candidate.getBaseTable(), 
(CubeDimensionTable) candidate.getTable(),
-                CandidateTablePruneCause.columnNotFound(col));
             }
           }
         }
@@ -583,32 +588,34 @@ class CandidateTableResolver implements ContextRewriter {
       Set<Dimension> dimSet = exprColEntry.getValue();
       ExpressionContext ec = 
cubeql.getExprCtx().getExpressionContext(col.getExprCol(), col.getAlias());
       for (Dimension dim : dimSet) {
-        for (CandidateTable candidate : removedCandidates.get(dim)) {
-          // check if evaluable expressions of this candidate are no more 
evaluable because dimension is not reachable
-          // if no evaluable expressions exist, then remove the candidate
-          Iterator<ExprSpecContext> escIter = 
ec.getEvaluableExpressions().get(candidate).iterator();
-          while (escIter.hasNext()) {
-            ExprSpecContext esc = escIter.next();
-            if (esc.getExprDims().contains(dim)) {
-              escIter.remove();
+        if (removedCandidates.get(dim) != null) {
+          for (CandidateTable candidate : removedCandidates.get(dim)) {
+            // check if evaluable expressions of this candidate are no more 
evaluable because dimension is not reachable
+            // if no evaluable expressions exist, then remove the candidate
+            Iterator<ExprSpecContext> escIter = 
ec.getEvaluableExpressions().get(candidate).iterator();
+            while (escIter.hasNext()) {
+              ExprSpecContext esc = escIter.next();
+              if (esc.getExprDims().contains(dim)) {
+                escIter.remove();
+              }
             }
-          }
-          if (cubeql.getExprCtx().isEvaluable(col.getExprCol(), candidate)) {
-            // candidate has other evaluable expressions
-            continue;
-          }
-          if (candidate instanceof CandidateFact) {
-            if (cubeql.getCandidateFacts().contains(candidate)) {
-              LOG.info("Not considering fact:" + candidate + " as is not 
reachable through any optional dim");
-              cubeql.getCandidateFacts().remove(candidate);
-              cubeql.addFactPruningMsgs(((CandidateFact) candidate).fact,
+            if (cubeql.getExprCtx().isEvaluable(col.getExprCol(), candidate)) {
+              // candidate has other evaluable expressions
+              continue;
+            }
+            if (candidate instanceof CandidateFact) {
+              if (cubeql.getCandidateFacts().contains(candidate)) {
+                LOG.info("Not considering fact:" + candidate + " as is not 
reachable through any optional dim");
+                cubeql.getCandidateFacts().remove(candidate);
+                cubeql.addFactPruningMsgs(((CandidateFact) candidate).fact,
+                  
CandidateTablePruneCause.expressionNotEvaluable(col.getExprCol()));
+              }
+            } else if 
(cubeql.getCandidateDimTables().containsKey(((CandidateDim) 
candidate).getBaseTable())) {
+              LOG.info("Not considering dimtable:" + candidate + " as is not 
reachable through any optional dim");
+              cubeql.getCandidateDimTables().get(((CandidateDim) 
candidate).getBaseTable()).remove(candidate);
+              cubeql.addDimPruningMsgs((Dimension) candidate.getBaseTable(), 
(CubeDimensionTable) candidate.getTable(),
                 
CandidateTablePruneCause.expressionNotEvaluable(col.getExprCol()));
             }
-          } else if 
(cubeql.getCandidateDimTables().containsKey(((CandidateDim) 
candidate).getBaseTable())) {
-            LOG.info("Not considering dimtable:" + candidate + " as is not 
reachable through any optional dim");
-            cubeql.getCandidateDimTables().get(((CandidateDim) 
candidate).getBaseTable()).remove(candidate);
-            cubeql.addDimPruningMsgs((Dimension) candidate.getBaseTable(), 
(CubeDimensionTable) candidate.getTable(),
-              
CandidateTablePruneCause.expressionNotEvaluable(col.getExprCol()));
           }
         }
       }

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/c8d38f7c/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 432c5f4..f939858 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
@@ -745,6 +745,8 @@ public class CubeTestSetup {
     cubeDimensions2.add(new BaseDimAttribute(new FieldSchema("dim11", 
"string", "basedim")));
     cubeDimensions2.add(new ReferencedDimAtrribute(new FieldSchema("dim12", 
"int", "ref dim"), "Dim2 refer",
       new TableReference("testdim2", "id")));
+    cubeDimensions2.add(new ReferencedDimAtrribute(new FieldSchema("dim22", 
"int", "ref dim"), "Dim2 refer",
+      "dim2chain", "id", null, null, null));
 
     Map<String, String> cubeProperties = new HashMap<String, String>();
     
cubeProperties.put(MetastoreUtil.getCubeTimedDimensionListKey(BASE_CUBE_NAME),
@@ -829,6 +831,16 @@ public class CubeTestSetup {
             });
           }
         });
+        add(new JoinChain("dim2chain", "dim2chain", "dim2chain") {
+          {
+            addPath(new ArrayList<TableReference>() {
+              {
+                add(new TableReference("basecube", "dim2"));
+                add(new TableReference("testdim2", "id"));
+              }
+            });
+          }
+        });
       }
     };
 
@@ -861,6 +873,7 @@ public class CubeTestSetup {
     dimensions.add("dim2");
     dimensions.add("dim11");
     dimensions.add("dim12");
+    dimensions.add("dim22");
     dimensions.add("d_time");
     dimensions.add("test_time_dim");
     client.createDerivedCube(BASE_CUBE_NAME, DERIVED_CUBE_NAME2, measures, 
dimensions, derivedProperties, 10L);
@@ -941,7 +954,6 @@ public class CubeTestSetup {
     // create fact only with extra measures
     factName = "testFact2_BASE";
     factColumns = new ArrayList<FieldSchema>();
-    factColumns.add(new FieldSchema("msr11", "int", "first measure"));
     factColumns.add(new FieldSchema("msr12", "float", "second measure"));
 
     // add dimensions of the cube
@@ -988,6 +1000,8 @@ public class CubeTestSetup {
     factColumns.add(new FieldSchema("dim1", "string", "base dim"));
     factColumns.add(new FieldSchema("dim11", "string", "base dim"));
     factColumns.add(new FieldSchema("dim12", "string", "base dim"));
+    factColumns.add(new FieldSchema("dim22", "string", "base dim"));
+    factColumns.add(new FieldSchema("cityid", "int", "city id"));
 
     storageAggregatePeriods = new HashMap<String, Set<UpdatePeriod>>();
     updates = new HashSet<UpdatePeriod>();

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/c8d38f7c/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 8c3c219..901b95e 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
@@ -242,6 +242,20 @@ public class TestDenormalizationResolver extends 
TestQueryRewrite {
   }
 
   @Test
+  public void testCubeQueryWithOptionalDimsRemoved() throws Exception {
+    String hqlQuery = rewrite("select cityzip.code, dim22, msr11 from basecube 
where " + TWO_DAYS_RANGE,
+      conf);
+    String joinExpr = " join " + getDbName()
+      + "c1_citytable citydim on basecube.cityid = citydim.id and (citydim.dt 
= 'latest') "
+      + " join " + getDbName() + "c1_ziptable cityzip on citydim.zipcode = 
cityzip.code and (cityzip.dt = 'latest')";
+    String expected =
+      getExpectedQuery("basecube", "select cityzip.code, basecube.dim22, 
basecube.msr11 FROM ",
+        joinExpr, null, null, null,
+        getWhereForHourly2days("basecube", "C1_testfact2_raw_base"));
+    TestCubeRewriter.compareQueries(hqlQuery, expected);
+  }
+
+  @Test
   public void testDimensionQueryWithTwoRefCols() throws Exception {
     Configuration tConf = new Configuration(conf);
     tConf.set(CubeQueryConfUtil.DRIVER_SUPPORTED_STORAGES, "");

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/c8d38f7c/lens-cube/src/test/resources/log4j.properties
----------------------------------------------------------------------
diff --git a/lens-cube/src/test/resources/log4j.properties 
b/lens-cube/src/test/resources/log4j.properties
index a87fd8f..deaf1e1 100644
--- a/lens-cube/src/test/resources/log4j.properties
+++ b/lens-cube/src/test/resources/log4j.properties
@@ -32,5 +32,5 @@ log4j.appender.STDOUT.Threshold=WARN
 log4j.appender.TEST_LOG_FILE=org.apache.log4j.RollingFileAppender
 log4j.appender.TEST_LOG_FILE.File=target/test.log
 log4j.appender.TEST_LOG_FILE.layout=org.apache.log4j.PatternLayout
-log4j.appender.TEST_LOG_FILE.layout.ConversionPattern=%d [%t] %-5p %c - %m%n
+log4j.appender.TEST_LOG_FILE.layout.ConversionPattern=%d{dd MMM yyyy 
HH:mm:ss,SSS} [%t] %-5p %c %L %x - %m%n
 log4j.appender.TEST_LOG_FILE.Threshold=INFO

Reply via email to