Skip to content

parse decimals without leading integer, e.g. .1 #1407

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

Merged
merged 1 commit into from
Dec 30, 2019
Merged

parse decimals without leading integer, e.g. .1 #1407

merged 1 commit into from
Dec 30, 2019

Conversation

sploiselle
Copy link
Contributor

Moving MaterializeInc/sqlparser#33 over

Added test case to assuage concern of parsing something like ..07 as a valid number. ..07 doesn't pass parsing because there are no expressions that begin with with a period; this is the same as the current behavior when trying to parse .07, which results in a SQL parsing error:

ERROR: sql parser error: Expected an expression, found: .

@benesch lmk if you'd still like this refactored!

@sploiselle sploiselle requested a review from benesch December 30, 2019 15:51
@sploiselle sploiselle added A-sql Area: SQL planning C-feature Category: new feature or request labels Dec 30, 2019
@benesch
Copy link
Contributor

benesch commented Dec 30, 2019

Oh, sorry, I gave you a bad counterexample but I think the issue does exist. Specifically it seems like something like .0.7.7 gets parsed as 0.0 rather than rejected as a syntax error.

Expr::Value(Value::Number(bigdecimal::BigDecimal::from(.1)))
);

#[cfg(not(feature = "bigdecimal"))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, also, I deleted the bigdecimal feature, so if you remember you can drop the bigdecimal conditional here, and just delete the hunk above entirely.

Copy link
Contributor

@benesch benesch left a comment

Choose a reason for hiding this comment

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

🎉🎉 lgtm!

@benesch
Copy link
Contributor

benesch commented Dec 30, 2019

Although not to clippy, apologies.

@sploiselle sploiselle merged commit 436240d into MaterializeInc:master Dec 30, 2019
@sploiselle sploiselle deleted the parse-numbers-begin-decimal branch December 30, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql Area: SQL planning C-feature Category: new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants