[GitHub] [calcite] yanlin-Lynn edited a comment on issue #1456: [CALCITE-3350] Keep same with origin type for RexLiteral when deserialized from json string

[GitBox <gi...@apache.org>]

yanlin-Lynn edited a comment on issue #1456: [CALCITE-3350] Keep same with origin type for RexLiteral when deserialized from json string
URL: https://github.com/apache/calcite/pull/1456#issuecomment-533401608
 
 
   Hi, @danny0405 , I agree that `CHAR(n) = VARCHAR` is a valid expression.
   I check the code, but I was hesitate to update.
   When convert call `job = "abd"` to RexNode in [converetCall](https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java#L761)
   The 'op' parameter is `=(SqlBinaryOperator)`, and 'operands' parameter is a List with two elements, one is `SqlIdentifier` for job column with type `VARCHAR(10)`, the other one is `SqlCharStringLiteral` for "abc" with type `CHAR(3)`.
   `=(SqlBinaryOperator)` has operandTypeChecker requires `LEAST_RESTRICTIVE` consistency, which cause the type coercion.
   I'm not sure that should I add a special check branch in [convertExpressionList](https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java#L789), when all nodes is string type, so that to just ignore the type coercion.
   Or shall we update the  operandTypeChecker for `=(SqlBinaryOperator)` to `COMPARE` consistency.
   Both of these two do not seem good enough for me, the first way breaks semantics of  `convertExpressionList` method. And the second, I worry it may cause other problems.
   Any suggestions? Thanks.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services