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

[CALCITE-5999] DECIMAL literals are sometimes unparsed looking as DOUBLE literals #3422

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

mihaibudiu
Copy link
Contributor

No description provided.

@@ -90,7 +90,7 @@ public boolean isExact() {
@Override public String toValue() {
BigDecimal bd = getValueNonNull();
if (isExact) {
return getValueNonNull().toString();
return getValueNonNull().toPlainString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use bd.toPlainString()?

And the bd should be final.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I pushed these changes. Since the PR is so small I have already squashed the two commits.

@sonarcloud
Copy link

sonarcloud bot commented Sep 16, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@JiajunBernoulli JiajunBernoulli added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Sep 18, 2023
@mihaibudiu mihaibudiu changed the title [CALCITE-5999] DECIMAL literals as sometimes unparsed looking as DOUBLE literals [CALCITE-5999] DECIMAL literals are sometimes unparsed looking as DOUBLE literals Sep 18, 2023
@JiajunBernoulli JiajunBernoulli merged commit 008c553 into apache:main Sep 19, 2023
32 checks passed
@mihaibudiu mihaibudiu deleted the issue5999 branch September 19, 2023 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM-will-merge-soon Overall PR looks OK. Only minor things left.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants