LucaCappelletti94 commented on code in PR #2494:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2494#discussion_r4093183982
##########
src/parser/mod.rs:
##########
Review Comment:
You could split the loop out of `parse_subexpr` so that `parse_substring`
can resume it on the expression it already has.
```suggestion
debug!("prefix: {expr:?}");
self.parse_infix_chain(expr, precedence)
}
/// Applies infix operators binding tighter than `precedence` to `expr`.
fn parse_infix_chain(&mut self, mut expr: Expr, precedence: u8) ->
Result<Expr, ParserError> {
loop {
let next_precedence = self.get_next_precedence()?;
debug!("next precedence: {next_precedence:?}");
if precedence >= next_precedence {
break;
}
// The period operator is handled exclusively by the
// compound field access parsing.
if Token::Period == self.peek_token_ref().token {
break;
}
expr = self.parse_infix(expr, next_precedence)?;
}
Ok(expr)
}
```
##########
src/parser/mod.rs:
##########
@@ -3041,7 +3044,25 @@ impl<'a> Parser<'a> {
}
};
self.expect_token(&Token::LParen)?;
- let expr = self.parse_expr()?;
+ // Parse at `LIKE` precedence so a bare `SIMILAR` isn't swallowed as
the start of a
+ // `SIMILAR TO` operator, allowing it to be recognized as the
substring syntax below.
+ let expr =
self.parse_subexpr(self.dialect.prec_value(Precedence::Like))?;
+
+ if self.parse_keyword(Keyword::SIMILAR) {
+ let from_expr = self.parse_expr()?;
+ self.expect_keyword_is(Keyword::ESCAPE)?;
+ let to_expr = self.parse_expr()?;
+ self.expect_token(&Token::RParen)?;
+ return Ok(Expr::Substring {
+ expr: Box::new(expr),
+ substring_from: Some(Box::new(from_expr)),
+ substring_for: Some(Box::new(to_expr)),
+ special: false,
+ shorthand,
+ similar: true,
+ });
+ }
Review Comment:
You should stop only at a bare `SIMILAR` and then resume the infix chain.
Parsing the source at `LIKE` precedence drops every lower-binding operator,
so `SUBSTRING(a IS NULL FROM 1)` and `SUBSTRING(a LIKE 'x' FROM 1)` would fail
in all dialects, and so do `SUBSTRING(1 = 1 FROM 1)` and `SUBSTRING(a ~ 'x'
FROM 1)` in Postgres.
`SUBSTRING(a SIMILAR TO 'x' FROM 1)` also fails because `SIMILAR TO` gets
taken for the substring form.
All of these parse on `main`. I would have hoped for the fuzzer to catch
these, but maybe there are still way too many errors in main before it can
actually function well enough.
```suggestion
// Stop before `SIMILAR` so the substring form can claim it, then
resume the infix chain.
let expr =
self.parse_subexpr(self.dialect.prec_value(Precedence::Like))?;
if self.peek_keyword(Keyword::SIMILAR)
&& !matches!(&self.peek_nth_token_ref(1).token, Token::Word(w)
if w.keyword == Keyword::TO)
{
self.advance_token();
let from_expr = self.parse_expr()?;
self.expect_keyword_is(Keyword::ESCAPE)?;
let to_expr = self.parse_expr()?;
self.expect_token(&Token::RParen)?;
return Ok(Expr::Substring {
expr: Box::new(expr),
substring_from: Some(Box::new(from_expr)),
substring_for: Some(Box::new(to_expr)),
special: false,
shorthand,
similar: true,
});
}
let expr = self.parse_infix_chain(expr,
self.dialect.prec_unknown())?;
```
##########
tests/sqlparser_common.rs:
##########
@@ -8425,6 +8425,11 @@ fn parse_substring() {
verified_stmt("SELECT SUBSTRING('foo' FROM 1 FOR 2) FROM t");
verified_stmt("SELECT SUBSTR('foo' FROM 1 FOR 2) FROM t");
verified_stmt("SELECT SUBSTR('foo', 1, 2) FROM t");
+ verified_stmt("SELECT SUBSTRING('1' SIMILAR '_' ESCAPE '#')");
Review Comment:
You may want to pin both regressions. On the current head the test fails
with `Expected: ), found: IS`.
```suggestion
verified_stmt("SELECT SUBSTRING('1' SIMILAR '_' ESCAPE '#')");
verified_stmt("SELECT SUBSTRING(a IS NULL FROM 1)");
verified_stmt("SELECT SUBSTRING(a SIMILAR TO 'x' FROM 1)");
```
--
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]
