You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org> on 2025/04/30 22:39:30 UTC
[PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
MohamedAbdeen21 opened a new pull request, #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835
The dialects that support underscore as a separator in numeric literals used to parse `._123` as a number, which is fine. But that means that something like `._abc` would be parsed as Number `._` and word `abc`, which is wrong.
This PR splits the tokenizer match branch for numbers and periods into two branches to make things easier, fixes the issue mentioned above, and adds tests.
CC: @mvzink
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2072413695
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
hmm most of this logic looks to already be duplicated on the [number parsing](https://github.com/apache/datafusion-sqlparser-rs/blob/f05913dd698a1fa2eeebf4f6a286f3cf19ddaff7/src/tokenizer.rs#L1224-L1276) code path, so that that side effect would be undesirable I think.
If I understood the issue being solved for, its only the case of `._` being parsed as a number, would it be possible/more-desirable to only update the existing logic to properly detect and handle that case or is the current logic not well equipped to handle that sanely?
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
Review Comment:
I wonder if its worth returning an error here or whether we lose anything by allowing the tokenizer continue? I'm guessing its still possible for the tokenizer to return `Token::Period` here as well?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2072423389
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
Review Comment:
AFAIK underscore after a dot can only be an identifier, so it makes sense to error here
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2080640522
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
Oh is that really the case that cloning the peekable iterator would clone the entire stream? (iirc it should only clone a struct with pointer offsets), the functionality is used in a few places in the tokenizer so I think it would be preferrable to avoid dup logic (if it turns out its an expensive thing to do then maybe we need to do a pass through the tokenizer to avoid that pattern overall, but that would be unrelated to this PR ofc)
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2082424681
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
Review Comment:
without the errors, '._123' as a whole will be parsed as a number, which is not valid in any standard AFAICT.
'._abc' as a whole would also be parsed as a word, which doesn't make sense because the prev token was not a word.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2072419795
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
The problem is the match happens on a peek and you need to consume the dot in order to peek the underscore.
What if the second peek wasn't an underscore? You need to un-consume the dot for it to be parsed as part of the 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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2072716889
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
Does the [peekable_clone](https://github.com/apache/datafusion-sqlparser-rs/blob/f05913dd698a1fa2eeebf4f6a286f3cf19ddaff7/src/tokenizer.rs#L1060-L1061) help in this case maybe? in order to lookahead without consuming characters
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2073850382
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
Would you prefer pulling the dup code into functions, if possible?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2072418073
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
I know. I tried to fix it without splitting the branches, but I couldn't.
Feel free to take a go at it if possible
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2072717456
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
Of course, but I prefer not cloning the entire token stream, especially since the dup code is only a few lines.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "MohamedAbdeen21 (via GitHub)" <gi...@apache.org>.
MohamedAbdeen21 commented on code in PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835#discussion_r2082425046
##########
src/tokenizer.rs:
##########
@@ -1281,20 +1262,91 @@ impl<'a> Tokenizer<'a> {
return Ok(Some(Token::make_word(s.as_str(), None)));
}
} else if prev_token == Some(&Token::Period) {
- // If the previous token was a period, thus not belonging to a number,
- // the value we have is part of an identifier.
+ // Handle as word if it follows a period
return Ok(Some(Token::make_word(s.as_str(), None)));
}
}
+ // Handle "L" suffix for long numbers
let long = if chars.peek() == Some(&'L') {
chars.next();
true
} else {
false
};
+
+ // Return the final token for the number
Ok(Some(Token::Number(s, long)))
}
+
+ // Period (`.`) handling
+ '.' => {
+ chars.next(); // consume the dot
+
+ match chars.peek() {
+ // Handle "._" case as a period followed by identifier
+ // if the last token was a word
+ Some('_') if matches!(prev_token, Some(Token::Word(_))) => {
+ Ok(Some(Token::Period))
+ }
+ Some('_') => {
+ self.tokenizer_error(
+ chars.location(),
+ "Unexpected underscore here".to_string(),
+ )
+ }
+ Some(ch)
+ // Hive and mysql dialects allow numeric prefixes for identifers
+ if ch.is_ascii_digit()
+ && self.dialect.supports_numeric_prefix()
+ && matches!(prev_token, Some(Token::Word(_))) =>
+ {
+ Ok(Some(Token::Period))
+ }
+ Some(ch) if ch.is_ascii_digit() => {
+ // Handle numbers starting with a dot (e.g., ".123")
+ let mut s = String::from(".");
+ let is_number_separator = |ch: char, next_char: Option<char>| {
+ self.dialect.supports_numeric_literal_underscores()
+ && ch == '_'
+ && next_char.is_some_and(|c| c.is_ascii_digit())
+ };
+
+ s += &peeking_next_take_while(chars, |ch, next_ch| {
+ ch.is_ascii_digit() || is_number_separator(ch, next_ch)
+ });
+
+ // Handle exponent part
+ if matches!(chars.peek(), Some('e' | 'E')) {
+ let mut exp = String::new();
+ exp.push(chars.next().unwrap());
+
+ if matches!(chars.peek(), Some('+' | '-')) {
+ exp.push(chars.next().unwrap());
+ }
+
+ if matches!(chars.peek(), Some(c) if c.is_ascii_digit()) {
+ exp += &peeking_take_while(chars, |c| c.is_ascii_digit());
+ s += &exp;
+ }
+ }
+
+ // Handle "L" suffix for long numbers
+ let long = if chars.peek() == Some(&'L') {
+ chars.next();
+ true
Review Comment:
updated
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: parsing ident starting with underscore in certain dialects [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #1835:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1835
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org