mihaibudiu commented on code in PR #4023:
URL: https://github.com/apache/calcite/pull/4023#discussion_r1821532490
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##########
@@ -280,6 +280,13 @@ protected boolean needToCast(SqlValidatorScope scope,
SqlNode node,
return false;
}
+ if (node.getKind() == SqlKind.LITERAL
Review Comment:
Is the plan bad because it's incorrect, or because it's not efficient?
I think correctness is not negotiable.
Efficiency can be improved, but never at the cost of correctness.
One rule for correctness of a plan is that it typechecks.
In general, you should not remove a cast that may change the semantics of
the program.
A cast can, for example, cause a runtime exception, if the value does not
fit in the target type. It's not obvious to me that the cast you are removing
here is not of this kind.
The semantics of the literal is reflected not only in the value of this
particular literal, but also in its type, which may be propagated to other
expressions that make use of this literal.
Regarding the union of values, I have a PR in the works for
https://issues.apache.org/jira/browse/CALCITE-6350, which will insert even
*more* casts.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
