TAJO-1099: LogicalPlanner::convertDataType causes NPE in some cases.

Closes #185
apache · Oct 8, 2014 · 561edf3 · 561edf3
1 parent f42baa0
commit 561edf3
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 14 deletions.
diff --git a/CHANGES b/CHANGES
@@ -163,6 +163,9 @@ Release 0.9.0 - unreleased
 
   BUG FIXES
 
+    TAJO-1099: LogicalPlanner::convertDataType causes NPE in some cases.
+    (hyunsik)
+
     TAJO-1102: Self-join with a partitioned table returns wrong result data.
     (Hyoungjun Kim)
 

diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/eval/SimpleEvalNodeVisitor.java b/tajo-core/src/main/java/org/apache/tajo/engine/eval/SimpleEvalNodeVisitor.java
@@ -18,6 +18,7 @@
 
 package org.apache.tajo.engine.eval;
 
+import com.google.common.base.Preconditions;
 import org.apache.tajo.exception.UnsupportedException;
 
 import java.util.Stack;
@@ -29,6 +30,8 @@
 public abstract class SimpleEvalNodeVisitor<CONTEXT> {
 
   public EvalNode visit(CONTEXT context, EvalNode evalNode, Stack<EvalNode> stack) {
+    Preconditions.checkNotNull(evalNode);
+
     EvalNode result;
 
     if (evalNode instanceof UnaryEval) {

diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/optimizer/eval/EvalTreeOptimizer.java b/tajo-core/src/main/java/org/apache/tajo/engine/optimizer/eval/EvalTreeOptimizer.java
@@ -18,6 +18,7 @@
 
 package org.apache.tajo.engine.optimizer.eval;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -65,6 +66,7 @@ public int compare(EvalTreeOptimizationRule o1, EvalTreeOptimizationRule o2) {
   }
 
   public EvalNode optimize(LogicalPlanner.PlanContext context, EvalNode node) {
+    Preconditions.checkNotNull(node);
 
     EvalNode optimized = node;
     for (EvalTreeOptimizationRule rule : rules) {

diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java b/tajo-core/src/main/java/org/apache/tajo/engine/planner/LogicalPlanPreprocessor.java
@@ -162,7 +162,9 @@ public LogicalNode visitProjection(LogicalPlanner.PlanContext ctx, Stack<Expr> s
       throws PlanningException {
     // If Non-from statement, it immediately returns.
     if (!expr.hasChild()) {
-      return ctx.plan.createNode(EvalExprNode.class);
+      EvalExprNode exprNode = ctx.plan.createNode(EvalExprNode.class);
+      exprNode.setTargets(buildTargets(ctx, expr.getNamedExprs()));
+      return exprNode;
     }
 
     stack.push(expr); // <--- push
@@ -200,20 +202,8 @@ public LogicalNode visitProjection(LogicalPlanner.PlanContext ctx, Stack<Expr> s
       }
     }
 
-    Target [] targets;
-    targets = new Target[projectTargetExprs.length];
+    Target [] targets = buildTargets(ctx, expr.getNamedExprs());
 
-    for (int i = 0; i < expr.getNamedExprs().length; i++) {
-      NamedExpr namedExpr = expr.getNamedExprs()[i];
-      TajoDataTypes.DataType dataType = typeDeterminant.determineDataType(ctx, namedExpr.getExpr());
-
-      if (namedExpr.hasAlias()) {
-        targets[i] = new Target(new FieldEval(new Column(namedExpr.getAlias(), dataType)));
-      } else {
-        String generatedName = ctx.plan.generateUniqueColumnName(namedExpr.getExpr());
-        targets[i] = new Target(new FieldEval(new Column(generatedName, dataType)));
-      }
-    }
     stack.pop(); // <--- Pop
 
     ProjectionNode projectionNode = ctx.plan.createNode(ProjectionNode.class);
@@ -224,6 +214,22 @@ public LogicalNode visitProjection(LogicalPlanner.PlanContext ctx, Stack<Expr> s
     return projectionNode;
   }
 
+  private Target [] buildTargets(LogicalPlanner.PlanContext context, NamedExpr [] exprs) throws PlanningException {
+    Target [] targets = new Target[exprs.length];
+    for (int i = 0; i < exprs.length; i++) {
+      NamedExpr namedExpr = exprs[i];
+      TajoDataTypes.DataType dataType = typeDeterminant.determineDataType(context, namedExpr.getExpr());
+
+      if (namedExpr.hasAlias()) {
+        targets[i] = new Target(new FieldEval(new Column(namedExpr.getAlias(), dataType)));
+      } else {
+        String generatedName = context.plan.generateUniqueColumnName(namedExpr.getExpr());
+        targets[i] = new Target(new FieldEval(new Column(generatedName, dataType)));
+      }
+    }
+    return targets;
+  }
+
   @Override
   public LogicalNode visitLimit(LogicalPlanner.PlanContext ctx, Stack<Expr> stack, Limit expr)
       throws PlanningException {

diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/planner/TypeDeterminant.java b/tajo-core/src/main/java/org/apache/tajo/engine/planner/TypeDeterminant.java
@@ -18,6 +18,7 @@
 
 package org.apache.tajo.engine.planner;
 
+import com.google.common.base.Preconditions;
 import org.apache.tajo.algebra.*;
 import org.apache.tajo.catalog.CatalogService;
 import org.apache.tajo.catalog.CatalogUtil;
@@ -77,6 +78,10 @@ public DataType visitBinaryOperator(LogicalPlanner.PlanContext ctx, Stack<Expr>
   }
 
   public DataType computeBinaryType(OpType type, DataType lhsDataType, DataType rhsDataType) throws PlanningException {
+    Preconditions.checkNotNull(type);
+    Preconditions.checkNotNull(lhsDataType);
+    Preconditions.checkNotNull(rhsDataType);
+
     if(OpType.isLogicalType(type) || OpType.isComparisonType(type)) {
       return BOOL_TYPE;
     } else if (OpType.isArithmeticType(type)) {
@@ -301,4 +306,16 @@ public DataType visitTimeLiteral(LogicalPlanner.PlanContext ctx, Stack<Expr> sta
       throws PlanningException {
     return CatalogUtil.newSimpleDataType(TajoDataTypes.Type.TIME);
   }
+
+  @Override
+  public DataType visitDateLiteral(LogicalPlanner.PlanContext ctx, Stack<Expr> stack, DateLiteral expr)
+      throws PlanningException {
+    return CatalogUtil.newSimpleDataType(TajoDataTypes.Type.DATE);
+  }
+
+  @Override
+  public DataType visitIntervalLiteral(LogicalPlanner.PlanContext ctx, Stack<Expr> stack, IntervalLiteral expr)
+      throws PlanningException {
+    return CatalogUtil.newSimpleDataType(TajoDataTypes.Type.INTERVAL);
+  }
 }
diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/planner/logical/EvalExprNode.java b/tajo-core/src/main/java/org/apache/tajo/engine/planner/logical/EvalExprNode.java
@@ -23,6 +23,7 @@
 
 import com.google.gson.annotations.Expose;
 import org.apache.tajo.engine.planner.PlanString;
+import org.apache.tajo.engine.planner.PlannerUtil;
 import org.apache.tajo.engine.planner.Target;
 import org.apache.tajo.util.TUtil;
 
@@ -41,6 +42,7 @@ public boolean hasTargets() {
   @Override
   public void setTargets(Target[] targets) {
     this.exprs = targets;
+    setOutSchema(PlannerUtil.targetToSchema(targets));
   }
 
   @Override