[
https://issues.apache.org/jira/browse/CALCITE-2464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16743210#comment-16743210
]
Ruben Quesada Lopez edited comment on CALCITE-2464 at 1/15/19 4:46 PM:
-----------------------------------------------------------------------
[~julianhyde], [~zabetak], thanks for your comments.
My initial ([PR#898|https://github.com/apache/calcite/pull/898/]) follows a
"conservative approach": struct types behave as before (they have by default
isNullable=false, i.e. NOT NULL constraint), but we can now create struct types
with isNullable=true via {{createTypeWithNullability(recordType, true)}} method
(which so far did not change the nullability of the struct type itself). This
means that, in order to have a nullable struct type, we'd need to go through
createTypeWithNullability, either explicitly or implicitly (e.g. during a join
result type computation).
However, the default behavior does not change, which means that the code
proposed by [~zabetak] in the comment above (or any similar test added to
type.iq) will still fail. In order to make it work, I think we would need to
change the default behavior, and start creating nullable struct types from the
beginning (perhaps the new default should be isNullable=true?), for example in
JavaTypeFactoryImpl.java:
{code}
public RelDataType createType(Type type) {
...
// return createStructType(clazz); // current code
return createTypeWithNullability(createStructType(clazz), true); // new code?
...
}
{code}
But this change would introduce some side effects (many unit tests fail, maybe
backwards compatibility might be an issue?). I am willing to carry on the task
and fix the tests that will need to be modified.
was (Author: rubenql):
[~julianhyde], [~zabetak], thanks for your comments.
My initial ([PR#898|https://github.com/apache/calcite/pull/898/]) follows a
"conservative approach": struct types behave as before (they have by default
isNullable=false, i.e. NOT NULL constraint), but we can now create struct types
with isNullable=true via {{createTypeWithNullability(recordType, true)}} method
(which so far did not change the nullability of the struct type itself). This
means that, in order to have a nullable struct type, we'd need to go through
createTypeWithNullability, either explicitly or implicitly (e.g. during a join
result type computation).
However, the default behavior does not change, which means that the code
proposed by [~zabetak] in the comment above (or any similar test added to
type.iq) will still fail. In order to make it work, I think we would need to
change the default behavior, and start creating nullable struct types from the
beginning (perhaps the new default should be isNullable=true?), for example in
JavaTypeFactoryImpl.java:
{code}
public RelDataType createType(Type type) {
...
// return createStructType(clazz); // old code
return createTypeWithNullability(createStructType(clazz), true); // new code?
...
}
{code}
But this change would introduce some side effects (many unit tests fail, maybe
backwards compatibility might be an issue?). I am willing to carry on the task
and fix the tests that will need to be modified.
> 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
> Assignee: Julian Hyde
> Priority: Major
>
> 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
(v7.6.3#76005)
