[CALCITE-6389] RexBuilder.removeCastFromLiteral does not preserve semantics for some… #3779
Conversation
mihaibudiu
changed the title
…antics for some types of literal Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
|
| return true; | ||
| } | ||
| value = value.stripTrailingZeros(); | ||
| if (value.scale() < 0) { |
There was a problem hiding this comment.
it would be great also to have a test with negative scale
There was a problem hiding this comment.
Frankly negative scales are something I don't understand well.
Many SQL dialects do not accept negative scales, but Calcite allegedly does, although there aren't many tests about that.
Maybe we can file a separate issue about testing negative scales in Calcite, I expect it may uncover quite a few bugs.
There was a problem hiding this comment.
I also think that whether negative scales are accepted or not should be a property of the type system.
There was a problem hiding this comment.
I haven't checked it however if you think that it is large area to work with, then separate issue could be probably also ok
There was a problem hiding this comment.
Yes, I think that many places in Calcite assume implicitly that decimal scales cannot be negative.
There was a problem hiding this comment.
what do we do about this PR in particular?
There was a problem hiding this comment.
as you mentioned if there is a separate issue for negative scale is required, could you please file one in Calcite jira?
All the rest is ok to me, thanks
… types of literal
There are at least two other issues that I attempted to fix which uncover this bug, so I decided to make a separate PR for fixing it. This should make the other PRs simpler and less controversial.