[jira] [Comment Edited] (CALCITE-4888) The type of generated search argument literal is wrong if the literal types of RelBuilder.in are different and compatible

["Jing Zhang (Jira)" <ji...@apache.org>]

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

Jing Zhang edited comment on CALCITE-4888 at 1/3/22, 6:11 AM:
--------------------------------------------------------------

Hi [~julianhyde]. 
It's my fault to introduce such a distracting test.
In fact, throwing ClassCastException is not related with this pull request. 
If types of the ranges are not compatible, 'RelBuilder#in' could generate a disjunction expression instead of a singleton expression with sars literal.
The exception is happened in the later phase, which is 'relBuilder#filter'. If simplify is enabled, RelBuilder would try to simplify filter predicates. For example, in the following example,  'RelBuilder#in' would generate a disjunction expression 'OR(=($2, 1), =($2, true))'.
'RelBuilder#filter' would thrown an expcetion in 'RexSimplify#SargCollector' because it try to cast a boolean value to int value when it simplify the generated disjunction expression ('OR(=($2, 1), =($2, true))').

{code:java}
RelNode root = b.scan("EMP")
          .filter(b.in(b.field(2), b.literal(1), b.literal(true)))
          .build();
{code}

In the last communication, I misunderstood that you prefer to update the ClassCastException to IllegalArgumentException in this case.

After I review the problem again, I think I should not change the 'RexSargBuilder' behavior. And remove the tests 'RelBuilder#testFilterInWithNotCompatibleArgumentsSimplification' and 'RelBuilder.testFilterInWithNotCompatibleArgumentsWithoutSimplification'.

WDYT?


was (Author: qingru zhang):
Hi [~julianhyde]. 
It's my fault to introduce such a distracting test.
In fact, throwing ClassCastException is not related with this pull request. 
If types of the ranges are not compatible, 'RelBuilder#in' could generate a disjunction expression instead of a singleton expression with sars literal.
The exception is happened in the later phase, which is 'relBuilder#filter'. If simplify is enabled, RelBuilder would try to simplify filter predicates. For example, in the following example,  'RelBuilder#in' would generate a disjunction expression 'OR(=($2, 1), =($2, true))'.
'RelBuilder#filter' would thrown an expcetion in 'RexSimplify#SargCollector' because it try to cast a boolean value to int value when it simplify the generated disjunction expression ('OR(=($2, 1), =($2, true))').

{code:java}
RelNode root = b.scan("EMP")
          .filter(b.in(b.field(2), b.literal(1), b.literal(true)))
          .build();
{code}

In the last communication, I misunderstood that you prefer to update the ClassCastException to IllegalArgumentException in this case.
So, after I review the problem again, I think I should not change the 'RexSargBuilder' behavior. And remove the tests 'RelBuilder#testFilterInWithNotCompatibleArgumentsSimplification' and 'RelBuilder.testFilterInWithNotCompatibleArgumentsWithoutSimplification'.

What do you think?


> The type of generated search argument literal is wrong if the literal types of RelBuilder.in are different and compatible
> -------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-4888
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4888
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Jing Zhang
>            Priority: Major
>              Labels: pull-request-available
>         Attachments: image-2021-11-23-10-31-05-137.png, screenshot-1.png, screenshot-2.png, screenshot-3.png
>
>          Time Spent: 2.5h
>  Remaining Estimate: 0h
>
> The type of resulted search argument literal is wrong if the literal types of RelBuilder.in are different and compatible with each other.
> For example, 
> {code:java}
>   /**
>    * Custom type system that converts different length of CHAR type to VARCHAR type.
>    */
>   public static class VaryingTypeSystem extends RelDataTypeSystemImpl {
>     public static final RelDataTypeSystem INSTANCE = new VaryingTypeSystem();
>     @Override public boolean shouldConvertRaggedUnionTypesToVarying() {
>       return true;
>     }
>   }
>     RelBuilder relBuilder = RelBuilder.create(config().typeSystem(VaryingTypeSystem.INSTANCE).build());
>     relBuilder.scan("EMP");
>     RexNode condition = relBuilder.in(relBuilder.field("JOB"), relBuilder.literal("CLERK"), relBuilder.literal("A"));
> {code}
> In the above example, we overwrite the shouldConvertRaggedUnionTypesToVarying to suggest the least restrictive type of a number of CHAR types of different lengths should be a VARCHAR type.
> We expect the type of the generated SARGS literal  to be VACHAR(5).
> Actually, the type of generated SARGS literal is CHAR(5) and the literal with value 'A' is converted to char(5), which leads to wrong result data.



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