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

Oliver Lee commented on CALCITE-5607:
-------------------------------------

Hey [~julianhyde] ,

I finally got around to following up on this.

I see that in my change that CAST was actually [handled 
separately|https://github.com/apache/calcite/pull/3129/files#diff-673904825afdbc42629c1eeb5abc0d713687722fcdead867cdbe460ebddc1e9cL563]
 by adding in the "type" to the JSON serialization. 

My change took that part out of the switch-statement and made it happen for all 
{{{}RexCall{}}}s.

 

Now that I think about it, I should keep the switch-statement for the CAST and 
add in another clause specifically for {{SqlKind.MINUS}} , to not abandon 
deriving the type from the arguments for the rest of RexCalls.

If you could give me confirmation, I can go ahead and update the PR. 

> Serialize return type during RelJson.toJson(RexNode node) serialization 
> ------------------------------------------------------------------------
>
>                 Key: CALCITE-5607
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5607
>             Project: Calcite
>          Issue Type: Improvement
>            Reporter: Oliver Lee
>            Assignee: Oliver Lee
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> We found a bug in {{RelJson#toRex}} for the {{TIMESTAMP_DIFF}} call for Big 
> Query dialect.
> {{TIMESTAMP_DIFF}} is translated to the {{MINUS_DATE}} 
> [operator|https://github.com/apache/calcite/blob/c28d1dcbc34e748b7bea9712ef6bcf43793a91e8/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java#L2113-L2116]
>  with a return type explicitly declared as the interval.
> {{MINUS_DATE}} uses an 
> {{[ARG2_NULLABLE|https://github.com/apache/calcite/blob/c28d1dcbc34e748b7bea9712ef6bcf43793a91e8/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java#L241]}}
>  return type inference which requires 3 operands. This is fine in most cases 
> where the RexCall is then used to generate SQL or for native implementations.
> However, in {{{}RelJson#toRex{}}}, when it tries to reconstruct the entire 
> call to a RexNode, it attempts to derive the return type of the 
> {{MINUS_DATE}} operator using the {{ARG2_NULLABLE}} inference. This throws an 
> error as there are only 2 operands given to the {{MINUS_DATE}} operator.
> We'd like to now add in the "type" when serializing the JSON so that 
> {{[jsonType|https://github.com/apache/calcite/blob/c28d1dcbc34e748b7bea9712ef6bcf43793a91e8/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java#L712]}}
>  will be defined in {{{}toRex{}}}.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to