Repository: incubator-drill Updated Branches: refs/heads/master e7a486d78 -> ea72a380e
DRILL-635: Fix run-time code generation bug for case expression, when the holder of return type does not have value field. Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/8e1865c0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/8e1865c0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/8e1865c0 Branch: refs/heads/master Commit: 8e1865c0e4986b4bc361ef295f77ce9e9833c94a Parents: a4a02ef Author: Jinfeng Ni <[email protected]> Authored: Mon May 12 13:19:55 2014 -0700 Committer: Aditya Kishore <[email protected]> Committed: Tue May 13 15:27:38 2014 -0700 ---------------------------------------------------------------------- .../drill/exec/expr/EvaluationVisitor.java | 23 ++++++------- .../exec/expr/fn/DrillSimpleFuncHolder.java | 35 +++++++++++--------- .../org/apache/drill/TestExampleQueries.java | 10 ++++++ 3 files changed, 40 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8e1865c0/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java ---------------------------------------------------------------------- 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 d700bf3..731ab6b 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); } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8e1865c0/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ---------------------------------------------------------------------- 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 0460ec4..01fc514 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; } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/8e1865c0/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java ---------------------------------------------------------------------- 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 69552f9..99940f4 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"); }
