DRILL-635: Fix run-time code generation bug for case expression, when the holder of return type does not have value field.

3 files changed, 40 insertions, 28 deletions

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
index d700bf3f0..731ab6b26 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
@@ -162,10 +162,10 @@ public class EvaluationVisitor {
         if (thenExpr.isOptional()) {
           JConditional newCond = jc._then()._if(thenExpr.getIsSet());
           JBlock b = newCond._then();
-          b.assign(output.getValue(), thenExpr.getValue());
+          b.assign(output.getHolder(), thenExpr.getHolder());
           b.assign(output.getIsSet(), thenExpr.getIsSet());
         } else {
-          jc._then().assign(output.getValue(), thenExpr.getValue());
+          jc._then().assign(output.getHolder(), thenExpr.getHolder());
         }
 
       }
@@ -174,11 +174,10 @@ public class EvaluationVisitor {
       if (elseExpr.isOptional()) {
         JConditional newCond = jc._else()._if(elseExpr.getIsSet());
         JBlock b = newCond._then();
-        b.assign(output.getValue(), elseExpr.getValue());
+        b.assign(output.getHolder(), elseExpr.getHolder());
         b.assign(output.getIsSet(), elseExpr.getIsSet());
       } else {
-        jc._else().assign(output.getValue(), elseExpr.getValue());
-
+        jc._else().assign(output.getHolder(), elseExpr.getHolder());
       }
       local.add(conditionalBlock);
       return output;
@@ -336,7 +335,7 @@ public class EvaluationVisitor {
 
       JExpression componentVariable = indexVariable.shrz(JExpr.lit(16));
       if (e.isSuperReader()) {
-        vv1 =  ((JExpression) vv1.component(componentVariable));
+        vv1 =  (vv1.component(componentVariable));
         indexVariable = indexVariable.band(JExpr.lit((int) Character.MAX_VALUE));
       }
 
@@ -467,7 +466,7 @@ public class EvaluationVisitor {
       JType holderType = generator.getHolderType(majorType);
       JVar var = generator.declareClassField("string", holderType);
       JExpression stringLiteral = JExpr.lit(e.value);
-      setup.assign(var, ((JClass)generator.getModel().ref(ValueHolderHelper.class)).staticInvoke("getVarCharHolder").arg(stringLiteral));
+      setup.assign(var, generator.getModel().ref(ValueHolderHelper.class).staticInvoke("getVarCharHolder").arg(stringLiteral));
       return new HoldingContainer(majorType, var, null, null);
     }
 
@@ -480,7 +479,7 @@ public class EvaluationVisitor {
       JVar var = generator.declareClassField("intervalday", holderType);
       JExpression dayLiteral = JExpr.lit(e.getIntervalDay());
       JExpression millisLiteral = JExpr.lit(e.getIntervalMillis());
-      setup.assign(var, ((JClass)generator.getModel().ref(ValueHolderHelper.class)).staticInvoke("getIntervalDayHolder").arg(dayLiteral).arg(millisLiteral));
+      setup.assign(var, generator.getModel().ref(ValueHolderHelper.class).staticInvoke("getIntervalDayHolder").arg(dayLiteral).arg(millisLiteral));
       return new HoldingContainer(majorType, var, null, null);
     }
 
@@ -493,7 +492,7 @@ public class EvaluationVisitor {
       JExpression valueLiteral = JExpr.lit(e.getIntFromDecimal());
       JExpression scaleLiteral = JExpr.lit(e.getScale());
       JExpression precisionLiteral = JExpr.lit(e.getPrecision());
-      setup.assign(var, ((JClass)generator.getModel().ref(ValueHolderHelper.class)).staticInvoke("getDecimal9Holder").arg(valueLiteral).arg(scaleLiteral).arg(precisionLiteral));
+      setup.assign(var, generator.getModel().ref(ValueHolderHelper.class).staticInvoke("getDecimal9Holder").arg(valueLiteral).arg(scaleLiteral).arg(precisionLiteral));
       return new HoldingContainer(majorType, var, null, null);
     }
 
@@ -506,7 +505,7 @@ public class EvaluationVisitor {
       JExpression valueLiteral = JExpr.lit(e.getLongFromDecimal());
       JExpression scaleLiteral = JExpr.lit(e.getScale());
       JExpression precisionLiteral = JExpr.lit(e.getPrecision());
-      setup.assign(var, ((JClass)generator.getModel().ref(ValueHolderHelper.class)).staticInvoke("getDecimal18Holder").arg(valueLiteral).arg(scaleLiteral).arg(precisionLiteral));
+      setup.assign(var, generator.getModel().ref(ValueHolderHelper.class).staticInvoke("getDecimal18Holder").arg(valueLiteral).arg(scaleLiteral).arg(precisionLiteral));
       return new HoldingContainer(majorType, var, null, null);
     }
 
@@ -518,7 +517,7 @@ public class EvaluationVisitor {
       JType holderType = generator.getHolderType(majorType);
       JVar var = generator.declareClassField("dec28", holderType);
       JExpression stringLiteral = JExpr.lit(e.getBigDecimal().toString());
-      setup.assign(var, ((JClass)generator.getModel().ref(ValueHolderHelper.class)).staticInvoke("getDecimal28Holder").arg(stringLiteral));
+      setup.assign(var, generator.getModel().ref(ValueHolderHelper.class).staticInvoke("getDecimal28Holder").arg(stringLiteral));
       return new HoldingContainer(majorType, var, null, null);
     }
 
@@ -530,7 +529,7 @@ public class EvaluationVisitor {
       JType holderType = generator.getHolderType(majorType);
       JVar var = generator.declareClassField("dec38", holderType);
       JExpression stringLiteral = JExpr.lit(e.getBigDecimal().toString());
-      setup.assign(var, ((JClass)generator.getModel().ref(ValueHolderHelper.class)).staticInvoke("getVarCharHolder").arg(stringLiteral));
+      setup.assign(var, generator.getModel().ref(ValueHolderHelper.class).staticInvoke("getVarCharHolder").arg(stringLiteral));
       return new HoldingContainer(majorType, var, null, null);
     }
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
index 0460ec444..01fc514d2 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
@@ -40,13 +40,13 @@ import com.sun.codemodel.JVar;
 
 class DrillSimpleFuncHolder extends DrillFuncHolder{
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillSimpleFuncHolder.class);
-  
+
   private final String setupBody;
   private final String evalBody;
   private final String resetBody;
   private final String cleanupBody;
-  
-  
+
+
   public DrillSimpleFuncHolder(FunctionScope scope, NullHandling nullHandling, boolean isBinaryCommutative, boolean isRandom,
       String[] registeredNames, ValueReference[] parameters, ValueReference returnValue, WorkspaceReference[] workspaceVars,
       Map<String, String> methods, List<String> imports) {
@@ -56,16 +56,16 @@ class DrillSimpleFuncHolder extends DrillFuncHolder{
     resetBody = methods.get("reset");
     cleanupBody = methods.get("cleanup");
     Preconditions.checkNotNull(evalBody);
-    
+
   }
 
   public boolean isNested(){
     return false;
   }
-  
+
   public HoldingContainer renderEnd(ClassGenerator<?> g, HoldingContainer[] inputVariables, JVar[]  workspaceJVars){
-    //If the function's annotation specifies a parameter has to be constant expression, but the HoldingContainer 
-    //for the argument is not, then raise exception.    
+    //If the function's annotation specifies a parameter has to be constant expression, but the HoldingContainer
+    //for the argument is not, then raise exception.
     for(int i =0; i < inputVariables.length; i++){
       if (parameters[i].isConstant && !inputVariables[i].isConstant()) {
         throw new DrillRuntimeException(String.format("The argument '%s' of Function '%s' has to be constant!", parameters[i].name, this.getRegisteredNames()[0]));
@@ -77,11 +77,11 @@ class DrillSimpleFuncHolder extends DrillFuncHolder{
     generateBody(g, BlockType.CLEANUP, cleanupBody, null, workspaceJVars, false);
     return c;
   }
- 
+
   protected HoldingContainer generateEvalBody(ClassGenerator<?> g, HoldingContainer[] inputVariables, String body, JVar[] workspaceJVars) {
-    
-    //g.getBlock().directStatement(String.format("//---- start of eval portion of %s function. ----//", functionName));
-    
+
+    g.getEvalBlock().directStatement(String.format("//---- start of eval portion of %s function. ----//", registeredNames[0]));
+
     JBlock sub = new JBlock(true, true);
     JBlock topSub = sub;
     HoldingContainer out = null;
@@ -99,7 +99,7 @@ class DrillSimpleFuncHolder extends DrillFuncHolder{
           }
         }
       }
-      
+
       if(e != null){
         // if at least one expression must be checked, set up the conditional.
         returnValueType = returnValue.type.toBuilder().setMode(DataMode.OPTIONAL).build();
@@ -110,18 +110,21 @@ class DrillSimpleFuncHolder extends DrillFuncHolder{
         sub = jc._else();
       }
     }
-    
+
     if(out == null) out = g.declare(returnValueType);
-    
+
     // add the subblock after the out declaration.
     g.getEvalBlock().add(topSub);
-    
-    
+
+
     JVar internalOutput = sub.decl(JMod.FINAL, g.getHolderType(returnValueType), returnValue.name, JExpr._new(g.getHolderType(returnValueType)));
     addProtectedBlock(g, sub, body, inputVariables, workspaceJVars, false);
     if (sub != topSub) sub.assign(internalOutput.ref("isSet"),JExpr.lit(1));// Assign null if NULL_IF_NULL mode
     sub.assign(out.getHolder(), internalOutput);
     if (sub != topSub) sub.assign(internalOutput.ref("isSet"),JExpr.lit(1));// Assign null if NULL_IF_NULL mode
+
+    g.getEvalBlock().directStatement(String.format("//---- end of eval portion of %s function. ----//", registeredNames[0]));
+
     return out;
   }
 
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
index 69552f988..99940f44b 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
@@ -39,6 +39,16 @@ public class TestExampleQueries extends BaseTestQuery{
   }
 
   @Test
+  public void testCaseReturnValueVarChar() throws Exception{
+    test("select case when employee_id < 1000 then 'ABC' else 'DEF' end from cp.`employee.json` limit 5");
+  }
+
+  @Test
+  public void testCaseReturnValueBigInt() throws Exception{
+    test("select case when employee_id < 1000 then 1000 else 2000 end from cp.`employee.json` limit 5" );
+  }
+
+  @Test
   public void testSelectWithLimit() throws Exception{
     test("select employee_id,  first_name, last_name from cp.`employee.json` order by employee_id limit 5 offset 10");
   }