[CALCITE-6041] MAP sub-query gives NullPointerException

core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java

@@ -19,6 +19,7 @@
 import org.apache.calcite.linq4j.tree.BlockBuilder;
 import org.apache.calcite.linq4j.tree.Expression;
 import org.apache.calcite.linq4j.tree.Expressions;
+import org.apache.calcite.linq4j.tree.ParameterExpression;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.rel.RelNode;
@@ -88,39 +89,73 @@ public static Collect create(RelNode input, RelDataType rowType) {
             getRowType(),
             JavaRowFormat.LIST);
 
+    final SqlTypeName collectionType = getCollectionType();
+
     // final Enumerable child = <<child adapter>>;
     // final Enumerable<Object[]> converted = child.select(<<conversion code>>);
-    // final List<Object[]> list = converted.toList();
+    // if collectionType is ARRAY or MULTISET: final List<Object[]> list = converted.toList();
+    // if collectionType is MAP:               final Map<Object, Object> map = converted.toMap();
     Expression child_ =
         builder.append(
             "child", result.block);
 
-    RelDataType collectionComponentType =
-        requireNonNull(rowType().getFieldList().get(0).getType().getComponentType());
-    RelDataType childRecordType = result.physType.getRowType().getFieldList().get(0).getType();
-
     Expression conv_ = child_;
-    if (!SqlTypeUtil.sameNamedType(collectionComponentType, childRecordType)) {
-      // In the internal representation of multisets , every element must be a record. In case the
-      // result above is a scalar type we have to wrap it around a physical type capable of
-      // representing records. For this reason the following conversion is necessary.
-      // REVIEW zabetak January 7, 2019: If we can ensure that the input to this operator
-      // has the correct physical type (e.g., respecting the Prefer.ARRAY above)
-      // then this conversion can be removed.
-      conv_ =
-          builder.append(
-              "converted", result.physType.convertTo(child_, JavaRowFormat.ARRAY));
-    }
+    Expression collectionExpr;
+    switch (collectionType) {
+    case ARRAY:
+    case MULTISET:
+      RelDataType collectionComponentType =
+          requireNonNull(rowType().getFieldList().get(0).getType().getComponentType());
+      RelDataType childRecordType = result.physType.getRowType().getFieldList().get(0).getType();
+
+      if (!SqlTypeUtil.sameNamedType(collectionComponentType, childRecordType)) {
+        // In the internal representation of multisets , every element must be a record. In case the
+        // result above is a scalar type we have to wrap it around a physical type capable of
+        // representing records. For this reason the following conversion is necessary.
+        // REVIEW zabetak January 7, 2019: If we can ensure that the input to this operator
+        // has the correct physical type (e.g., respecting the Prefer.ARRAY above)
+        // then this conversion can be removed.
+        conv_ =
+            builder.append(
+                "converted", result.physType.convertTo(child_, JavaRowFormat.ARRAY));
+      }
+
+      collectionExpr =
+          builder.append("list",
+              Expressions.call(conv_,
+                  BuiltInMethod.ENUMERABLE_TO_LIST.method));
+      break;
+    case MAP:
+      // Convert input 'Object[]' to MAP data, we don't specify a comparator, just
+      // keep the original order of this map. (the inner map is a LinkedHashMap)
+      ParameterExpression input = Expressions.parameter(Object.class, "input");
 
-    Expression list_ =
-        builder.append("list",
-            Expressions.call(conv_,
-                BuiltInMethod.ENUMERABLE_TO_LIST.method));
+      // keySelector lambda: input -> ((Object[])input)[0]
+      Expression keySelector =
+          Expressions.lambda(
+              Expressions.arrayIndex(Expressions.convert_(input, Object[].class),
+                  Expressions.constant(0)), input);
+
+      // valueSelector lambda: input -> ((Object[])input)[1]
+      Expression valueSelector =
+          Expressions.lambda(
+              Expressions.arrayIndex(Expressions.convert_(input, Object[].class),
+                  Expressions.constant(1)), input);
+
+      collectionExpr =
+          builder.append("map",
+              Expressions.call(conv_,
+                  BuiltInMethod.ENUMERABLE_TO_MAP.method, keySelector, valueSelector));
+      break;
+    default:
+      throw new IllegalArgumentException("unknown collection type " + collectionType);
+    }
 
     builder.add(
         Expressions.return_(null,
             Expressions.call(
-                BuiltInMethod.SINGLETON_ENUMERABLE.method, list_)));
+                BuiltInMethod.SINGLETON_ENUMERABLE.method, collectionExpr)));
+
     return implementor.result(physType, builder.toBlock());
   }
 }

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

