You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by da...@apache.org on 2020/07/15 09:09:53 UTC

[calcite] branch master updated: [CALCITE-4085] Improve return type nullability for SqlDotOperator & SqlItemOperator (Dawid Wysakowicz)

This is an automated email from the ASF dual-hosted git repository.

danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new e2942fe  [CALCITE-4085] Improve return type nullability for SqlDotOperator & SqlItemOperator (Dawid Wysakowicz)
e2942fe is described below

commit e2942feef4d52c577a46cc3f2b92fa462035bb20
Author: Dawid Wysakowicz <dw...@apache.org>
AuthorDate: Mon Jun 22 12:28:37 2020 +0200

    [CALCITE-4085] Improve return type nullability for SqlDotOperator & SqlItemOperator (Dawid Wysakowicz)
    
    This PR introduces a logic that makes the accesed field nullable if the
    accessed row is nullable.
    
    Make the AliasNamespace keep the original nullability & StructKing of
    the aliased record.
    
    close apache/calcite#2042
---
 .../org/apache/calcite/sql/fun/SqlDotOperator.java |  3 ++
 .../apache/calcite/sql/fun/SqlItemOperator.java    |  6 +++-
 .../calcite/sql/validate/AliasNamespace.java       | 12 ++++++-
 .../apache/calcite/test/MockSqlOperatorTable.java  | 41 ++++++++++++++++++++++
 .../org/apache/calcite/test/SqlValidatorTest.java  | 15 ++++++++
 .../test/catalog/MockCatalogReaderExtended.java    | 33 +++++++++++++++++
 6 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
index 6855df1..7e7af24 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
@@ -111,6 +111,9 @@ public class SqlDotOperator extends SqlSpecialOperator {
           Static.RESOURCE.unknownField(fieldName));
     }
     RelDataType type = field.getType();
+    if (nodeType.isNullable()) {
+      type = validator.getTypeFactory().createTypeWithNullability(type, true);
+    }
 
     // Validate and determine coercibility and resulting collation
     // name of binary operator if needed.
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
index 8bbcdcd..cd2ffe6 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
@@ -135,7 +135,11 @@ class SqlItemOperator extends SqlSpecialOperator {
         throw new AssertionError("Cannot infer type of field '"
             + fieldName + "' within ROW type: " + operandType);
       } else {
-        return field.getType();
+        RelDataType fieldType = field.getType();
+        if (operandType.isNullable()) {
+          fieldType = typeFactory.createTypeWithNullability(fieldType, true);
+        }
+        return fieldType;
       }
     case ANY:
     case DYNAMIC_STAR:
diff --git a/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java b/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
index 0cc24af..4abd72e 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/AliasNamespace.java
@@ -17,6 +17,7 @@
 package org.apache.calcite.sql.validate;
 
 import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactoryImpl;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlIdentifier;
@@ -91,9 +92,18 @@ public class AliasNamespace extends AbstractNamespace {
     for (RelDataTypeField field : rowType.getFieldList()) {
       typeList.add(field.getType());
     }
-    return validator.getTypeFactory().createStructType(
+    final RelDataType aliasedType = validator.getTypeFactory().createStructType(
+        rowType.getStructKind(),
         typeList,
         nameList);
+
+    // As per suggestion in CALCITE-4085, JavaType has its special nullability handling.
+    if (rowType instanceof RelDataTypeFactoryImpl.JavaType) {
+      return aliasedType;
+    } else {
+      return validator.getTypeFactory()
+          .createTypeWithNullability(aliasedType, rowType.isNullable());
+    }
   }
 
   private String getString(RelDataType rowType) {
diff --git a/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java b/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
index b1ac440..0da69b8 100644
--- a/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
+++ b/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
@@ -18,6 +18,9 @@ package org.apache.calcite.test;
 
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
+import org.apache.calcite.rel.type.RelRecordType;
+import org.apache.calcite.rel.type.StructKind;
 import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlFunction;
 import org.apache.calcite.sql.SqlFunctionCategory;
@@ -37,6 +40,8 @@ import org.apache.calcite.util.Optionality;
 
 import com.google.common.collect.ImmutableList;
 
+import java.util.Arrays;
+
 /**
  * Mock operator table for testing purposes. Contains the standard SQL operator
  * table, plus a list of operators.
@@ -63,6 +68,7 @@ public class MockSqlOperatorTable extends ChainedSqlOperatorTable {
     opTab.addOperator(new DedupFunction());
     opTab.addOperator(new MyFunction());
     opTab.addOperator(new MyAvgAggFunction());
+    opTab.addOperator(new RowFunction());
   }
 
   /** "RAMP" user-defined function. */
@@ -168,4 +174,39 @@ public class MockSqlOperatorTable extends ChainedSqlOperatorTable {
       return false;
     }
   }
+
+  /** "ROW_FUNC" user-defined function whose return type is
+   * nullable row type with non-nullable fields. */
+  public static class RowFunction extends SqlFunction {
+    public RowFunction() {
+      super("ROW_FUNC",
+          SqlKind.OTHER_FUNCTION,
+          null,
+          null,
+          OperandTypes.NILADIC,
+          SqlFunctionCategory.USER_DEFINED_FUNCTION);
+    }
+
+    public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
+      final RelDataTypeFactory typeFactory =
+          opBinding.getTypeFactory();
+      final RelDataType bigIntNotNull = typeFactory.createSqlType(SqlTypeName.BIGINT);
+      final RelDataType bigIntNullable =
+          typeFactory.createTypeWithNullability(bigIntNotNull, true);
+      return new RelRecordType(
+          StructKind.FULLY_QUALIFIED,
+          Arrays.asList(
+              new RelDataTypeFieldImpl(
+                  "NOT_NULL_FIELD",
+                  0,
+                  bigIntNotNull),
+              new RelDataTypeFieldImpl(
+                  "NULLABLE_FIELD",
+                  0,
+                  bigIntNullable)
+          ),
+          true
+      );
+    }
+  }
 }
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 60da2b9..9a0a513 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -11491,4 +11491,19 @@ class SqlValidatorTest extends SqlValidatorTestCase {
     assertThat(resultType.toString(), is("INTEGER"));
   }
 
