IndexSeek commented on code in PR #1990:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1990#discussion_r2255722803
##########
src/parser/mod.rs:
##########
@@ -11229,6 +11229,30 @@ impl<'a> Parser<'a> {
}
}
+ /// Parse an optionally signed integer literal.
+ fn parse_signed_integer(&mut self) -> Result<i64, ParserError> {
+ let next_token = self.next_token();
+ let (sign, number_token) = match next_token.token {
+ Token::Minus => {
+ let number_token = self.next_token();
+ (-1, number_token)
+ }
+ Token::Plus => {
+ let number_token = self.next_token();
+ (1, number_token)
+ }
+ _ => (1, next_token),
+ };
Review Comment:
Thanks for the suggestion! It does read cleaner not starting with negation,
looking back, opening with `if !self.consume_token(&Token::Minus)` felt a bit
strange.
I like how your solution still handles the + sign, but I noticed a few
issues with the logic flow. I think that with an optional plus token `let _ =
self.consume_token(&Token::Plus);`, it calls self.advance_token() and then
self.get_current_token(), which could advance twice and potentially skip the
actual number token.
I did take a look at `peek_token_ref` and was wondering if we could use it
like so?
```rs
fn parse_signed_integer(&mut self) -> Result<i64, ParserError> {
let is_negative = self.consume_token(&Token::Minus);
if !is_negative {
let _ = self.consume_token(&Token::Plus);
}
let current_token = self.peek_token_ref();
match ¤t_token.token {
Token::Number(s, _) => {
let s = s.clone();
let span_start = current_token.span.start;
self.advance_token();
let value = Self::parse::<i64>(s, span_start)?;
Ok(if is_negative { -value } else { value })
}
_ => self.expected_ref("number", current_token),
}
}
```
I think that should avoid us needing to consume the token until we know we
have a number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
