You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "lyne7-sc (via GitHub)" <gi...@apache.org> on 2026/06/03 02:53:48 UTC
[PR] fix: regex simplification of anchored patterns produces wrong results [datafusion]
lyne7-sc opened a new pull request, #22727:
URL: https://github.com/apache/datafusion/pull/22727
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Closes #22726.
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
The regex simplification rule rewrites anchored regex matches (`^literal$`, `^(a|b)$`) into cheaper `=` / `IN` / `LIKE` expressions. Two bugs in that path:
1. The literal was always built as `Utf8` via `lit(...)`, so on a `Utf8View` / `LargeUtf8` column the rewritten comparison failed at execution with `Invalid comparison operation: Utf8View == Utf8`.
2. A `~*` (case-insensitive) anchored literal was rewritten to a case-sensitive `=`, silently dropping rows that differ only in case.
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
- Build the extracted literal with `string_scalar.to_expr(...)` so its type follows the column type (`Utf8` / `LargeUtf8` / `Utf8View`), consistent with the existing `LIKE` branches.
- Rewrite `~*` anchored literals to `ILIKE` instead of `=`. The existing `is_safe_for_like` guard ensures the literal has no `%` / `_`, so this is an exact case-insensitive match. (Anchored alternations under `~*` still fall back to regex evaluation.)
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
Yes. `predicates.slt` now covers anchored `~` / `~*`, single literals and alternations, over both `Utf8` and `Utf8View` columns. Existing `regex.rs` unit tests still pass.
## Are there any user-facing changes?
Yes, bug fixes only
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#issuecomment-4663358467
Than you @lyne7-sc @martin-g and @Jefffrey
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#issuecomment-4648131314
FYI @Dandandan
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #22727:
URL: https://github.com/apache/datafusion/pull/22727
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "lyne7-sc (via GitHub)" <gi...@apache.org>.
lyne7-sc commented on PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#issuecomment-4665674538
Thank you all for the reviews!
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#discussion_r3354634910
##########
datafusion/sqllogictest/test_files/predicates.slt:
##########
@@ -204,12 +204,65 @@ SELECT * FROM test WHERE column1 ~ 'z'
----
Bazzz
+query T
+SELECT * FROM test WHERE column1 ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test WHERE column1 ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+statement ok
+CREATE TABLE test_regex_utf8view(s VARCHAR) AS VALUES ('foo'), ('Bazzz');
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+# Case-insensitive anchored match over Utf8View: must be simplified to ILIKE
+# (not a case-sensitive Eq) and must keep operand types as Utf8View.
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~* '^bazzz$'
+----
+Bazzz
+
+# Case-insensitive anchored alternation over Utf8View
+query T rowsort
+SELECT * FROM test_regex_utf8view WHERE s ~* '^(foo|bazzz)$'
+----
+Bazzz
+foo
+
+statement ok
+DROP TABLE test_regex_utf8view;
+
query T
SELECT * FROM test WHERE column1 ~* 'z'
----
Bazzz
ZZZZZ
+query T
+SELECT * FROM test WHERE column1 ~* '^barrr$'
+----
+Barrr
+
+query T
+SELECT * FROM test WHERE column1 ~* '^(barrr|bazzz)$'
Review Comment:
Tests with negation+regex are missing (`!~` and `!~*`).
##########
datafusion/sqllogictest/test_files/predicates.slt:
##########
@@ -204,12 +204,65 @@ SELECT * FROM test WHERE column1 ~ 'z'
----
Bazzz
+query T
+SELECT * FROM test WHERE column1 ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test WHERE column1 ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+statement ok
+CREATE TABLE test_regex_utf8view(s VARCHAR) AS VALUES ('foo'), ('Bazzz');
Review Comment:
Question to educate myself: How the values here are Utf8View ?
I'd expect some casting to achieve that.
##########
datafusion/sqllogictest/test_files/predicates.slt:
##########
@@ -204,12 +204,65 @@ SELECT * FROM test WHERE column1 ~ 'z'
----
Bazzz
+query T
+SELECT * FROM test WHERE column1 ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test WHERE column1 ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+statement ok
+CREATE TABLE test_regex_utf8view(s VARCHAR) AS VALUES ('foo'), ('Bazzz');
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+# Case-insensitive anchored match over Utf8View: must be simplified to ILIKE
+# (not a case-sensitive Eq) and must keep operand types as Utf8View.
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~* '^bazzz$'
+----
+Bazzz
Review Comment:
How this asserts the expected result ?
Neither the optimization nor the type is asserted.
Maybe use `EXPLAIN ...` and assert its output instead ?!
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "lyne7-sc (via GitHub)" <gi...@apache.org>.
lyne7-sc commented on code in PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#discussion_r3360172956
##########
datafusion/sqllogictest/test_files/predicates.slt:
##########
@@ -204,12 +204,65 @@ SELECT * FROM test WHERE column1 ~ 'z'
----
Bazzz
+query T
+SELECT * FROM test WHERE column1 ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test WHERE column1 ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+statement ok
+CREATE TABLE test_regex_utf8view(s VARCHAR) AS VALUES ('foo'), ('Bazzz');
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+# Case-insensitive anchored match over Utf8View: must be simplified to ILIKE
+# (not a case-sensitive Eq) and must keep operand types as Utf8View.
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~* '^bazzz$'
+----
+Bazzz
Review Comment:
sounds good to me, it makes the intent clearer. I've already added EXPLAIN assertions for all the anchored cases.
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "lyne7-sc (via GitHub)" <gi...@apache.org>.
lyne7-sc commented on code in PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#discussion_r3360132782
##########
datafusion/sqllogictest/test_files/predicates.slt:
##########
@@ -204,12 +204,65 @@ SELECT * FROM test WHERE column1 ~ 'z'
----
Bazzz
+query T
+SELECT * FROM test WHERE column1 ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test WHERE column1 ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+statement ok
+CREATE TABLE test_regex_utf8view(s VARCHAR) AS VALUES ('foo'), ('Bazzz');
Review Comment:
I think it's because the config `map_string_types_to_utf8view` defaults to `true`, so a `VARCHAR` column is planned as `Utf8View` in slt
https://github.com/apache/datafusion/blob/e1d8d463b51e67e777b3ef744e80fa75593b3e5b/datafusion/common/src/config.rs#L292-L295
--
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] fix: regex simplification of anchored patterns produces wrong results [datafusion]
Posted by "lyne7-sc (via GitHub)" <gi...@apache.org>.
lyne7-sc commented on code in PR #22727:
URL: https://github.com/apache/datafusion/pull/22727#discussion_r3360143705
##########
datafusion/sqllogictest/test_files/predicates.slt:
##########
@@ -204,12 +204,65 @@ SELECT * FROM test WHERE column1 ~ 'z'
----
Bazzz
+query T
+SELECT * FROM test WHERE column1 ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test WHERE column1 ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+statement ok
+CREATE TABLE test_regex_utf8view(s VARCHAR) AS VALUES ('foo'), ('Bazzz');
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^Bazzz$'
+----
+Bazzz
+
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~ '^(foo|Bazzz)$'
+----
+foo
+Bazzz
+
+# Case-insensitive anchored match over Utf8View: must be simplified to ILIKE
+# (not a case-sensitive Eq) and must keep operand types as Utf8View.
+query T
+SELECT * FROM test_regex_utf8view WHERE s ~* '^bazzz$'
+----
+Bazzz
+
+# Case-insensitive anchored alternation over Utf8View
+query T rowsort
+SELECT * FROM test_regex_utf8view WHERE s ~* '^(foo|bazzz)$'
+----
+Bazzz
+foo
+
+statement ok
+DROP TABLE test_regex_utf8view;
+
query T
SELECT * FROM test WHERE column1 ~* 'z'
----
Bazzz
ZZZZZ
+query T
+SELECT * FROM test WHERE column1 ~* '^barrr$'
+----
+Barrr
+
+query T
+SELECT * FROM test WHERE column1 ~* '^(barrr|bazzz)$'
Review Comment:
added `!~` and `!~*` coverage.
--
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