[ 
https://issues.apache.org/jira/browse/CALCITE-2464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17140377#comment-17140377
 ] 

Ruben Q L commented on CALCITE-2464:
------------------------------------

[~danny0405], I think your example cannot be considered as a regression, 
because before this patch it was just not possible to set the record type (e.g. 
User) as nullable=true (all record types were implicitly nullable=false before).
The idea behind this patch was to allow setting nullability on record type 
level, without causing any regression in the previous behavior:
- a) If nullable = true -> Do a deep copy, setting all fields of the record 
type to be nullable regardless of initial nullability (new feature, thus IMHO 
impossible to consider as a regression).
- b) If nullable = false -> Do a deep copy, setting not nullable at top 
RelRecordType level only, keeping its fields' nullability as before (i.e. keep 
same behavior as before, precisely to avoid regressions).

Now, we could discuss if the new approach in a) (setting all fields of the 
record type to be nullable regardless of initial nullability) was more or less 
correct (the idea behind it was to reflect SQL standard regarding struct and 
struct fields nullability). Of course, we can discuss and improve this in a 
separate issue.



> Allow to set nullability for columns of structured types
> --------------------------------------------------------
>
>                 Key: CALCITE-2464
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2464
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.17.0
>            Reporter: Stamatis Zampetakis
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.19.0
>
>          Time Spent: 3h 10m
>  Remaining Estimate: 0h
>
> Struct types are always not nullable. This can lead to bugs in many parts of 
> Calcite (e.g., expression simplification, optimization, code generation) that 
> are considering the nullability of a RelDataType. 
> The method 
> [isNullable|https://github.com/apache/calcite/blob/3c6b5ec759caadabb67f09d7a4963cc7d9386d0c/core/src/main/java/org/apache/calcite/rel/type/RelRecordType.java#L55]
>  in the RelRecordType, which is used to represent a structured type, always 
> returns false. The nullability of the RelRecordType should be a parameter in 
> the constructor as it is the case for various other RelDataTypes. 
> Additionally, the data type cache should also take into account the 
> nullability of the type in order to return a correct equivalent.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to