This is an automated email from the ASF dual-hosted git repository.
mbudiu 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 53216ad33f [CALCITE-6813] UNNEST infers incorrect nullability for the
result when applied to an array that contains nullable ROW values
53216ad33f is described below
commit 53216ad33fd0c5ef53086ca32e1871eb21d8bed0
Author: Mihai Budiu <[email protected]>
AuthorDate: Tue Feb 4 10:35:54 2025 -0800
[CALCITE-6813] UNNEST infers incorrect nullability for the result when
applied to an array that contains nullable ROW values
Signed-off-by: Mihai Budiu <[email protected]>
---
.../org/apache/calcite/rel/core/Uncollect.java | 34 +++++++++++++++++-----
.../org/apache/calcite/sql/SqlUnnestOperator.java | 25 ++++++++++++++--
.../org/apache/calcite/test/SqlValidatorTest.java | 13 +++++++++
.../apache/calcite/test/SqlValidatorFixture.java | 15 ++++++++++
4 files changed, 76 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java
b/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java
index 48e6c8ec2a..7a6040d942 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java
@@ -166,23 +166,41 @@ 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();
- builder.add(SqlUnnestOperator.MAP_KEY_COLUMN_NAME,
mapType.getKeyType());
- builder.add(SqlUnnestOperator.MAP_VALUE_COLUMN_NAME,
mapType.getValueType());
+ 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);
} else {
- RelDataType ret = field.getType().getComponentType();
- if (null == ret) {
+ 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), ret);
- } else if (ret.isStruct()) {
- builder.addAll(ret.getFieldList());
+ builder.add(itemAliases.get(i), componentType);
+ } else if (componentType.isStruct()) {
+ for (RelDataTypeField fieldInfo : componentType.getFieldList()) {
+ RelDataType fieldType = fieldInfo.getType();
+ if (containerIsNullable || isNullable) {
+ fieldType = typeFactory.enforceTypeWithNullability(fieldType,
true);
+ }
+ builder.add(fieldInfo.getName(), fieldType);
+ }
} else {
// Element type is not a record, use the field name of the element
directly
- builder.add(field.getName(), ret);
+ builder.add(field.getName(), componentType);
}
}
}
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java
b/core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java
index d92eafe8ee..887bf1e6d8 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java
@@ -18,6 +18,7 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.sql.type.ArraySqlType;
import org.apache.calcite.sql.type.MapSqlType;
import org.apache.calcite.sql.type.MultisetSqlType;
@@ -81,15 +82,33 @@ 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;
- builder.add(MAP_KEY_COLUMN_NAME, mapType.getKeyType());
- builder.add(MAP_VALUE_COLUMN_NAME, mapType.getValueType());
+ 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);
} else {
RelDataType componentType = requireNonNull(type.getComponentType(),
"componentType");
+ boolean isNullable = componentType.isNullable();
if (!allowAliasUnnestItems(opBinding) && componentType.isStruct()) {
- builder.addAll(componentType.getFieldList());
+ for (RelDataTypeField field : componentType.getFieldList()) {
+ RelDataType fieldType = field.getType();
+ if (containerIsNullable || 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);
}
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 9fc0a0f5e3..0f0a9f9a84 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -1057,6 +1057,19 @@ void testDyadicCollateOperator() {
+ "Was expecting 3 arguments");
}
+ /** Test case for <a
href="https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6813";>
+ * [CALCITE-6813] UNNEST infers incorrect nullability for the result when
applied to
+ * an array that contains nullable ROW values</a>. */
+ @Test void testUnnestRow() {
+ sql("with orders(data) as\n"
+ + " (values (ARRAY[ROW(1, 'Alice'), ROW(2, NULL), ROW(NULL, 'Bob'),
NULL]))\n"
+ + "select e.EXPR$0\n"
+ + "from orders, UNNEST(orders.data) as e")
+ .type(actualType -> {
+ assertTrue(actualType.getFieldList().get(0).getType().isNullable());
+ });
+ }
+
@Test void testOverlay() {
expr("overlay('ABCdef' placing 'abc' from 1)").ok();
expr("overlay('ABCdef' placing 'abc' from 1 for 3)").ok();
diff --git
a/testkit/src/main/java/org/apache/calcite/test/SqlValidatorFixture.java
b/testkit/src/main/java/org/apache/calcite/test/SqlValidatorFixture.java
index c551587293..41f4d1c5da 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlValidatorFixture.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlValidatorFixture.java
@@ -51,6 +51,7 @@
import java.nio.charset.Charset;
import java.util.List;
+import java.util.function.Consumer;
import java.util.function.UnaryOperator;
import static com.google.common.base.Preconditions.checkArgument;
@@ -234,6 +235,20 @@ public SqlValidatorFixture type(String expectedType) {
return this;
}
+ /**
+ * Passes the returned type of a query to a consumer.
+ *
+ * @param check Consumer run on the specific data type.
+ * @return The fixture itself.
+ */
+ public SqlValidatorFixture type(Consumer<RelDataType> check) {
+ tester.validateAndThen(factory, sap, (sql, validator, n) -> {
+ RelDataType actualType = validator.getValidatedNodeType(n);
+ check.accept(actualType);
+ });
+ return this;
+ }
+
/**
* Checks that a query returns a single column, and that the column has the
* expected type. For example,
