From e7f0bbf1f45b0d39d3796be81207aecd716d6aae Mon Sep 17 00:00:00 2001 From: Volodymyr Vysotskyi Date: Sat, 9 Mar 2019 23:57:33 +0200 Subject: DRILL-6524: Assign holder fields instead of assigning object references in generated code to allow scalar replacement for more cases closes #1686 --- .../apache/drill/exec/expr/EvaluationVisitor.java | 27 ++++++-- .../drill/exec/expr/fn/DrillSimpleFuncHolder.java | 14 ++-- .../drill/exec/vector/ValueHolderHelper.java | 77 +++++++++++++++++++++- 3 files changed, 105 insertions(+), 13 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 6f797480f..4d85c6a9a 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 @@ -237,13 +237,17 @@ public class EvaluationVisitor { generator.unNestEvalBlock(); + List holderFields = ValueHolderHelper.getHolderParams(output.getMajorType()); if (thenExpr.isOptional()) { JConditional newCond = jc._then()._if(thenExpr.getIsSet().ne(JExpr.lit(0))); JBlock b = newCond._then(); - b.assign(output.getHolder(), thenExpr.getHolder()); - //b.assign(output.getIsSet(), thenExpr.getIsSet()); + for (String holderField : holderFields) { + b.assign(output.f(holderField), thenExpr.f(holderField)); + } } else { - jc._then().assign(output.getHolder(), thenExpr.getHolder()); + for (String holderField : holderFields) { + jc._then().assign(output.f(holderField), thenExpr.f(holderField)); + } } generator.nestEvalBlock(jc._else()); @@ -255,10 +259,13 @@ public class EvaluationVisitor { if (elseExpr.isOptional()) { JConditional newCond = jc._else()._if(elseExpr.getIsSet().ne(JExpr.lit(0))); JBlock b = newCond._then(); - b.assign(output.getHolder(), elseExpr.getHolder()); - //b.assign(output.getIsSet(), elseExpr.getIsSet()); + for (String holderField : holderFields) { + b.assign(output.f(holderField), elseExpr.f(holderField)); + } } else { - jc._else().assign(output.getHolder(), elseExpr.getHolder()); + for (String holderField : holderFields) { + jc._else().assign(output.f(holderField), elseExpr.f(holderField)); + } } local.add(conditionalBlock); return output; @@ -1436,7 +1443,13 @@ public class EvaluationVisitor { */ private HoldingContainer renderConstantExpression(ClassGenerator generator, HoldingContainer input) { JVar fieldValue = generator.declareClassField("constant", generator.getHolderType(input.getMajorType())); - generator.getEvalBlock().assign(fieldValue, input.getHolder()); + // 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 holderFields = ValueHolderHelper.getHolderParams(input.getMajorType()); + for (String holderField : holderFields) { + generator.getEvalBlock().assign(fieldValue.ref(holderField), input.getHolder().ref(holderField)); + } generator.getMappingSet().exitConstant(); return new HoldingContainer(input.getMajorType(), fieldValue, fieldValue.ref("value"), fieldValue.ref("isSet")) .setConstant(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 1cc25563c..6744585d2 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 @@ -36,9 +36,11 @@ import com.sun.codemodel.JExpr; import com.sun.codemodel.JExpression; import com.sun.codemodel.JMod; import com.sun.codemodel.JVar; +import org.apache.drill.exec.vector.ValueHolderHelper; + +import java.util.List; public class DrillSimpleFuncHolder extends DrillFuncHolder { - static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillSimpleFuncHolder.class); private final String drillFuncClass; // each function should be wrapped unique class loader associated with its jar @@ -140,12 +142,14 @@ public class DrillSimpleFuncHolder extends DrillFuncHolder { JVar internalOutput = sub.decl(JMod.FINAL, g.getHolderType(returnValueType), getReturnValue().getName(), 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 + + List holderFields = ValueHolderHelper.getHolderParams(returnValueType); + for (String holderField : holderFields) { + sub.assign(out.f(holderField), internalOutput.ref(holderField)); } - sub.assign(out.getHolder(), internalOutput); + if (sub != topSub) { - sub.assign(internalOutput.ref("isSet"),JExpr.lit(1));// Assign null if NULL_IF_NULL mode + sub.assign(out.f("isSet"),JExpr.lit(1)); // Assign null if NULL_IF_NULL mode } g.getEvalBlock().directStatement(String.format("//---- end of eval portion of %s function. ----//", getRegisteredNames()[0])); diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java index 7087687c1..15a2a2d02 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java @@ -20,7 +20,10 @@ package org.apache.drill.exec.vector; import io.netty.buffer.DrillBuf; import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import org.apache.drill.common.types.TypeProtos; import org.apache.drill.exec.expr.holders.BigIntHolder; import org.apache.drill.exec.expr.holders.BitHolder; import org.apache.drill.exec.expr.holders.DateHolder; @@ -43,7 +46,6 @@ import org.apache.drill.exec.util.DecimalUtility; import org.apache.drill.shaded.guava.com.google.common.base.Charsets; - public class ValueHolderHelper { static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ValueHolderHelper.class); @@ -229,4 +231,77 @@ public class ValueHolderHelper { return dch; } + + /** + * Returns list of field names which belong to holder corresponding to the specified {@code TypeProtos.MajorType type}. + * + * @param type type of holder whose fields should be returned + * @return list of field names which belong to holder corresponding to the specified {@code TypeProtos.MajorType type}. + */ + public static List getHolderParams(TypeProtos.MajorType type) { + ArrayList result = new ArrayList<>(); + switch (type.getMode()) { + case OPTIONAL: + result.add("isSet"); + // fall through + case REQUIRED: + switch (type.getMinorType()) { + case BIGINT: + case FLOAT4: + case FLOAT8: + case INT: + case MONEY: + case SMALLINT: + case TINYINT: + case UINT1: + case UINT2: + case UINT4: + case UINT8: + case INTERVALYEAR: + case DATE: + case TIME: + case TIMESTAMP: + case BIT: + case DECIMAL9: + case DECIMAL18: + result.add("value"); + return result; + case DECIMAL28DENSE: + case DECIMAL28SPARSE: + case DECIMAL38DENSE: + case DECIMAL38SPARSE: + result.add("start"); + result.add("buffer"); + result.add("scale"); + result.add("precision"); + return result; + case INTERVAL: { + result.add("months"); + result.add("days"); + result.add("milliseconds"); + return result; + } + case INTERVALDAY: { + result.add("days"); + result.add("milliseconds"); + return result; + } + case VARDECIMAL: + result.add("scale"); + result.add("precision"); + // fall through + case VAR16CHAR: + case VARBINARY: + case VARCHAR: + result.add("start"); + result.add("end"); + result.add("buffer"); + return result; + case UNION: + result.add("reader"); + return result; + } + } + return result; + } } -- cgit v1.2.3