iffyio commented on code in PR #1990: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1990#discussion_r2253656315
########## 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: Ah yeah that makes sense, I confused `peek_token_ref` and was hoping to avoid the clone entirely but looking at `Self::parse` now I see its not currently set up to take in references so the current approach to clone the string sounds reasonable! Here's a couple minor updates to your example I think should do what we intended? ```rust fn parse_signed_integer(&mut self) -> Result<i64, ParserError> { if self.consume_token(&Token::Minus) { return i64::try_from(self.parse_literal_uint()?) .map(|v| -v) .or_else(|_| self.expected_ref("i64 literal", self.peek_token_ref())) } let _ = self.consume_token(&Token::Plus); self.advance_token(); let next_token = self.get_current_token(); match &next_token.token { Token::Number(s, _) => Self::parse::<i64>(s.clone(), next_token.span.start), _ => self.expected_ref("literal int", next_token), } } ``` -- 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