LucaCappelletti94 commented on code in PR #2894:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2894#discussion_r4515122215
##########
src/tokenizer.rs:
##########
Review Comment:
Two considerations:
1. I suggest to keep tokenizer bugs as spans bugs in a wishlist and handle
them when we will refactor those aspects of the library, as local solutions
tend to be patchy and the current state of both tokenizer and spans evidently
is an accumulation of maladaptive organic local patches. More patches are not
the solution.
2. You should find the closing tag with a lookahead copy of the tokenizer
state instead of collecting the rest of the input. Right now, every ClickHouse
`$tag$` copies everything after it into a `String`, including tags that do
close. That makes valid input quadratic. `SELECT $a$x$a$, ...` with 40,000
strings (360 KB) takes 8.1 s to tokenize, against 16 ms on `main`. With the
lookahead it takes 13 ms, and the PR's tests still pass. We really need to add
some criterion benchmarks in CI of sorts to catch these things but the current
solutions I am aware of have problems.
```suggestion
let mut temp = String::new();
let end_delimiter = format!("${value}$");
let mut lookahead = State {
peekable: chars.peekable.clone(),
line: chars.line,
col: chars.col,
};
loop {
match lookahead.next() {
Some(ch) => {
temp.push(ch);
if ch == '$' && temp.ends_with(&end_delimiter) {
if let Some(temp) =
temp.strip_suffix(&end_delimiter) {
s.push_str(temp);
}
break;
}
}
None => {
if temp.ends_with(&end_delimiter) {
if let Some(temp) =
temp.strip_suffix(&end_delimiter) {
s.push_str(temp);
}
break;
}
if
self.dialect.supports_unterminated_dollar_tag_as_word() {
let word = self.tokenize_word(end_delimiter,
chars);
return Ok(Token::make_word(&word, None));
}
return self.tokenizer_error(
lookahead.location(),
"Unterminated dollar-quoted, expected $",
);
}
}
}
*chars = lookahead;
```
--
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]