zabetak commented on a change in pull request #898: [CALCITE-2464] Allow to set
nullability for columns of structured types (Ruben Quesada Lopez)
URL: https://github.com/apache/calcite/pull/898#discussion_r258525450
##########
File path:
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
##########
@@ -317,16 +320,12 @@ public RelDataType createTypeWithNullability(
} else if (type instanceof RelRecordType) {
// REVIEW: angel 18-Aug-2005 dtbug 336 workaround
// Changed to ignore nullable parameter if nullable is false since
- // copyRecordType implementation is doubtful
- if (nullable) {
- // Do a deep copy, setting all fields of the record type
- // to be nullable regardless of initial nullability
- newType = copyRecordType((RelRecordType) type, false, true);
- } else {
- // Keep same type as before, ignore nullable parameter
- // RelRecordType currently always returns a nullability of false
- newType = copyRecordType((RelRecordType) type, true, false);
- }
+ // copyRecordType implementation is doubtful:
+ // If nullable -> Do a deep copy, setting all fields of the record type
+ // to be nullable regardless of initial nullability
+ // If not nullable -> set not nullable at top RelRecordType level only,
Review comment:
According to the SQL standard nullability for struct types can be defined
only for **columns** which translates to top level structs. Nested struct
attributes are always nullable, so in principle we could always set the nested
attributes to be nullable. However, this might create regressions so we could
skip this for now. Should we at least update a bit the comment?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
