[GitHub] Ben-Zvi closed pull request #1510: DRILL-6811: Fix type inference to return correct data mode for boolean functions

[GitBox <gi...@apache.org>]

Ben-Zvi closed pull request #1510: DRILL-6811: Fix type inference to return correct data mode for boolean functions
URL: https://github.com/apache/drill/pull/1510
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java
index 031da3e3ab5..016473c03db 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java
@@ -284,16 +284,21 @@ public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
           // In summary, if we have a boolean output function in the WHERE-CLAUSE,
           // this logic can validate and execute user queries seamlessly
           boolean allBooleanOutput = true;
+          boolean isNullable = false;
           for (DrillFuncHolder function : functions) {
             if (function.getReturnType().getMinorType() != TypeProtos.MinorType.BIT) {
               allBooleanOutput = false;
               break;
             }
+            if (function.getReturnType().getMode() == TypeProtos.DataMode.OPTIONAL
+                || function.getNullHandling() == FunctionTemplate.NullHandling.NULL_IF_NULL) {
+              isNullable = true;
+            }
           }
 
-          if(allBooleanOutput) {
+          if (allBooleanOutput) {
             return factory.createTypeWithNullability(
-                factory.createSqlType(SqlTypeName.BOOLEAN), true);
+                factory.createSqlType(SqlTypeName.BOOLEAN), isNullable);
           } else {
             return factory.createTypeWithNullability(
                 factory.createSqlType(SqlTypeName.ANY),
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
index 719df6ff53e..bf1f4aa8580 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsQuery.java
@@ -24,11 +24,16 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.ZoneOffset;
+import java.util.Arrays;
+import java.util.List;
 
 import org.apache.drill.categories.SqlFunctionTest;
 import org.apache.drill.common.exceptions.UserRemoteException;
+import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.record.BatchSchema;
 import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.test.rowSet.schema.SchemaBuilder;
 import org.hamcrest.CoreMatchers;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -977,4 +982,30 @@ public void testNegate() throws Exception {
             .baselineValues(new BigDecimal("-1.1"))
             .go();
   }
+
+  @Test
+  public void testBooleanConditionsMode() throws Exception {
+    List<String> conditions = Arrays.asList(
+        "employee_id IS NULL",
+        "employee_id IS NOT NULL",
+        "employee_id > 0 IS TRUE",
+        "employee_id > 0 IS NOT TRUE",
+        "employee_id > 0 IS FALSE",
+        "employee_id > 0 IS NOT FALSE",
+        "employee_id IS NULL OR position_id IS NULL",
+        "employee_id IS NULL AND position_id IS NULL",
+        "isdate(employee_id)",
+        "NOT (employee_id IS NULL)");
+
+    BatchSchema expectedSchema = new SchemaBuilder()
+        .add("col1", TypeProtos.MinorType.BIT)
+        .build();
+
+    for (String condition : conditions) {
+      testBuilder()
+          .sqlQuery("SELECT %s AS col1 FROM cp.`employee.json` LIMIT 0", condition)
+          .schemaBaseLine(expectedSchema)
+          .go();
+    }
+  }
 }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
index 08b15fc0f37..207638d9aa1 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestFunctionsWithTypeExpoQueries.java
@@ -235,7 +235,7 @@ public void testIsNull() throws Exception {
     List<Pair<SchemaPath, TypeProtos.MajorType>> expectedSchema = Lists.newArrayList();
     TypeProtos.MajorType majorType = TypeProtos.MajorType.newBuilder()
         .setMinorType(TypeProtos.MinorType.BIT)
-        .setMode(TypeProtos.DataMode.OPTIONAL)
+        .setMode(TypeProtos.DataMode.REQUIRED)
         .build();
     expectedSchema.add(Pair.of(SchemaPath.getSimplePath("col"), majorType));
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services