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]

Reply via email to