This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit 64b40bebb11a980c1d7710c33a7d15b2cb47d42d Author: Volodymyr Vysotskyi <[email protected]> AuthorDate: Sun May 17 23:31:17 2020 +0300 DRILL-7739: Allow implicit casts from required to nullable data type closes #2080 --- .../exec/expr/ExpressionTreeMaterializer.java | 40 +++++++++++++++++----- .../apache/drill/exec/resolver/TypeCastRules.java | 4 ++- .../drill/exec/fn/impl/TestVarArgFunctions.java | 21 +++++++++--- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java index 603d766..b242fee 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java @@ -465,7 +465,7 @@ public class ExpressionTreeMaterializer { for (int i = 0; i < call.argCount(); ++i) { LogicalExpression currentArg = call.arg(i); - TypeProtos.MajorType parmType = matchedFuncHolder.getParamMajorType(i); + TypeProtos.MajorType paramType = matchedFuncHolder.getParamMajorType(i); // Case 1: If 1) the argument is NullExpression // 2) the minor type of parameter of matchedFuncHolder is not LATE @@ -473,24 +473,27 @@ public class ExpressionTreeMaterializer { // 3) the parameter of matchedFuncHolder allows null input, or func's null_handling // is NULL_IF_NULL (means null and non-null are exchangeable). // then replace NullExpression with a TypedNullConstant - if (currentArg.equals(NullExpression.INSTANCE) && !MinorType.LATE.equals(parmType.getMinorType()) && - (TypeProtos.DataMode.OPTIONAL.equals(parmType.getMode()) || + if (currentArg.equals(NullExpression.INSTANCE) && !MinorType.LATE.equals(paramType.getMinorType()) && + (TypeProtos.DataMode.OPTIONAL.equals(paramType.getMode()) || matchedFuncHolder.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL)) { // Case 1: argument is a null expression, convert it to a typed null - argsWithCast.add(new TypedNullConstant(parmType)); - } else if (Types.softEquals(parmType, currentArg.getMajorType(), + argsWithCast.add(new TypedNullConstant(paramType)); + } else if (Types.softEquals(paramType, currentArg.getMajorType(), matchedFuncHolder.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL) || matchedFuncHolder.isFieldReader(i)) { + currentArg = addNullableCast(currentArg, paramType, functionLookupContext); // Case 2: argument and parameter matches, or parameter is FieldReader. Do nothing. argsWithCast.add(currentArg); } else { // Case 3: insert cast if param type is different from arg type. - if (Types.isDecimalType(parmType)) { + if (Types.isDecimalType(paramType)) { // We are implicitly promoting a decimal type, set the required scale and precision - parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()). + paramType = MajorType.newBuilder().setMinorType(paramType.getMinorType()).setMode(paramType.getMode()). setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build(); } - argsWithCast.add(addCastExpression(currentArg, parmType, functionLookupContext, errorCollector)); + LogicalExpression castExpression = addCastExpression(currentArg, paramType, functionLookupContext, errorCollector); + castExpression = addNullableCast(castExpression, paramType, functionLookupContext); + argsWithCast.add(castExpression); } } @@ -511,6 +514,27 @@ public class ExpressionTreeMaterializer { return funcExpr; } + /** + * Adds cast to {@code DataMode.OPTIONAL} data mode if specified {@code LogicalExpression argument} + * has {@code DataMode.REQUIRED} data mode and specified {@code MajorType targetParamType} + * has {@code DataMode.OPTIONAL} one. + * + * @param argument argument expression + * @param functionParamType type of function argument + * @param functionLookupContext function lookup context + * @return expression with the added cast to {@code DataMode.OPTIONAL} data mode if required, + * otherwise unchanged {@code LogicalExpression argument} + */ + private LogicalExpression addNullableCast(LogicalExpression argument, MajorType functionParamType, + FunctionLookupContext functionLookupContext) { + if (functionParamType.getMode() == DataMode.OPTIONAL + && argument.getMajorType().getMode() == DataMode.REQUIRED) { + // convert argument to nullable type if function require nullable argument + argument = convertToNullableType(argument, functionParamType.getMinorType(), functionLookupContext, errorCollector); + } + return argument; + } + private LogicalExpression bindNonDrillFunc(FunctionCall call, FunctionLookupContext functionLookupContext, AbstractFuncHolder matchedNonDrillFuncHolder) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java index 8603b20..9baf6c5 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/resolver/TypeCastRules.java @@ -877,8 +877,10 @@ public class TypeCastRules { for (int i = varArgIndex; i < numOfArgs; i++) { if (holder.isFieldReader(varArgIndex)) { break; - } else if (holder.getParamMajorType(varArgIndex).getMode() != argumentTypes.get(i).getMode()) { + } else if (holder.getParamMajorType(varArgIndex).getMode() == DataMode.REQUIRED + && holder.getParamMajorType(varArgIndex).getMode() != argumentTypes.get(i).getMode()) { // prohibit using vararg functions for types with different nullability + // if function accepts required arguments, but provided optional return -1; } } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarArgFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarArgFunctions.java index f63d081..1004cd0 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarArgFunctions.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarArgFunctions.java @@ -18,7 +18,6 @@ package org.apache.drill.exec.fn.impl; import org.apache.drill.common.exceptions.DrillRuntimeException; -import org.apache.drill.common.exceptions.UserException; import org.apache.drill.common.exceptions.UserRemoteException; import org.apache.drill.test.ClusterFixture; import org.apache.drill.test.ClusterTest; @@ -99,10 +98,22 @@ public class TestVarArgFunctions extends ClusterTest { @Test public void testVarargUdfWithDifferentDataModes() throws Exception { - thrown.expect(UserException.class); - thrown.expectMessage(containsString("Missing function implementation: [concat_varchar")); - run("SELECT concat_varchar(first_name, ' ', last_name) as c1\n" + - "from cp.`employee.json` limit 10"); + testBuilder() + .sqlQuery("SELECT concat_varchar(first_name, ' ', last_name) as c1\n" + + "from cp.`employee.json` limit 10") + .unOrdered() + .baselineColumns("c1") + .baselineValues("Sheri Nowmer") + .baselineValues("Derrick Whelply") + .baselineValues("Michael Spence") + .baselineValues("Maya Gutierrez") + .baselineValues("Roberta Damstra") + .baselineValues("Rebecca Kanagaki") + .baselineValues("Kim Brunner") + .baselineValues("Brenda Blumberg") + .baselineValues("Darren Stanz") + .baselineValues("Jonathan Murraiin") + .go(); } @Test
