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 a4cfbecc4d [CALCITE-6764] Field access from a nullable ROW should be
nullable (part 2)
a4cfbecc4d is described below
commit a4cfbecc4d4c5d2ddba9762086db2d5ca5a98362
Author: Mihai Budiu <[email protected]>
AuthorDate: Fri Jan 10 17:04:49 2025 -0800
[CALCITE-6764] Field access from a nullable ROW should be nullable (part 2)
Signed-off-by: Mihai Budiu <[email protected]>
---
.../calcite/rel/type/RelDataTypeFactory.java | 29 ++++-
.../calcite/rel/type/RelDataTypeFactoryImpl.java | 25 +++++
.../java/org/apache/calcite/rex/RexBuilder.java | 2 +-
.../org/apache/calcite/sql/type/BasicSqlType.java | 2 +-
.../calcite/sql/type/SqlTypeFactoryImpl.java | 22 ++++
.../calcite/sql/validate/SqlValidatorImpl.java | 8 ++
.../apache/calcite/test/TableInRootSchemaTest.java | 119 ++++++++++++++++-----
site/_docs/history.md | 10 ++
8 files changed, 186 insertions(+), 31 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java
b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java
index 9821451b9a..5912f78ac7 100644
--- a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java
+++ b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java
@@ -179,18 +179,37 @@ public interface RelDataTypeFactory {
* Creates a type that is the same as another type but with possibly
* different nullability. The output type may be identical to the input
* type. For type systems without a concept of nullability, the return value
- * is always the same as the input.
+ * is always the same as the input. This function never returns a nullable
+ * record type; when applied to a record type, it recursively makes all
fields
+ * nullable and the record itself is non-nullable.
*
- * @param type input type
- * @param nullable true to request a nullable type; false to request a NOT
- * NULL type
- * @return output type, same as input type except with specified nullability
+ * @param type input type
+ * @param nullable true to request a nullable type; false to request a NOT
NULL type.
+ * @return output type, same as input type except with specified nullability
(see description
+ * for record types).
* @throws NullPointerException if type is null
*/
RelDataType createTypeWithNullability(
RelDataType type,
boolean nullable);
+ /**
+ * Creates a type that is the same as another type but with possibly
+ * different nullability. The output type may be identical to the input
+ * type. For type systems without a concept of nullability, the return value
+ * is always the same as the input. This differs from {@link
+ * #createTypeWithNullability(RelDataType, boolean)} in the handling of
record
+ * types. This function returns a nullable struct.
+ *
+ * @param type input type
+ * @param nullable true to request a nullable type; false to request a NOT
NULL type.
+ * @return output type, same as input type except with specified nullability
+ * @throws NullPointerException if type is null
+ */
+ RelDataType enforceTypeWithNullability(
+ RelDataType type,
+ boolean nullable);
+
/**
* Creates a type that is the same as another type but with possibly
* different charset or collation. For types without a concept of charset or
diff --git
a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
index ed952fc5b0..89aac416be 100644
--- a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
@@ -410,6 +410,31 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
return canonize(newType);
}
+ @Override public RelDataType enforceTypeWithNullability(
+ final RelDataType type,
+ final boolean nullable) {
+ requireNonNull(type, "type");
+ RelDataType newType;
+ if (type.isNullable() == nullable) {
+ newType = type;
+ } else if (type instanceof RelRecordType) {
+ return createStructType(type.getStructKind(),
+ new AbstractList<RelDataType>() {
+ @Override public RelDataType get(int index) {
+ return type.getFieldList().get(index).getType();
+ }
+
+ @Override public int size() {
+ return type.getFieldCount();
+ }
+ },
+ type.getFieldNames(), nullable);
+ } else {
+ newType = copySimpleType(type, nullable);
+ }
+ return canonize(newType);
+ }
+
/**
* Registers a type, or returns the existing type if it is already
* registered.
diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
index 3cddb33a71..a7d5559c54 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
@@ -257,7 +257,7 @@ public class RexBuilder {
}
if (expr.getType().isNullable()) {
- fieldType = typeFactory.createTypeWithNullability(fieldType, true);
+ fieldType = typeFactory.enforceTypeWithNullability(fieldType, true);
}
return new RexFieldAccess(expr, field, fieldType);
}
diff --git a/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
b/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
index 9441303dad..e0642659f3 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java
@@ -126,7 +126,7 @@ public class BasicSqlType extends AbstractSqlType {
/**
* Constructs a type with nullablity.
*/
- BasicSqlType createWithNullability(boolean nullable) {
+ public BasicSqlType createWithNullability(boolean nullable) {
if (nullable == this.isNullable) {
return this;
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
index f71804e7a8..82fd14d6f4 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
@@ -253,6 +253,28 @@ public class SqlTypeFactoryImpl extends
RelDataTypeFactoryImpl {
return canonize(newType);
}
+ @Override public RelDataType enforceTypeWithNullability(
+ final RelDataType type,
+ final boolean nullable) {
+ final RelDataType newType;
+ if (type instanceof BasicSqlType) {
+ newType = ((BasicSqlType) type).createWithNullability(nullable);
+ } else if (type instanceof MapSqlType) {
+ newType = copyMapType(type, nullable);
+ } else if (type instanceof ArraySqlType) {
+ newType = copyArrayType(type, nullable);
+ } else if (type instanceof MultisetSqlType) {
+ newType = copyMultisetType(type, nullable);
+ } else if (type instanceof IntervalSqlType) {
+ newType = copyIntervalType(type, nullable);
+ } else if (type instanceof ObjectSqlType) {
+ newType = copyObjectType(type, nullable);
+ } else {
+ return super.enforceTypeWithNullability(type, nullable);
+ }
+ return canonize(newType);
+ }
+
private static void assertBasic(SqlTypeName typeName) {
assert typeName != null;
assert typeName != SqlTypeName.MULTISET
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 29b398a53c..dbf3d26863 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -7166,7 +7166,15 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
throw newValidationError(id.getComponent(i),
RESOURCE.unknownField(name));
}
+ boolean recordIsNullable = type.isNullable();
type = field.getType();
+ if (recordIsNullable) {
+ // If parent record is nullable, field must also be nullable.
+ // Consider CREATE TABLE T(p ROW(k VARCHAR) NULL);
+ // Since T.p is nullable, T.p.k also has to be nullable, even if
+ // the type of k itself is not nullable.
+ type = getTypeFactory().enforceTypeWithNullability(type, true);
+ }
}
type =
SqlTypeUtil.addCharsetAndCollation(
diff --git
a/core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java
b/core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java
index e8bc9b2e91..99d742cd44 100644
--- a/core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java
+++ b/core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java
@@ -87,29 +87,13 @@ class TableInRootSchemaTest {
connection.close();
}
- /**
- * Helper class for the test for [CALCITE-6764] below.
- */
- private static class RowTable extends AbstractQueryableTable {
- protected RowTable() {
+ /** Represents a table with no data. An abstract base class,
+ * derived classes need to define the schema. */
+ private abstract static class EmptyTable extends AbstractQueryableTable {
+ protected EmptyTable() {
super(Object[].class);
}
- @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
- final PairList<String, RelDataType> columnDesc =
PairList.withCapacity(1);
- // Schema contains a column whose type is MAP<VARCHAR, ROW(VARCHAR)>, but
- // the ROW type can be nullable.
- final RelDataType colType =
-
typeFactory.createMapType(typeFactory.createSqlType(SqlTypeName.VARCHAR),
- new RelRecordType(
- StructKind.PEEK_FIELDS,
- ImmutableList.of(
- new RelDataTypeFieldImpl("K", 0,
- typeFactory.createSqlType(SqlTypeName.VARCHAR))),
true));
- columnDesc.add("P", colType);
- return typeFactory.createStructType(columnDesc);
- }
-
@Override public <T> Queryable<T> asQueryable(
QueryProvider queryProvider, SchemaPlus schema, String tableName) {
return new AbstractTableQueryable<T>(queryProvider, schema, this,
@@ -134,14 +118,101 @@ class TableInRootSchemaTest {
}
}
- /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-6764";>[CALCITE-6764]
+ /** Helper class for the test for [CALCITE-6764] below. */
+ private static class TableWithNullableRowInMap extends EmptyTable {
+ protected TableWithNullableRowInMap() {
+ super();
+ }
+
+ @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+ final PairList<String, RelDataType> columnDesc =
PairList.withCapacity(1);
+ // Schema contains a column whose type is MAP<VARCHAR, ROW(VARCHAR)>, but
+ // the ROW type can be nullable. This can conceivably be created by a
+ // declaration such as
+ // CREATE TABLE T(P MAP<VARCHAR, ROW(K VARCHAR NON NULL, S VARCHAR
NULL)>);
+ final RelDataType colType =
+
typeFactory.createMapType(typeFactory.createSqlType(SqlTypeName.VARCHAR),
+ new RelRecordType(
+ StructKind.PEEK_FIELDS,
+ ImmutableList.of(
+ new RelDataTypeFieldImpl("K", 0,
+ typeFactory.createSqlType(SqlTypeName.VARCHAR)),
+ new RelDataTypeFieldImpl("S", 1,
+ typeFactory.createTypeWithNullability(
+ typeFactory.createSqlType(SqlTypeName.VARCHAR),
true))),
+ true));
+ columnDesc.add("P", colType);
+ return typeFactory.createStructType(columnDesc);
+ }
+ }
+
+ /** Helper class for the test for [CALCITE-6764] below. */
+ private static class TableWithNullableRowToplevel extends EmptyTable {
+ protected TableWithNullableRowToplevel() {
+ super();
+ }
+
+ @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+ final PairList<String, RelDataType> columnDesc =
PairList.withCapacity(1);
+ // This table can conceivably be created by a declaration such as
+ // CREATE TABLE T(P ROW(K VARCHAR NOT NULL) NULL,
+ // Q ROW(S ROW(L VARCHAR NOT NULL, M VARCHAR NULL) NULL)
NULL);
+ final RelDataType pColType =
+ new RelRecordType(
+ StructKind.PEEK_FIELDS, ImmutableList.of(
+ new RelDataTypeFieldImpl(
+ "K", 0, typeFactory.createSqlType(SqlTypeName.VARCHAR))),
+ true);
+ final RelDataType sType =
+ new RelRecordType(
+ StructKind.PEEK_FIELDS, ImmutableList.of(
+ new RelDataTypeFieldImpl(
+ "L", 0, typeFactory.createSqlType(SqlTypeName.VARCHAR)),
+ new RelDataTypeFieldImpl(
+ "M", 1, typeFactory.createSqlType(SqlTypeName.VARCHAR))),
+ false);
+ final RelDataType qColType =
+ new RelRecordType(
+ StructKind.PEEK_FIELDS, ImmutableList.of(
+ new RelDataTypeFieldImpl("S", 0, sType)),
+ true);
+ columnDesc.add("P", pColType);
+ columnDesc.add("Q", qColType);
+ return typeFactory.createStructType(columnDesc);
+ }
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6764";>[CALCITE-6764]
+ * Field access from a nullable ROW should be nullable</a>. */
+ @Test void testNullableRowInMap() throws Exception {
+ Connection connection = DriverManager.getConnection("jdbc:calcite:");
+ CalciteConnection calciteConnection =
connection.unwrap(CalciteConnection.class);
+ calciteConnection.getRootSchema().add("T", new
TableWithNullableRowInMap());
+ Statement statement = calciteConnection.createStatement();
+ // Without the fix to this issue the Validator crashes with an
AssertionFailure:
+ // java.lang.RuntimeException: java.lang.AssertionError:
+ // Conversion to relational algebra failed to preserve datatypes:
+ // validated type:
+ ResultSet resultSet = statement.executeQuery("SELECT P['a'].K, P['a'].S
FROM T");
+ resultSet.close();
+ statement.close();
+ connection.close();
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6764";>[CALCITE-6764]
* Field access from a nullable ROW should be nullable</a>. */
- @Test void testNullableValue() throws Exception {
+ @Test void testNullableRowTopLevel() throws Exception {
Connection connection = DriverManager.getConnection("jdbc:calcite:");
CalciteConnection calciteConnection =
connection.unwrap(CalciteConnection.class);
- calciteConnection.getRootSchema().add("T", new RowTable());
+ calciteConnection.getRootSchema().add("T", new
TableWithNullableRowToplevel());
Statement statement = calciteConnection.createStatement();
- ResultSet resultSet = statement.executeQuery("SELECT P['a'].K FROM T");
+ // Without the fix to this issue the Validator crashes with an
AssertionFailure:
+ // java.lang.RuntimeException: java.lang.AssertionError:
+ // Conversion to relational algebra failed to preserve datatypes:
+ // validated type:
+ ResultSet resultSet = statement.executeQuery("SELECT T.P.K, T.Q.S,
T.Q.S.L, T.Q.S.M FROM T");
resultSet.close();
statement.close();
connection.close();
diff --git a/site/_docs/history.md b/site/_docs/history.md
index feaa06069d..b18f414d46 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -49,6 +49,16 @@ other software versions as specified in gradle.properties.
#### Breaking Changes
{: #breaking-1-39-0}
+*RelDataTypeFactory interface*. The fix for [<a
+href="https://issues.apache.org/jira/browse/CALCITE-6764";>CALCITE-6764</a>]
+introduces a new method
+`RelDataTypeFactory#enforceTypeWithNullability` in the existing
+`RelDataTypeFactory` interface. The behavior of the new function is
+similar to the existing API `createTypeWithNullability`; however, the
+existing implementations of the `createTypeWithNullability` API cannot
+create nullable record (`ROW`) types. Nullable record types are
+legitimate in several SQL dialects.
+
*Checked arithmetic*. [<a
href="https://issues.apache.org/jira/browse/CALCITE-6685";>CALCITE-6685</a>]
introduces support for checked arithmetic on short integer types. A
