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

Alessandro Solimando edited comment on CALCITE-4917 at 12/3/21, 9:41 AM:
-------------------------------------------------------------------------

I tried to add the test cases to the two methods were I initially looked for 
them, that is:
 # _testSimplifyAndIsNotNull_
 # _testSimplifyIsNotNull_

For 1. the problem is that it covers a specific issue from a ticket that has 
nothing to do with equality, the new test cases felt "buried" under the many 
lines of code of the existing issue, and not visible enough.

For 2., all the existing cases cover "IS NOT NULL" as top operator, new test 
cases would break homogeneity.

So I took a third way, merging the two test cases into a new test method 
mentioning "equality" explicitly; I have also added comments following your 
suggestion.

I have pushed the updated version in the PR.


was (Author: asolimando):
I tried to add the test cases to the two methods were I initially looked for 
them, that is:
 # _testSimplifyAndIsNotNull_
 # _testSimplifyIsNotNull_

For 1. the problem is that it covers a specific issue from a ticket that has 
nothing to do with equality, the new test cases felt "buried" under the many 
lines of code of the existing issue, and not visible enough.

For 2., all the existing cases cover "IS NOT NULL" as top operator, new test 
cases would break homogeneity.

So I took a third way, merging the two test cases into a new test method 
mentioning "equality" explicitly, the new version is in the PR.

> Add test for 'a IS NOT NULL AND a = b' simplification
> -----------------------------------------------------
>
>                 Key: CALCITE-4917
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4917
>             Project: Calcite
>          Issue Type: Test
>          Components: core
>    Affects Versions: 1.28.0
>            Reporter: Alessandro Solimando
>            Assignee: Alessandro Solimando
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> Simplification (from _RexSimplify_ class) is mostly covered in 
> {_}RexProgramTest{_}, but tests for expressions of the form "IS NOT NULL(a) 
> AND a=b" (with nullable and not nullable types) seem to be missing.
>  
> Since I had to write tests to make sure the simplification was as expected, I 
> assume others might end up doing the same, and that the tests will both act 
> as documentation and it will also protect against regressions.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to