@@ -148,6 +148,7 @@ public static Collect create(RelNode input,
     RelDataType rowType;
     switch (sqlKind) {
     case ARRAY_QUERY_CONSTRUCTOR:
+    case MAP_QUERY_CONSTRUCTOR:
     case MULTISET_QUERY_CONSTRUCTOR:
       rowType =
           deriveRowType(input.getCluster().getTypeFactory(), collectionType,

core/src/main/java/org/apache/calcite/sql/fun/SqlMapQueryConstructor.java

@@ -29,6 +29,6 @@ public class SqlMapQueryConstructor extends SqlMultisetQueryConstructor {
   //~ Constructors -----------------------------------------------------------
 
   public SqlMapQueryConstructor() {
-    super("MAP", SqlKind.MAP_QUERY_CONSTRUCTOR, SqlTypeTransforms.TO_MAP);
+    super("MAP", SqlKind.MAP_QUERY_CONSTRUCTOR, SqlTypeTransforms.TO_MAP_QUERY);
   }
 }

core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java

@@ -273,6 +273,17 @@ private SqlTypeName toVar(RelDataType type) {
           SqlTypeUtil.createMapTypeFromRecord(opBinding.getTypeFactory(),
               typeToTransform);
 
+  /**
+   * Parameter type-inference transform strategy that converts a two-field
+   * record type to a MAP query type.
+   *
+   * @see org.apache.calcite.sql.fun.SqlMapQueryConstructor
+   */
+  public static final SqlTypeTransform TO_MAP_QUERY =
+      (opBinding, typeToTransform) ->
+          TO_MAP.transformType(opBinding,
+              SqlTypeUtil.deriveCollectionQueryComponentType(SqlTypeName.MAP, typeToTransform));
+
   /**
    * Parameter type-inference transform strategy that converts a type to a MAP type,
    * which key and value type is same.

core/src/main/java/org/apache/calcite/util/BuiltInMethod.java

@@ -289,6 +289,7 @@ public enum BuiltInMethod {
   AS_ENUMERABLE(Linq4j.class, "asEnumerable", Object[].class),
   AS_ENUMERABLE2(Linq4j.class, "asEnumerable", Iterable.class),
   ENUMERABLE_TO_LIST(ExtendedEnumerable.class, "toList"),
+  ENUMERABLE_TO_MAP(ExtendedEnumerable.class, "toMap", Function1.class, Function1.class),
   AS_LIST(Primitive.class, "asList", Object.class),
   MEMORY_GET0(MemoryFactory.Memory.class, "get"),
   MEMORY_GET1(MemoryFactory.Memory.class, "get", int.class),

core/src/test/resources/sql/sub-query.iq

@@ -3685,4 +3685,40 @@ FROM dept d1;
 
 !ok
 
+# [CALCITE-6041] MAP sub-query gives NullPointerException
+# map size > 1
+SELECT map(SELECT empno, deptno from emp where deptno < 20);
++-----------------------------+
+| EXPR$0                      |
++-----------------------------+
+| {7782=10, 7839=10, 7934=10} |
++-----------------------------+
+(1 row)
+
+!ok
+
+# [CALCITE-6041] MAP sub-query gives NullPointerException
+# map size = 1
+SELECT map(SELECT empno, deptno from emp where empno = 7369);
++-----------+
+| EXPR$0    |
++-----------+
+| {7369=20} |
++-----------+
+(1 row)
+
+!ok
+
+# [CALCITE-6041] MAP sub-query gives NullPointerException
+# empty map
+SELECT map(SELECT empno, deptno from emp where false);
++--------+
+| EXPR$0 |
++--------+
+| {}     |
++--------+
+(1 row)
+
+!ok
+
 # End sub-query.iq

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

@@ -10731,6 +10731,40 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
         "(CHAR(2) NOT NULL, DECIMAL(11, 1) NOT NULL) MAP NOT NULL");
   }
 
+  @Test void testMapQueryConstructor() {
+    final SqlOperatorFixture f = fixture();
+    f.setFor(SqlStdOperatorTable.MAP_QUERY, VmName.EXPAND);
+    // must be 2 fields
+    f.checkFails("map(select 1)", "MAP requires exactly two fields, got 1; "
+        + "row type RecordType\\(INTEGER EXPR\\$0\\)", false);
+    f.checkFails("map(select 1, 2, 3)", "MAP requires exactly two fields, got 3; "
+        + "row type RecordType\\(INTEGER EXPR\\$0, INTEGER EXPR\\$1, "
+        + "INTEGER EXPR\\$2\\)", false);
+    f.checkFails("map(select 1, 'x', 2, 'x')", "MAP requires exactly two fields, got 4; "
+        + "row type RecordType\\(INTEGER EXPR\\$0, CHAR\\(1\\) EXPR\\$1, INTEGER EXPR\\$2, "
+        + "CHAR\\(1\\) EXPR\\$3\\)", false);
+    f.checkScalar("map(select 1, 2)", "{1=2}",
+          "(INTEGER NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+    f.checkScalar("map(select 1, 2.0)", "{1=2.0}",
+        "(INTEGER NOT NULL, DECIMAL(2, 1) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map(select 1, true)", "{1=true}",
+        "(INTEGER NOT NULL, BOOLEAN NOT NULL) MAP NOT NULL");
+    f.checkScalar("map(select 'x', 1)", "{x=1}",
+        "(CHAR(1) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+    // element cast
+    f.checkScalar("map(select cast(1 as bigint), 2)", "{1=2}",
+        "(BIGINT NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+    f.checkScalar("map(select 1, cast(2 as varchar))", "{1=2}",
+        "(INTEGER NOT NULL, VARCHAR NOT NULL) MAP NOT NULL");
+    // null key or value
+    f.checkScalar("map(select 1, null)", "{1=null}",
+        "(INTEGER NOT NULL, NULL) MAP NOT NULL");
+    f.checkScalar("map(select null, 1)", "{null=1}",
+        "(NULL, INTEGER NOT NULL) MAP NOT NULL");
+    f.checkScalar("map(select null, null)", "{null=null}",
+        "(NULL, NULL) MAP NOT NULL");
+  }
+
   @Test void testCeilFunc() {
     final SqlOperatorFixture f = fixture();
     f.setFor(SqlStdOperatorTable.CEIL, VM_FENNEL);