You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ar...@apache.org on 2019/07/15 11:58:31 UTC
[drill] branch master updated: DRILL-7307: casthigh for decimal
type can lead to the issues with VarDecimalHolder
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 <dm...@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();
+ }
+
}