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

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

Hey [~julianhyde]  I can fix that this week

 

For the Jira summary, I was trying to follow the format and brevity in the test 
case above it 
[here|https://github.com/apache/calcite/blob/7049f9a33501c199fe04b589dd067adcae9f1ee7/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java#L1031]

Are you saying I should have the full correct commit message summary in the 
test case javadoc too? 

 

 

Regarding the point about datetime minus how is this for the commit message:

 
CALCITE-5607 Serialize return type during RelJson.toJson(RexNode nOde) for 
SqlKind.MINUS
 
Uncovered a bug in RelJson#toRex for the TIMESTAMP_DIFF call for Big Query 
dialect.
When serialize datetime minus 
({{{}SqlDatetimeSubtractionOperator.MINUS_DATE{}}}), the operator uses an 
ARG2_NULLABLE return type inference which requires 3 operands, however in 
BigQuery syntax there are only 2 operands. The solution here is to do something 
similar to how we handle CAST and to add in "type" when serializing to JSON in 
RelJson.toJson(RexNode node) for SqlKind.MINUS so that jsonType will be defined 
in toRex.
 

> Error when deserializing TIMESTAMP_DIFF in RelJson#toJson
> ---------------------------------------------------------
>
>                 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
>             Fix For: 1.37.0
>
>          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.
>  
> The solution here is to do something similar to how we handle {{CAST}} and to 
> add in "type" when serializing to JSON in {{RelJson.toJson(RexNode node)}} 
> for {{SqlKind.MINUS}} 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