You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by vo...@apache.org on 2020/05/25 22:34:51 UTC
[drill] 02/04: DRILL-7739: Allow implicit casts from required to
nullable data type
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 <vv...@gmail.com>
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