This is an automated email from the ASF dual-hosted git repository.
taoran pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new c5f3b8dcf1 [CALCITE-6041] MAP sub-query gives NullPointerException
c5f3b8dcf1 is described below
commit c5f3b8dcf14bd66d435a7fd3b6150ea1b3d3ccd7
Author: Ran Tao <[email protected]>
AuthorDate: Tue Oct 17 19:41:25 2023 +0800
[CALCITE-6041] MAP sub-query gives NullPointerException
---
.../adapter/enumerable/EnumerableCollect.java | 77 ++++++++++++++++------
.../java/org/apache/calcite/rel/core/Collect.java | 1 +
.../calcite/sql/fun/SqlMapQueryConstructor.java | 2 +-
.../apache/calcite/sql/type/SqlTypeTransforms.java | 11 ++++
.../org/apache/calcite/util/BuiltInMethod.java | 1 +
core/src/test/resources/sql/sub-query.iq | 36 ++++++++++
.../org/apache/calcite/test/SqlOperatorTest.java | 34 ++++++++++
7 files changed, 140 insertions(+), 22 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
index 64ead33167..d794dc6c4e 100644
---
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
@@ -19,6 +19,7 @@ package org.apache.calcite.adapter.enumerable;
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 class EnumerableCollect extends Collect implements
EnumerableRel {
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());
}
}
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Collect.java
b/core/src/main/java/org/apache/calcite/rel/core/Collect.java
index 6d5d06a2cf..88e2cc7927 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Collect.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Collect.java
@@ -148,6 +148,7 @@ public class Collect extends SingleRel {
RelDataType rowType;
switch (sqlKind) {
case ARRAY_QUERY_CONSTRUCTOR:
+ case MAP_QUERY_CONSTRUCTOR:
case MULTISET_QUERY_CONSTRUCTOR:
rowType =
deriveRowType(input.getCluster().getTypeFactory(), collectionType,
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlMapQueryConstructor.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlMapQueryConstructor.java
index bb8840889d..6b494be012 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlMapQueryConstructor.java
+++ b/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);
}
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java
b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java
index ad91c54b99..63c1e43559 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeTransforms.java
@@ -273,6 +273,17 @@ public abstract class SqlTypeTransforms {
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.
diff --git a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
index 78eaac9fc5..a5fc4b03e2 100644
--- a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
+++ b/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),
diff --git a/core/src/test/resources/sql/sub-query.iq
b/core/src/test/resources/sql/sub-query.iq
index 42bc10884a..0d1b165705 100644
--- a/core/src/test/resources/sql/sub-query.iq
+++ b/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
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index 9b5caad9ad..b0338bc759 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -10731,6 +10731,40 @@ public class SqlOperatorTest {
"(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);
