You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Vova Vysotskyi (Jira)" <ji...@apache.org> on 2019/11/01 15:59:00 UTC

[jira] [Comment Edited] (CALCITE-3457) RexSimplify incorrectly simplifies IS NOT NULL operator with ITEM call

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

Vova Vysotskyi edited comment on CALCITE-3457 at 11/1/19 3:58 PM:
------------------------------------------------------------------

This assertion should check the correspondence of strong policy and nullability of returning and argument types. The issue I have pointed above appeared because of actual nullability of the rex call was changed, but the previous nullability was passed when a call is recreated with simplified arguments.

I tried to avoid passing the wrong nullability (updated {{RexSimplify.simplifyGenericNode}} method), so it helped to fix this issue, but other tests failed because some functions like {{REINTERPRET}}, {{SqlDatetimeSubtractionOperator}} and other cannot infer return type using {{RexCallBinding}}. {{SqlReturnTypeInference}} for {{SqlDatetimeSubtractionOperator}} for example expects the additional argument with returning type, but corresponding rex call doesn't have this additional operand, only sql call has it.

So we can't specify the type with correct nullability to ensure that this assertion wouldn't have false-positive failures. Regarding the tweaking fix for CALCITE-2695, I don't see a way how to do it.


was (Author: vvysotskyi):
This assertion should check the correspondence of strong policy and nullability of returning and argument types. The issue I have pointed above appeared because of actual nullability of the rex call was changed, but the previous nullability was passed when a call is recreated with simplified arguments.

I tried to avoid passing the wrong nullability (updated {{RexSimplify.simplifyGenericNode}} method), so it helped to fix this issue, but other tests failed because some functions like {{REINTERPRET}}, {{SqlDatetimeSubtractionOperator}} and other cannot infer return type using {{RexCallBinding}}. {{SqlReturnTypeInference}} for {{SqlDatetimeSubtractionOperator}} for example expects the additional argument with returning type, but corresponding rex call doesn't have this additional operand, only sql call has it.

> RexSimplify incorrectly simplifies IS NOT NULL operator with ITEM call
> ----------------------------------------------------------------------
>
>                 Key: CALCITE-3457
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3457
>             Project: Calcite
>          Issue Type: Bug
>    Affects Versions: 1.22.0
>            Reporter: Vova Vysotskyi
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.22.0
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> In CALCITE-3390 ITEM was marked with {{Policy.ANY}} strong policy, but according to its JavaDoc, the result may be null if and only if at least one of its arguments is null. This statement was used in {{RexSimplify.simplifyIsNotNull()}} method, so {{t1.c_nationkey[0] is not null}} will be simplified to {{IS NOT NULL($0)}} which is wrong, since array may be empty, or index may be less than the size of the array.
> Unit test which helps to reproduce this issue:
> {noformat}
>   @Test public void testSimplifyItemIsNotNull() {
>     String query = "select * from sales.customer as t1 where t1.c_nationkey[0] is not null";
>     sql(query)
>         .withTester(t -> createDynamicTester())
>         .withRule(ReduceExpressionsRule.FILTER_INSTANCE)
>         .check();
>   }
> {noformat}
> Returns plan with incorrectly simplified ITEM expression:
> {noformat}
> LogicalProject(**=[$1])
>   LogicalFilter(condition=[IS NOT NULL($0)])
>     LogicalTableScan(table=[[CATALOG, SALES, CUSTOMER]])
> {noformat}
> But the initial intention of CALCITE-3390 was to allow pushing ITEM expression to the right input of left-outer-join.
> I propose to add a new element to the {{Policy}} which will have a relaxed condition - expression is null if at least one of its arguments is null.



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