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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org