drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From volody...@apache.org
Subject [drill] 02/04: DRILL-7739: Allow implicit casts from required to nullable data type
Date Mon, 25 May 2020 22:34:51 GMT
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 <vvovyk@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


Mime
View raw message