+  @Test void testAccessingNestedFieldsOfNullableRecord() {
+    sql("select ROW_COLUMN_ARRAY[0].NOT_NULL_FIELD from NULLABLEROWS.NR_T1")
+        .withExtendedCatalog()
+        .type("RecordType(BIGINT EXPR$0) NOT NULL");
+    sql("select ROW_COLUMN_ARRAY[0]['NOT_NULL_FIELD'] from NULLABLEROWS.NR_T1")
+        .withExtendedCatalog()
+        .type("RecordType(BIGINT EXPR$0) NOT NULL");
+
+    final MockSqlOperatorTable operatorTable =
+        new MockSqlOperatorTable(SqlStdOperatorTable.instance());
+    MockSqlOperatorTable.addRamp(operatorTable);
+    sql("select * FROM TABLE(ROW_FUNC()) AS T(a, b)")
+        .withOperatorTable(operatorTable)
+        .type("RecordType(BIGINT A, BIGINT B) NOT NULL");
+  }
 }
diff --git a/core/src/test/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java b/core/src/test/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java
index a12cf3b..9246113 100644
--- a/core/src/test/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java
+++ b/core/src/test/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java
@@ -16,9 +16,14 @@
  */
 package org.apache.calcite.test.catalog;
 
+import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
+import org.apache.calcite.rel.type.RelRecordType;
+import org.apache.calcite.rel.type.StructKind;
 import org.apache.calcite.schema.TableMacro;
 import org.apache.calcite.schema.TranslatableTable;
+import org.apache.calcite.sql.type.SqlTypeName;
 
 import com.google.common.collect.ImmutableList;
 
@@ -163,6 +168,34 @@ public class MockCatalogReaderExtended extends MockCatalogReaderSimple {
     complexTypeColumnsTable.addColumn("rowArrayMultisetType", f.rowArrayMultisetType);
     registerTable(complexTypeColumnsTable);
 
+    MockSchema nullableRowsSchema = new MockSchema("NULLABLEROWS");
+    registerSchema(nullableRowsSchema);
+    final MockTable nullableRowsTable =
+        MockTable.create(this, nullableRowsSchema, "NR_T1", false, 100);
+    RelDataType bigIntNotNull = typeFactory.createSqlType(SqlTypeName.BIGINT);
+    RelDataType nullableRecordType = new RelRecordType(
+        StructKind.FULLY_QUALIFIED,
+        Arrays.asList(
+            new RelDataTypeFieldImpl(
+                "NOT_NULL_FIELD",
+                0,
+                bigIntNotNull),
+            new RelDataTypeFieldImpl(
+                "NULLABLE_FIELD",
+                0,
+                typeFactory.createTypeWithNullability(bigIntNotNull, true)
+            )
+        ),
+        true
+    );
+
+    nullableRowsTable.addColumn("ROW_COLUMN", nullableRecordType, false);
+    nullableRowsTable.addColumn(
+        "ROW_COLUMN_ARRAY",
+        typeFactory.createArrayType(nullableRecordType, -1),
+        true);
+    registerTable(nullableRowsTable);
+
     return this;
   }
 }