-
Notifications
You must be signed in to change notification settings - Fork 612
Parse exponent literal as number #768
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
Conversation
@@ -144,6 +144,7 @@ pub fn all_dialects() -> TestedDialects { | |||
Box::new(RedshiftSqlDialect {}), | |||
Box::new(MySqlDialect {}), | |||
Box::new(BigQueryDialect {}), | |||
Box::new(SQLiteDialect {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -617,6 +618,36 @@ impl<'a> Tokenizer<'a> { | |||
return Ok(Some(Token::Period)); | |||
} | |||
|
|||
// Parse exponent as number | |||
if chars.peek() == Some(&'e') || chars.peek() == Some(&'E') { | |||
let mut char_clone = chars.peekable.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this copy needed?
Given chars
is already peekable I don't see why it can't be used directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed a way to peek more than just the next char, since is only valid exponent if e
followed by optional sign and an actual number. Found easiest way was to simply clone the iter and use that, and if found not to be an exponent and safely discard it and continue regular behaviour with original iter.
Token::Comma, | ||
Token::Whitespace(Whitespace::Space), | ||
Token::Number(String::from("1e-10"), false), | ||
Token::make_word("a", None), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this very strange that a new token is formed without whitespace after a number. I expected that this is a token error but this implementation agrees with postgres 🤯
postgres=# select 12e-10a;
a
--------------
0.0000000012
(1 row)
postgres=# select 12e-10 a;
a
--------------
0.0000000012
(1 row)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise
postgres=# select 1e-10-10;
?column?
---------------
-9.9999999999
(1 row)
postgres=# select 1e-10 -10;
?column?
---------------
-9.9999999999
(1 row)
🤯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this behaviour is part of what bit me when trying to implement for Hive dialect 😅
Thank you @Jefffrey |
Pull Request Test Coverage Report for Build 3757065788
💛 - Coveralls |
Resolve part of #610
Allows parsing of exponent literals as numbers, provided the dialect does not allow an identifier to being with a number (Hive dialect). Will need more thinking on how to handle that specific case.