Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support DECIMAL literals #3593

Closed
big-andy-coates opened this issue Oct 16, 2019 · 3 comments · Fixed by #4335
Closed

Support DECIMAL literals #3593

big-andy-coates opened this issue Oct 16, 2019 · 3 comments · Fixed by #4335
Milestone

Comments

@big-andy-coates
Copy link
Contributor

big-andy-coates commented Oct 16, 2019

Similar to #3591 but for DECIMALs.

We need a way for users to construct decimals. Currently, our SQL parsing rules handle DECIMAL creation:

DECIMAL_VALUE
    : DIGIT+ '.' DIGIT*
    | '.' DIGIT+
    | DIGIT+ ('.' DIGIT*)? EXPONENT
    | '.' DIGIT+ EXPONENT
    ;

However, this is converted to a DoubleLiteral in ParserUtil:

public static DoubleLiteral parseDecimalLiteral(final DecimalLiteralContext context) {
   ...
}

Given we don't support implicit conversion of DOUBLE -> DECIMAL, and any such conversion would be potentially lossy anyway, it means we don't have any way for users to create DECIMAL literals to use in INSERT VALUES or indeed to create DECIMALs to use in any other part of our SQL dialect.

We should look to change our current decimal parsing to return a DECIMAL literal, though doing so would be a breaking change without careful management. We can maybe look to add a compatibility breaking config to decide how decimals should be parsed, and then look to remove this option in the next major release.

@agavra
Copy link
Contributor

agavra commented Oct 16, 2019

Perhaps this isn't desired behavior, but you can currently use INSERT INTO with a decimal if you represent it as a string (e.g. INSERT INTO topic (decimalCol) VALUES ("1.12345"))

@big-andy-coates
Copy link
Contributor Author

We should consider make a breaking change in the next major version so that 12.4555 is a decimal literal, which can implicitly be converted to a double, and not a double, which a) might not be able to represent the number exactly and b) doesn't support implicit casting to decimal.

Marking for 6.0 release.

@big-andy-coates big-andy-coates added this to the 6.0 milestone Oct 18, 2019
@big-andy-coates
Copy link
Contributor Author

When we switch to proper decimal support we should remove the magic around string -> decimal coercion. e.g. in INSERT VALUES functionality, and in ExpectedRecordComparator.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants