This is an automated email from the ASF dual-hosted git repository. arina pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push: new ddcb578 DRILL-7307: casthigh for decimal type can lead to the issues with VarDecimalHolder ddcb578 is described below commit ddcb57865f388f2588df35bcad3fd52e69d22db4 Author: Dmytriy Grinchenko <dmytriy.grinche...@gmail.com> AuthorDate: Wed Jul 3 16:11:42 2019 +0300 DRILL-7307: casthigh for decimal type can lead to the issues with VarDecimalHolder - Fixed code-gen for VarDecimal type - Fixed code-gen issue with nullable holders for simple cast functions with passed constants as arguments. - Code-gen now honnoring DataType.Optional type defined by UDF for NULL-IF-NULL functions. --- .../apache/drill/exec/expr/fn/HiveFuncHolder.java | 2 +- .../src/main/codegen/templates/CastHigh.java | 10 +- .../apache/drill/exec/expr/EvaluationVisitor.java | 276 +++++---------------- .../drill/exec/expr/fn/AbstractFuncHolder.java | 4 +- .../drill/exec/expr/fn/DrillAggFuncHolder.java | 3 +- .../expr/fn/DrillComplexWriterAggFuncHolder.java | 3 +- .../exec/expr/fn/DrillComplexWriterFuncHolder.java | 4 +- .../drill/exec/expr/fn/DrillSimpleFuncHolder.java | 11 +- .../expr/fn/output/DecimalReturnTypeInference.java | 9 +- .../drill/exec/sql/TestSimpleCastFunctions.java | 34 +++ 10 files changed, 123 insertions(+), 233 deletions(-) diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java index 80f299e..7ac1460 100644 --- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java +++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java @@ -145,7 +145,7 @@ public class HiveFuncHolder extends AbstractFuncHolder { @Override public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables, - JVar[] workspaceJVars, FieldReference fieldReference) { + JVar[] workspaceJVars, FunctionHolderExpression holderExpr) { generateSetup(classGenerator, workspaceJVars); return generateEval(classGenerator, inputVariables, workspaceJVars); } diff --git a/exec/java-exec/src/main/codegen/templates/CastHigh.java b/exec/java-exec/src/main/codegen/templates/CastHigh.java index 1a876c8..08f0efe 100644 --- a/exec/java-exec/src/main/codegen/templates/CastHigh.java +++ b/exec/java-exec/src/main/codegen/templates/CastHigh.java @@ -60,11 +60,17 @@ public class CastHighFunctions { public void setup() {} public void eval() { - <#if type.value> + <#if type.from.contains("VarDecimal")> + out.buffer = (DrillBuf) in.buffer; + out.start = (int) in.start; + out.scale = (int) in.scale; + out.precision = (int) in.precision; + out.end = (int) in.end; + <#elseif type.value> out.value = (double) in.value; <#else> out = in; - </#if> + </#if> } } </#list> 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 54644bf..1f8a779 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 @@ -65,6 +65,7 @@ import org.apache.drill.exec.compile.sig.ConstantExpressionIdentifier; import org.apache.drill.exec.compile.sig.GeneratorMapping; import org.apache.drill.exec.compile.sig.MappingSet; import org.apache.drill.exec.expr.ClassGenerator.HoldingContainer; +import org.apache.drill.exec.expr.annotations.FunctionTemplate; import org.apache.drill.exec.expr.fn.AbstractFuncHolder; import org.apache.drill.exec.expr.holders.ValueHolder; import org.apache.drill.exec.physical.impl.filter.ReturnValueExpression; @@ -105,6 +106,13 @@ public class EvaluationVisitor { return e.accept(new CSEFilter(constantBoundaries), generator); } + /** + * Callback function for the expression visitor + */ + private interface VisitorCallback { + HoldingContainer getHolder(); + } + private class ExpressionHolder { private LogicalExpression expression; private GeneratorMapping mapping; @@ -203,7 +211,7 @@ public class EvaluationVisitor { generator.getMappingSet().exitChild(); } - return holder.renderEnd(generator, args, workspaceVars, holderExpr.getFieldReference()); + return holder.renderEnd(generator, args, workspaceVars, holderExpr); } @Override @@ -1111,6 +1119,8 @@ public class EvaluationVisitor { } } + + private class ConstantFilter extends EvalVisitor { private Set<LogicalExpression> constantBoundaries; @@ -1126,327 +1136,157 @@ public class EvaluationVisitor { + "It should have been converted to FunctionHolderExpression in materialization"); } - @Override - public HoldingContainer visitFunctionHolderExpression(FunctionHolderExpression e, ClassGenerator<?> generator) - throws RuntimeException { + private HoldingContainer visitExpression(LogicalExpression e, ClassGenerator<?> generator, + VisitorCallback visitorCallback) throws RuntimeException { if (constantBoundaries.contains(e)) { generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitFunctionHolderExpression(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); + HoldingContainer c = visitorCallback.getHolder(); + + return renderConstantExpression(generator, c, e); } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitFunctionHolderExpression(e, generator).setConstant(true); + return visitorCallback.getHolder().setConstant(true); } else { - return super.visitFunctionHolderExpression(e, generator); + return visitorCallback.getHolder(); } } @Override + public HoldingContainer visitFunctionHolderExpression(FunctionHolderExpression e, ClassGenerator<?> generator) + throws RuntimeException { + return visitExpression(e, generator, () -> super.visitFunctionHolderExpression(e, generator)); + } + + @Override public HoldingContainer visitBooleanOperator(BooleanOperator e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitBooleanOperator(e, generator); - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitBooleanOperator(e, generator).setConstant(true); - } else { - return super.visitBooleanOperator(e, generator); - } + return visitExpression(e, generator, () -> super.visitBooleanOperator(e, generator)); } @Override public HoldingContainer visitIfExpression(IfExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitIfExpression(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitIfExpression(e, generator).setConstant(true); - } else { - return super.visitIfExpression(e, generator); - } + return visitExpression(e, generator, () -> super.visitIfExpression(e, generator)); } @Override public HoldingContainer visitSchemaPath(SchemaPath e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitSchemaPath(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitSchemaPath(e, generator).setConstant(true); - } else { - return super.visitSchemaPath(e, generator); - } + return visitExpression(e, generator, () -> super.visitSchemaPath(e, generator)); } @Override public HoldingContainer visitLongConstant(LongExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitLongConstant(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitLongConstant(e, generator).setConstant(true); - } else { - return super.visitLongConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitLongConstant(e, generator)); } @Override public HoldingContainer visitDecimal9Constant(Decimal9Expression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitDecimal9Constant(e, generator); - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitDecimal9Constant(e, generator).setConstant(true); - } else { - return super.visitDecimal9Constant(e, generator); - } + return visitExpression(e, generator, () -> super.visitDecimal9Constant(e, generator)); } @Override public HoldingContainer visitDecimal18Constant(Decimal18Expression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitDecimal18Constant(e, generator); - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitDecimal18Constant(e, generator).setConstant(true); - } else { - return super.visitDecimal18Constant(e, generator); - } + return visitExpression(e, generator, () -> super.visitDecimal18Constant(e, generator)); } @Override public HoldingContainer visitDecimal28Constant(Decimal28Expression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitDecimal28Constant(e, generator); - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitDecimal28Constant(e, generator).setConstant(true); - } else { - return super.visitDecimal28Constant(e, generator); - } + return visitExpression(e, generator, () -> super.visitDecimal28Constant(e, generator)); } @Override public HoldingContainer visitDecimal38Constant(Decimal38Expression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitDecimal38Constant(e, generator); - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitDecimal38Constant(e, generator).setConstant(true); - } else { - return super.visitDecimal38Constant(e, generator); - } + return visitExpression(e, generator, () -> super.visitDecimal38Constant(e, generator)); } @Override public HoldingContainer visitVarDecimalConstant(VarDecimalExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitVarDecimalConstant(e, generator); - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitVarDecimalConstant(e, generator).setConstant(true); - } else { - return super.visitVarDecimalConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitVarDecimalConstant(e, generator)); } @Override public HoldingContainer visitIntConstant(IntExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitIntConstant(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitIntConstant(e, generator).setConstant(true); - } else { - return super.visitIntConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitIntConstant(e, generator)); } @Override public HoldingContainer visitDateConstant(DateExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitDateConstant(e, generator); - - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitDateConstant(e, generator).setConstant(true); - } else { - return super.visitDateConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitDateConstant(e, generator)); } @Override public HoldingContainer visitTimeConstant(TimeExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitTimeConstant(e, generator); - - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitTimeConstant(e, generator).setConstant(true); - } else { - return super.visitTimeConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitTimeConstant(e, generator)); } @Override public HoldingContainer visitIntervalYearConstant(IntervalYearExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitIntervalYearConstant(e, generator); - - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitIntervalYearConstant(e, generator).setConstant(true); - } else { - return super.visitIntervalYearConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitIntervalYearConstant(e, generator)); } @Override public HoldingContainer visitTimeStampConstant(TimeStampExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitTimeStampConstant(e, generator); - - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitTimeStampConstant(e, generator).setConstant(true); - } else { - return super.visitTimeStampConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitTimeStampConstant(e, generator)); } @Override public HoldingContainer visitFloatConstant(FloatExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitFloatConstant(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitFloatConstant(e, generator).setConstant(true); - } else { - return super.visitFloatConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitFloatConstant(e, generator)); } @Override public HoldingContainer visitDoubleConstant(DoubleExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitDoubleConstant(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitDoubleConstant(e, generator).setConstant(true); - } else { - return super.visitDoubleConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitDoubleConstant(e, generator)); } @Override public HoldingContainer visitBooleanConstant(BooleanExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitBooleanConstant(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitBooleanConstant(e, generator).setConstant(true); - } else { - return super.visitBooleanConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitBooleanConstant(e, generator)); } @Override public HoldingContainer visitUnknown(LogicalExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitUnknown(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitUnknown(e, generator).setConstant(true); - } else { - return super.visitUnknown(e, generator); - } + return visitExpression(e, generator, () -> super.visitUnknown(e, generator)); } @Override public HoldingContainer visitQuotedStringConstant(QuotedString e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitQuotedStringConstant(e, generator); - // generator.getMappingSet().exitConstant(); - // return c; - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitQuotedStringConstant(e, generator).setConstant(true); - } else { - return super.visitQuotedStringConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitQuotedStringConstant(e, generator)); } @Override public HoldingContainer visitIntervalDayConstant(IntervalDayExpression e, ClassGenerator<?> generator) throws RuntimeException { - if (constantBoundaries.contains(e)) { - generator.getMappingSet().enterConstant(); - HoldingContainer c = super.visitIntervalDayConstant(e, generator); - - return renderConstantExpression(generator, c); - } else if (generator.getMappingSet().isWithinConstant()) { - return super.visitIntervalDayConstant(e, generator).setConstant(true); - } else { - return super.visitIntervalDayConstant(e, generator); - } + return visitExpression(e, generator, () -> super.visitIntervalDayConstant(e, generator)); } /* * Get a HoldingContainer for a constant expression. The returned - * HoldingContainder will indicate it's for a constant expression. + * HoldingContainer will indicate it's for a constant expression. */ - private HoldingContainer renderConstantExpression(ClassGenerator<?> generator, HoldingContainer input) { - JVar fieldValue = generator.declareClassField("constant", generator.getHolderType(input.getMajorType())); + private HoldingContainer renderConstantExpression(ClassGenerator<?> generator, HoldingContainer input, + LogicalExpression expr) { + MajorType.Builder newTypeBuilder = MajorType.newBuilder(input.getMajorType()); + + if (expr instanceof DrillFuncHolderExpr && + ((DrillFuncHolderExpr) expr).getHolder().getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL) { + newTypeBuilder.setMode(expr.getMajorType().getMode()); + } + MajorType newType = newTypeBuilder.build(); + JVar fieldValue = generator.declareClassField("constant", generator.getHolderType(newType)); + // Creates a new vector for class field and assigns to its fields values from output field // to allow scalar replacement for source objects - generator.getEvalBlock().assign(fieldValue, JExpr._new(generator.getHolderType(input.getMajorType()))); - List<String> holderFields = ValueHolderHelper.getHolderParams(input.getMajorType()); + generator.getEvalBlock().assign(fieldValue, JExpr._new(generator.getHolderType(newType))); + List<String> holderFields = ValueHolderHelper.getHolderParams(newType); for (String holderField : holderFields) { generator.getEvalBlock().assign(fieldValue.ref(holderField), input.getHolder().ref(holderField)); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/AbstractFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/AbstractFuncHolder.java index 7dd58ac..94397f1 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/AbstractFuncHolder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/AbstractFuncHolder.java @@ -44,11 +44,11 @@ public abstract class AbstractFuncHolder implements FuncHolder { * @param classGenerator the class responsible for code generation * @param inputVariables the source of the vector holders * @param workspaceJVars class fields - * @param fieldReference reference of the output field + * @param holderExpr holder for the function expression * @return HoldingContainer for return value */ public abstract HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables, - JVar[] workspaceJVars, FieldReference fieldReference); + JVar[] workspaceJVars, FunctionHolderExpression holderExpr); public boolean isNested() { return false; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java index 76d416e..9f5838c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java @@ -21,6 +21,7 @@ import static org.apache.drill.shaded.guava.com.google.common.base.Preconditions import org.apache.drill.common.exceptions.DrillRuntimeException; import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.types.TypeProtos; import org.apache.drill.common.types.TypeProtos.DataMode; import org.apache.drill.common.types.TypeProtos.MajorType; @@ -127,7 +128,7 @@ class DrillAggFuncHolder extends DrillFuncHolder { @Override public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables, - JVar[] workspaceJVars, FieldReference fieldReference) { + JVar[] workspaceJVars, FunctionHolderExpression holderExpr) { HoldingContainer out = null; JVar internalOutput = null; if (getReturnType().getMinorType() != TypeProtos.MinorType.LATE) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterAggFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterAggFuncHolder.java index 44766bd..ab54222 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterAggFuncHolder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterAggFuncHolder.java @@ -19,6 +19,7 @@ package org.apache.drill.exec.expr.fn; import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.types.TypeProtos; import org.apache.drill.exec.expr.ClassGenerator; import org.apache.drill.exec.expr.ClassGenerator.HoldingContainer; @@ -111,7 +112,7 @@ public class DrillComplexWriterAggFuncHolder extends DrillAggFuncHolder { @Override public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables, - JVar[] workspaceJVars, FieldReference fieldReference) { + JVar[] workspaceJVars, FunctionHolderExpression holderExpr) { HoldingContainer out = null; JVar internalOutput = null; if (getReturnType().getMinorType() != TypeProtos.MinorType.LATE) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java index cb78bec..92fb340 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java @@ -18,6 +18,7 @@ package org.apache.drill.exec.expr.fn; import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.exec.expr.ClassGenerator; import org.apache.drill.exec.expr.ClassGenerator.HoldingContainer; import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling; @@ -47,8 +48,9 @@ public class DrillComplexWriterFuncHolder extends DrillSimpleFuncHolder { @Override protected HoldingContainer generateEvalBody(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables, String body, - JVar[] workspaceJVars, FieldReference fieldReference) { + JVar[] workspaceJVars, FunctionHolderExpression holderExpr) { + FieldReference fieldReference = holderExpr.getFieldReference(); classGenerator.getEvalBlock().directStatement(String.format("//---- start of eval portion of %s function. ----//", getRegisteredNames()[0])); JBlock sub = new JBlock(true, true); 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 6744585..b77b9ac 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 @@ -21,7 +21,7 @@ import static org.apache.drill.shaded.guava.com.google.common.base.Preconditions import com.sun.codemodel.JOp; import org.apache.drill.common.exceptions.DrillRuntimeException; -import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.FunctionHolderExpression; import org.apache.drill.common.types.TypeProtos.DataMode; import org.apache.drill.common.types.TypeProtos.MajorType; import org.apache.drill.exec.expr.ClassGenerator; @@ -77,7 +77,8 @@ public class DrillSimpleFuncHolder extends DrillFuncHolder { @Override public HoldingContainer renderEnd(ClassGenerator<?> classGenerator, HoldingContainer[] inputVariables, - JVar[] workspaceJVars, FieldReference fieldReference) { + JVar[] workspaceJVars, FunctionHolderExpression holderExpr) { + //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++) { @@ -86,14 +87,14 @@ public class DrillSimpleFuncHolder extends DrillFuncHolder { } } generateBody(classGenerator, BlockType.SETUP, setupBody(), inputVariables, workspaceJVars, true); - HoldingContainer c = generateEvalBody(classGenerator, inputVariables, evalBody(), workspaceJVars, fieldReference); + HoldingContainer c = generateEvalBody(classGenerator, inputVariables, evalBody(), workspaceJVars, holderExpr); generateBody(classGenerator, BlockType.RESET, resetBody(), null, workspaceJVars, false); generateBody(classGenerator, BlockType.CLEANUP, cleanupBody(), null, workspaceJVars, false); return c; } protected HoldingContainer generateEvalBody(ClassGenerator<?> g, HoldingContainer[] inputVariables, String body, - JVar[] workspaceJVars, FieldReference ref) { + JVar[] workspaceJVars, FunctionHolderExpression holderExpr) { g.getEvalBlock().directStatement(String.format("//---- start of eval portion of %s function. ----//", getRegisteredNames()[0])); @@ -129,6 +130,8 @@ public class DrillSimpleFuncHolder extends DrillFuncHolder { JConditional jc = sub._if(e); jc._then().assign(out.getIsSet(), JExpr.lit(0)); sub = jc._else(); + } else if (holderExpr.getMajorType().getMode() == DataMode.OPTIONAL) { + returnValueType = getReturnType().toBuilder().setMode(DataMode.OPTIONAL).build(); } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/output/DecimalReturnTypeInference.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/output/DecimalReturnTypeInference.java index b30703d..b735021 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/output/DecimalReturnTypeInference.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/output/DecimalReturnTypeInference.java @@ -88,6 +88,7 @@ public class DecimalReturnTypeInference { public TypeProtos.MajorType getType(List<LogicalExpression> logicalExpressions, FunctionAttributes attributes) { int scale = 0; int precision = 0; + TypeProtos.DataMode mode = FunctionUtils.getReturnTypeDataMode(logicalExpressions, attributes); // Get the max scale and precision from the inputs for (LogicalExpression e : logicalExpressions) { @@ -99,7 +100,7 @@ public class DecimalReturnTypeInference { .setMinorType(attributes.getReturnValue().getType().getMinorType()) .setScale(scale) .setPrecision(precision) - .setMode(TypeProtos.DataMode.OPTIONAL) + .setMode(mode) .build(); } } @@ -295,6 +296,7 @@ public class DecimalReturnTypeInference { @Override public TypeProtos.MajorType getType(List<LogicalExpression> logicalExpressions, FunctionAttributes attributes) { int scale = 0; + TypeProtos.DataMode mode = FunctionUtils.getReturnTypeDataMode(logicalExpressions, attributes); // Get the max scale and precision from the inputs for (LogicalExpression e : logicalExpressions) { @@ -305,7 +307,7 @@ public class DecimalReturnTypeInference { .setMinorType(TypeProtos.MinorType.VARDECIMAL) .setScale(scale) .setPrecision(DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision()) - .setMode(TypeProtos.DataMode.OPTIONAL) + .setMode(mode) .build(); } } @@ -323,6 +325,7 @@ public class DecimalReturnTypeInference { @Override public TypeProtos.MajorType getType(List<LogicalExpression> logicalExpressions, FunctionAttributes attributes) { int scale = 0; + TypeProtos.DataMode mode = FunctionUtils.getReturnTypeDataMode(logicalExpressions, attributes); // Get the max scale and precision from the inputs for (LogicalExpression e : logicalExpressions) { @@ -334,7 +337,7 @@ public class DecimalReturnTypeInference { .setScale(Math.min(Math.max(6, scale), DRILL_REL_DATATYPE_SYSTEM.getMaxNumericScale())) .setPrecision(DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision()) - .setMode(TypeProtos.DataMode.OPTIONAL) + .setMode(mode) .build(); } } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSimpleCastFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSimpleCastFunctions.java index 7565508..482c2da 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSimpleCastFunctions.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSimpleCastFunctions.java @@ -26,6 +26,7 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import javax.annotation.Nullable; +import java.math.BigDecimal; import java.util.Arrays; import java.util.List; @@ -160,4 +161,37 @@ public class TestSimpleCastFunctions extends BaseTestQuery { } } + @Test + public void testDecimalConstantCast() throws Exception { + testBuilder() + .sqlQuery("select casthigh(cast(1025.0 as decimal(28,8))) a") + .unOrdered() + .baselineColumns("a") + .baselineValues(BigDecimal.valueOf(102500000000L, 8)) + .go(); + } + + @Test + public void testNullableDecimalConstantCast() throws Exception { + testBuilder() + .sqlQuery("select casthigh(case when true then cast(1025.0 as decimal(28,8)) else null end) a") + .unOrdered() + .baselineColumns("a") + .baselineValues(BigDecimal.valueOf(102500000000L, 8)) + .go(); + + } + + @Test + public void testDecimalCastAsVariable() throws Exception { + testBuilder() + .sqlQuery("select casthigh(cast(a as decimal(28,8))) a from (VALUES (1.0), (2.0), (3.0)) as t1(a)") + .unOrdered() + .baselineColumns("a") + .baselineValues(BigDecimal.valueOf(100000000, 8)) + .baselineValues(BigDecimal.valueOf(200000000, 8)) + .baselineValues(BigDecimal.valueOf(300000000, 8)) + .go(); + } + }