[jira] [Commented] (CALCITE-3029) Java-oriented field type is wrongly forced to be NOT NULL after being converted to SQL-oriented

[Hongze Zhang (JIRA)]

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

Hongze Zhang commented on CALCITE-3029:
---------------------------------------

Also, I noticed that we introduced an argument named {{mustSetNullability}} to 
method {{JavaTypeFactoryImpl#toSql}}. Currently I am not able to understand the 
argument in deep so it would be great if someone could help to explain a bit 
more. Below is what the code looks like:

{code:java}
private static RelDataType toSql(final RelDataTypeFactory typeFactory,
    RelDataType type, boolean mustSetNullability) {
  RelDataType sqlType = type;
  if (type instanceof RelRecordType) {
    // We do not need to change the nullability of the nested fields,
    // since it can be overridden by the existing implementation of 
createTypeWithNullability
    // when we treat the nullability of the root struct type.
    sqlType = typeFactory.createStructType(
            Lists.transform(type.getFieldList(),
              field -> toSql(typeFactory, field.getType(), false)),
            type.getFieldNames());
  } else if (type instanceof JavaType) {
    sqlType = typeFactory.createSqlType(type.getSqlTypeName());
  }
  return mustSetNullability
          ? typeFactory.createTypeWithNullability(sqlType, type.isNullable())
          : sqlType;
}
{code}

I tried to change it to:

{code:java}
public static RelDataType toSql(final RelDataTypeFactory typeFactory,
    RelDataType type) {
  if (type instanceof RelRecordType) {
    return typeFactory.createTypeWithNullability(
        typeFactory.createStructType(
            Lists.transform(type.getFieldList(),
                field -> toSql(typeFactory, field.getType())),
            type.getFieldNames()),
        type.isNullable());
  }
  if (type instanceof JavaType) {
    return typeFactory.createTypeWithNullability(
        typeFactory.createSqlType(type.getSqlTypeName()),
        type.isNullable());
  }
  return type;
}
{code}

And it does not break any test cases. Do we really need such a boolean flag? I 
only modified the branch under condition {{if (type instanceof JavaType)}} to 
make sure least code is affected, but seems to be good if we could do more to 
simplify things.

> Java-oriented field type is wrongly forced to be NOT NULL after being 
> converted to SQL-oriented
> -----------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-3029
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3029
>             Project: Calcite
>          Issue Type: Bug
>    Affects Versions: 1.19.0
>            Reporter: Hongze Zhang
>            Assignee: Hongze Zhang
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.20.0
>
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> A Java-oriented field type loses its nullable constraint after calling method 
> {{org.apache.calcite.jdbc.JavaTypeFactoryImpl#toSql(org.apache.calcite.rel.type.RelDataType)}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)