asolimando commented on code in PR #4139:
URL: https://github.com/apache/calcite/pull/4139#discussion_r1917235042
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java:
##########
@@ -191,6 +193,24 @@ 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
Review Comment:
```suggestion
* is always the same as the input. This differs from {@link
```
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java:
##########
@@ -191,6 +193,24 @@ 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
struct
+ * types. This function returns a nullable struct.
+ *
+ * @param type input type
Review Comment:
Nit: I see that some javadoc here has this kind of formatting with extra
spaces to align columns "horizontally", AFAICT this is not what we generally
use in the codebase, so I'd rather not enforce this here
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java:
##########
@@ -191,6 +193,24 @@ 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
struct
+ * types. This function returns a nullable struct.
Review Comment:
```suggestion
* types. This function returns a nullable struct.
```
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java:
##########
@@ -410,6 +410,31 @@ private RelDataType copyRecordType(
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 {
Review Comment:
Aren't there other non simple types like maps (`MultisetSqlType`)/arrays
(`ArraySqlType`)? Is it enough in this form?
EDIT: I have just seen you have done that for the `SqlTypeFactoryImpl`, why
here it is not needed?
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java:
##########
@@ -179,7 +179,9 @@ RelDataType createMultisetType(
* 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
Review Comment:
```suggestion
* is always the same as the input. This function never returns a nullable
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
