0xdeafbeef opened a new pull request, #26831: URL: https://github.com/apache/datafusion/pull/26831
## Which issue does this PR close? No issue filed. The reproduction is below. ## Rationale for this change `Unparser::expr_to_sql` [turned `Expr::SimilarTo` into `LIKE`](https://github.com/apache/datafusion/blob/02b7149b5faae0dd2f8b6a6dea3dba7cfe1c69e8/datafusion/sql/src/unparser/expr.rs#L354-L368), so the unparsed SQL matched different rows. `LIKE` reads `(`, `|` and `)` as literal characters. `SIMILAR TO 'a(b|c)%'` matches strings that start with `ab` or `ac`; `LIKE 'a(b|c)%'` matches strings that start with the literal `a(b|c)`. On `main`, `generate_round_trip_statement(GenericDialect {}, sql)` from `datafusion/sql/tests/cases/plan_to_sql.rs` gives: ``` SELECT j1_string FROM j1 WHERE j1_string SIMILAR TO 'a(b|c)%' -> SELECT j1.j1_string FROM j1 WHERE j1.j1_string LIKE 'a(b|c)%' SELECT j1_string NOT SIMILAR TO '_b%' FROM j1 -> SELECT j1.j1_string NOT LIKE '_b%' FROM j1 ``` The unparser also dropped the `case_insensitive` field of `Expr::SimilarTo` silently. ## What changes are included in this PR? `Expr::SimilarTo` now unparses to `ast::Expr::SimilarTo`, which writes `SIMILAR TO` or `NOT SIMILAR TO`, with `ESCAPE` when the expression has an escape character. The unparser writes it for every dialect, as it already does for `ILIKE`. An `Expr::SimilarTo` with `case_insensitive: true` now returns `not_impl_err!("Unsupported expression: case-insensitive SIMILAR TO has no SQL syntax")`. SQL has no syntax for a case-insensitive `SIMILAR TO`, and the physical planner runs such an expression as a case-insensitive regex match (`RegexIMatch`, i.e. `~*`), so writing `SIMILAR TO` without the flag would change the result. The SQL planner always creates `Expr::SimilarTo` with `case_insensitive: false`, so only expressions built in Rust reach this error. An alternative is to write `~*` for dialects that have a case-insensitive regex operator; this PR leaves that out. ## What is the testing strategy for this PR? `roundtrip_statement` in `datafusion/sql/tests/cases/plan_to_sql.rs` gets three queries: ```sql select j1_string from j1 where j1_string SIMILAR TO 'a(b|c)%' select j1_string NOT SIMILAR TO '_b%' from j1 select j1_string from j1 where j1_string SIMILAR TO 'a!%%' ESCAPE '!' ``` Without the fix it fails on the first one: the original plan has `SimilarTo`, the plan of the unparsed SQL has `Like`. The `expr_to_sql_ok` case for `Expr::SimilarTo` came from #9902 together with the `LIKE` output and expected `a LIKE 'foo' ESCAPE 'o'` for `case_insensitive: true`. It now has `case_insensitive: false` and expects `a SIMILAR TO 'foo' ESCAPE 'o'`; a new case expects `a NOT SIMILAR TO 'foo'`. The new unit test `case_insensitive_similar_to_is_not_supported` checks the error. Run locally, not in CI, all passing: `cargo test -p datafusion-sql`, `cargo fmt --all -- --check`, `cargo clippy -p datafusion-sql --all-targets -- -D warnings` with and without `--all-features`, and `typos` on the two changed files. ## Are there any user-facing changes? The unparser writes `SIMILAR TO` instead of `LIKE` for `Expr::SimilarTo`, and returns an error for `case_insensitive: true`. No public API changes. -- 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]
