[CALCITE-6813] UNNEST infers incorrect nullability for the result whe…

…n applied to an array that contains nullable ROW values

Signed-off-by: Mihai Budiu <[email protected]>

core/src/main/java/org/apache/calcite/rel/core/Uncollect.java

@@ -166,34 +166,23 @@ public static RelDataType deriveUncollectRowType(RelNode rel,
 
     for (int i = 0; i < fields.size(); i++) {
       RelDataTypeField field = fields.get(i);
-      boolean containerIsNullable = field.getType().isNullable();
       if (field.getType() instanceof MapSqlType) {
         // This code is similar to SqlUnnestOperator::inferReturnType.
         MapSqlType mapType = (MapSqlType) field.getType();
-        RelDataType keyType = mapType.getKeyType();
-        RelDataType valueType = mapType.getValueType();
-        if (containerIsNullable) {
-          keyType = typeFactory.enforceTypeWithNullability(keyType, true);
-          valueType = typeFactory.enforceTypeWithNullability(valueType, true);
-        }
-        builder.add(SqlUnnestOperator.MAP_KEY_COLUMN_NAME, keyType);
-        builder.add(SqlUnnestOperator.MAP_VALUE_COLUMN_NAME, valueType);
+        builder.add(SqlUnnestOperator.MAP_KEY_COLUMN_NAME, mapType.getKeyType());
+        builder.add(SqlUnnestOperator.MAP_VALUE_COLUMN_NAME, mapType.getValueType());
       } else {
         RelDataType componentType = field.getType().getComponentType();
         if (null == componentType) {
           throw RESOURCE.unnestArgument().ex();
         }
         boolean isNullable = componentType.isNullable();
-        if (containerIsNullable || isNullable) {
-          componentType = typeFactory.enforceTypeWithNullability(componentType, true);
-        }
-
         if (requireAlias) {
           builder.add(itemAliases.get(i), componentType);
         } else if (componentType.isStruct()) {
           for (RelDataTypeField fieldInfo : componentType.getFieldList()) {
             RelDataType fieldType = fieldInfo.getType();
-            if (containerIsNullable || isNullable) {
+            if (isNullable) {
               fieldType = typeFactory.enforceTypeWithNullability(fieldType, true);
             }
             builder.add(fieldInfo.getName(), fieldType);

core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java

@@ -68,7 +68,7 @@ public SqlUnnestOperator(boolean withOrdinality) {
       RelDataType type = opBinding.getOperandType(operand);
       if (type.getSqlTypeName() == SqlTypeName.ANY) {
         // Unnest Operator in schema less systems returns one column as the output
-        // $unnest is a place holder to specify that one column with type ANY is output.
+        // $unnest is a placeholder to specify that one column with type ANY is output.
         return builder
             .add("$unnest",
                 SqlTypeName.ANY)
@@ -83,32 +83,22 @@ public SqlUnnestOperator(boolean withOrdinality) {
       assert type instanceof ArraySqlType || type instanceof MultisetSqlType
           || type instanceof MapSqlType;
       // If a type is nullable, all field accesses inside the type are also nullable
-      boolean containerIsNullable = type.isNullable();
       if (type instanceof MapSqlType) {
         MapSqlType mapType = (MapSqlType) type;
-        RelDataType keyType = mapType.getKeyType();
-        RelDataType valueType = mapType.getValueType();
-        if (containerIsNullable) {
-          keyType = typeFactory.enforceTypeWithNullability(keyType, true);
-          valueType = typeFactory.enforceTypeWithNullability(valueType, true);
-        }
-        builder.add(MAP_KEY_COLUMN_NAME, keyType);
-        builder.add(MAP_VALUE_COLUMN_NAME, valueType);
+        builder.add(MAP_KEY_COLUMN_NAME, mapType.getKeyType());
+        builder.add(MAP_VALUE_COLUMN_NAME, mapType.getValueType());
       } else {
         RelDataType componentType = requireNonNull(type.getComponentType(), "componentType");
         boolean isNullable = componentType.isNullable();
         if (!allowAliasUnnestItems(opBinding) && componentType.isStruct()) {
           for (RelDataTypeField field : componentType.getFieldList()) {
             RelDataType fieldType = field.getType();
-            if (containerIsNullable || isNullable) {
+            if (isNullable) {
               fieldType = typeFactory.enforceTypeWithNullability(fieldType, true);
             }
             builder.add(field.getName(), fieldType);
           }
         } else {
-          if (containerIsNullable) {
-            componentType = typeFactory.enforceTypeWithNullability(componentType, true);
-          }
           builder.add(SqlUtil.deriveAliasFromOrdinal(operand),
               componentType);
         }

core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java

@@ -1066,8 +1066,45 @@ void testDyadicCollateOperator() {
         + "select e.EXPR$0\n"
         + "from orders, UNNEST(orders.data) as e")
         .type(actualType -> {
+          // Unfortunately the string representation does not contain nullability information
+          assertThat(actualType, hasToString("RecordType(INTEGER EXPR$0)"));
+          assertTrue(actualType.isStruct());
+          assertThat(actualType.getFieldCount(), is(1));
+          // The field type should be nullable
           assertTrue(actualType.getFieldList().get(0).getType().isNullable());
         });
+    sql("with orders(data) as\n"
+        + "  (values (ARRAY[ROW(1, 'Alice'), ROW(2, NULL), ROW(NULL, 'Bob'), NULL]))\n"
+        + "select e.*\n"
+        + "from orders, UNNEST(orders.data) as e")
+        .type(actualType -> {
+          assertThat(actualType, hasToString("RecordType(INTEGER EXPR$0, CHAR(5) EXPR$1)"));
+          assertTrue(actualType.isStruct());
+          assertTrue(actualType.getFieldList().get(0).getType().isNullable());
+          assertTrue(actualType.getFieldList().get(1).getType().isNullable());
+        });
+    sql("with orders(data) as\n"
+        + "  (values (ARRAY[ROW(1, 'Alice'), ROW(2, NULL)]))\n"
+        + "select e.*\n"
+        + "from orders, UNNEST(orders.data) as e")
+        .type(actualType -> {
+          // The array unnested is not nullable
+          assertThat(actualType, hasToString("RecordType(INTEGER EXPR$0, CHAR(5) EXPR$1)"));
+          assertTrue(actualType.isStruct());
+          assertThat(actualType.getFieldList().get(0).getType().isNullable(), is(false));
+          assertTrue(actualType.getFieldList().get(1).getType().isNullable());
+        });
+    sql("with orders(data) as\n"
+        + "  (values (ARRAY[ARRAY[ROW(1, 'Alice'), ROW(2, NULL)], NULL]))\n"
+        + "select e.*\n"
+        + "from orders, UNNEST(orders.data[1]) as e")
+        .type(actualType -> {
+          // The inner array that is unnested is nullable in this example
+          assertThat(actualType, hasToString("RecordType(INTEGER EXPR$0, CHAR(5) EXPR$1)"));
+          assertTrue(actualType.isStruct());
+          assertThat(actualType.getFieldList().get(0).getType().isNullable(), is(false));
+          assertTrue(actualType.getFieldList().get(1).getType().isNullable());
+        });
   }
 
   @Test void testOverlay() {