You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "BenSatori (via GitHub)" <gi...@apache.org> on 2026/09/10 15:55:29 UTC
[PR] Postgres: Support SUBSTRING SIMILAR/ESCAPE syntax and reversed FOR/FROM order [datafusion-sqlparser-rs]
BenSatori opened a new pull request, #2494:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2494
Fixes parsing of two legal Postgres `SUBSTRING` forms:
- `SUBSTRING('hello world' FOR 5 FROM 1)` (`FOR` before `FROM`)
- `SUBSTRING('hello world' SIMILAR '%#"o w#"%' ESCAPE '#')`
See https://www.postgresql.org/docs/current/functions-string.html
Both previously failed with a parser error. Adds a `similar` flag to `Expr::Substring` to track the `SIMILAR ... ESCAPE` display form, and allows `FOR` to be parsed before `FROM` (normalized to `FROM ... FOR` on output).
Generated with AI assistance (GitHub Copilot).
--
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] Postgres: Support SUBSTRING SIMILAR/ESCAPE syntax and reversed FOR/FROM order [datafusion-sqlparser-rs]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2494:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2494#issuecomment-5803831698
## [Codecov](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2494?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `68.42105%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.95%. Comparing base ([`9ae00e7`](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/commit/9ae00e73b54ebbca2341c3de22312a8011e98757?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`edd19dd`](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/commit/edd19dd3256723df35df80a0ee4d1ed20dfb3ee6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2494?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [src/parser/mod.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2494?src=pr&el=tree&filepath=src%2Fparser%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL3BhcnNlci9tb2QucnM=) | 68.42% | [0 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2494?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2494 +/- ##
==========================================
- Coverage 80.96% 80.95% -0.01%
==========================================
Files 42 42
Lines 33359 33377 +18
Branches 33359 33377 +18
==========================================
+ Hits 27009 27022 +13
Misses 2790 2790
- Partials 3560 3565 +5
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2494?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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] Postgres: Support SUBSTRING SIMILAR/ESCAPE syntax and reversed FOR/FROM order [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
LucaCappelletti94 commented on code in PR #2494:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2494#discussion_r4301995730
##########
tests/sqlparser_common.rs:
##########
@@ -8428,6 +8428,13 @@ 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(a IS NULL FROM 1)");
+ verified_stmt("SELECT SUBSTRING(a LIKE 'x' FROM 1)");
+ verified_stmt("SELECT SUBSTRING('1' SIMILAR '_' ESCAPE '#')");
+ one_statement_parses_to(
+ "SELECT SUBSTRING('1' FOR 3 FROM 1)",
+ "SELECT SUBSTRING('1' FROM 1 FOR 3)",
+ );
Review Comment:
You may want to pin the reversed order as a round trip. On the current head it fails with `left: "SELECT SUBSTRING('1' FOR 3 FROM 1)"`, `right: "SELECT SUBSTRING('1' FROM 1 FOR 3)"`.
```suggestion
verified_stmt("SELECT SUBSTRING('1' FOR 3 FROM 1)");
```
##########
src/ast/mod.rs:
##########
@@ -1211,6 +1215,11 @@ pub enum Expr {
/// true if the expression is represented using the `SUBSTR` shorthand
/// This flag is used for formatting.
shorthand: bool,
+
+ /// true if the expression uses the `SUBSTRING(expr SIMILAR pattern ESCAPE escape)` syntax,
+ /// in which case `substring_from` holds the pattern and `substring_for` holds the escape.
+ /// This flag is used for formatting.
+ similar: bool,
Review Comment:
Would a single syntax enum be better here, replacing `special` and `similar` with one variant each for the comma, `FROM`/`FOR`, `FOR`/`FROM` and `SIMILAR`/`ESCAPE` forms? `SUBSTRING('1' FOR 3 FROM 1)` now prints as `SUBSTRING('1' FROM 1 FOR 3)`, which breaks the round trip the README promises, and a third flag for the order would leave combinations such as `special` with `similar` representable.
--
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] Postgres: Support SUBSTRING SIMILAR/ESCAPE syntax and reversed FOR/FROM order [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
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: 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