[
https://issues.apache.org/jira/browse/CALCITE-3457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16965312#comment-16965312
]
Vova Vysotskyi edited comment on CALCITE-3457 at 11/2/19 10:44 AM:
-------------------------------------------------------------------
I found the major difference between your code and my assertions.
I have added these validation checks inside {{simplifyIsNotNull}} and
{{simplifyIsNull}} methods, so they have applied after the simplification of
arguments, but you have added it to the {{simplifyIs2}} right before these
methods calls, so it is applied only to the arguments before simplification and
all these issues with the wrong nullability I mentioned before does not affect
the assertion. In this case, all tests pass even without additional
simplifications for the cast.
Thanks for looking into these issues!
I've pushed the code with assertions into the PR.
was (Author: vvysotskyi):
I found the major difference between your code and my assertions.
I have added these validation checks inside {{simplifyIsNotNull}} and
{{simplifyIsNull}} methods, so they have applied after the simplification of
arguments, but you have added it to the {{simplifyIs2}} right before these
methods calls, so it is applied only to the arguments before simplification. In
this case, all tests pass even without additional simplifications for the cast.
> 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: 40m
> 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)
