[ 
https://issues.apache.org/jira/browse/CALCITE-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Vladimir Sitnikov updated CALCITE-4331:
---------------------------------------
    Description: 
Calcite ignores data types completely if one of them is {{SqlTypeName.ANY}}.
That enables to transform rowtype from {{name ANY NON NULL}} to {{name ANY 
NULLABLE}} in a planner rule, even though, the planner does not allow to loosen 
field nullability.

What I got was silent wrong results {{count(name)}} was transformed to 
{{count(\*)}} since Calcite believed the field was non-nullable, and it did not 
care that I provided nullable rex for the implementation. Frankly speaking, I 
assumed "no asserts => the rowtypes in call.transformTo(..) are ok"

Current code:

{code:java}
    for (Pair<RelDataTypeField, RelDataTypeField> pair : Pair.zip(f1, f2)) {
      final RelDataType type1 = pair.left.getType();
      final RelDataType type2 = pair.right.getType();
      // If one of the types is ANY comparison should succeed
      if (type1.getSqlTypeName() == SqlTypeName.ANY
          || type2.getSqlTypeName() == SqlTypeName.ANY) {
        continue;
      }
      if (!type1.equals(type2)) {
        return false;
      }
{code}

I'm inclined add a check that both types have the same {{isNullable()}} value.

ANY checks were added in 
https://github.com/apache/calcite/commit/5562fc3952f61d943f109796c3d62b7390a8ac48

[~jacques], [~julianhyde], do you know if it is OK to add the following?

{code:java}
if (type1.isNullable() != type2.isNullable()) {
  return false;
}
{code}

  was:
Calcite ignores data types completely if one of them is {{SqlTypeName.ANY}}.
That enables to transform rowtype from {{name ANY NON NULL}} to {{name ANY 
NULLABLE}} in a planner rule, even though, the planner does not allow to loosen 
field nullability.

What I got was silent wrong results {{count(name)}} was transformed to 
{{count(*)}} since Calcite believed the field was non-nullable, and it did not 
care that I provided nullable rex for the implementation. Frankly speaking, I 
assumed "no asserts => the rowtypes in call.transformTo(..) are ok"

Current code:

{code:java}
    for (Pair<RelDataTypeField, RelDataTypeField> pair : Pair.zip(f1, f2)) {
      final RelDataType type1 = pair.left.getType();
      final RelDataType type2 = pair.right.getType();
      // If one of the types is ANY comparison should succeed
      if (type1.getSqlTypeName() == SqlTypeName.ANY
          || type2.getSqlTypeName() == SqlTypeName.ANY) {
        continue;
      }
      if (!type1.equals(type2)) {
        return false;
      }
{code}

I'm inclined add a check that both types have the same {{isNullable()}} value.

ANY checks were added in 
https://github.com/apache/calcite/commit/5562fc3952f61d943f109796c3d62b7390a8ac48

[~jacques], [~julianhyde], do you know if it is OK to add {{if 
(type1.isNullable() != type2.isNullable()) { return false; } }} ?


> RelOptUtil#areRowTypesEqual should compare type nullability even for ANY types
> ------------------------------------------------------------------------------
>
>                 Key: CALCITE-4331
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4331
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.26.0
>            Reporter: Vladimir Sitnikov
>            Priority: Major
>
> Calcite ignores data types completely if one of them is {{SqlTypeName.ANY}}.
> That enables to transform rowtype from {{name ANY NON NULL}} to {{name ANY 
> NULLABLE}} in a planner rule, even though, the planner does not allow to 
> loosen field nullability.
> What I got was silent wrong results {{count(name)}} was transformed to 
> {{count(\*)}} since Calcite believed the field was non-nullable, and it did 
> not care that I provided nullable rex for the implementation. Frankly 
> speaking, I assumed "no asserts => the rowtypes in call.transformTo(..) are 
> ok"
> Current code:
> {code:java}
>     for (Pair<RelDataTypeField, RelDataTypeField> pair : Pair.zip(f1, f2)) {
>       final RelDataType type1 = pair.left.getType();
>       final RelDataType type2 = pair.right.getType();
>       // If one of the types is ANY comparison should succeed
>       if (type1.getSqlTypeName() == SqlTypeName.ANY
>           || type2.getSqlTypeName() == SqlTypeName.ANY) {
>         continue;
>       }
>       if (!type1.equals(type2)) {
>         return false;
>       }
> {code}
> I'm inclined add a check that both types have the same {{isNullable()}} value.
> ANY checks were added in 
> https://github.com/apache/calcite/commit/5562fc3952f61d943f109796c3d62b7390a8ac48
> [~jacques], [~julianhyde], do you know if it is OK to add the following?
> {code:java}
> if (type1.isNullable() != type2.isNullable()) {
>   return false;
> }
> {code}



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

Reply via email to