Fix escaped like wildcards in like_utf8 / nlike_utf8 kernels
#2258
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character. This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not.
jhorstmann
reviewed
Aug 1, 2022
Codecov Report
@@ Coverage Diff @@
## master #2258 +/- ##
==========================================
- Coverage 82.29% 82.15% -0.15%
==========================================
Files 243 248 +5
Lines 62443 63537 +1094
==========================================
+ Hits 51387 52196 +809
- Misses 11056 11341 +285
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
alamb
approved these changes
Aug 2, 2022
There was a problem hiding this comment.
Thank you @daniel-martinez-maqueda-sap -- this looks like an improvement to me for sure. I left some suggestions but I think this PR is already better than master and I think it could be merged as is
| @@ -342,7 +377,7 @@ pub fn nlike_utf8_scalar<OffsetSize: OffsetSizeTrait>( | |||
| result.append(!left.value(i).ends_with(&right[1..])); | |||
| } | |||
| } else { | |||
| let re_pattern = escape(right).replace('%', ".*").replace('_', "."); | |||
| let re_pattern = replace_like_wildcards(right)?; | |||
There was a problem hiding this comment.
I really like how you have refactored this code 👍
alamb
reviewed
Aug 3, 2022
| @@ -298,10 +298,15 @@ pub fn like_utf8_scalar<OffsetSize: OffsetSizeTrait>( | |||
| Ok(BooleanArray::from(data)) | |||
| } | |||
|
|
|||
| fn replace_like_wildcards(text: &str) -> Result<String> { | |||
| /// Transforms a like `pattern` to a regex compatible pattern. To achieve that, it does: | |||
alamb
approved these changes
Aug 3, 2022
alamb
changed the title
like_utf8 / nlike_utf8 kernels
daniel-martinez-maqueda-sap
requested review from
alamb and
jhorstmann
and removed request for
jhorstmann
alamb
approved these changes
Aug 3, 2022
|
Benchmark runs are scheduled for baseline = 22185fd and contender = f78d2e6. f78d2e6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
MazterQyou
pushed a commit
to cube-js/arrow-rs
that referenced
this pull request
Nov 30, 2022
…che#2258) * Fix escaped like wildcards Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character. This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not. * Rewrite logic with peek after PR feedback * Simplifly logic * Add documentation and refactor string creation in tests * Add small fix and cargo fmt
MazterQyou
pushed a commit
to cube-js/arrow-rs
that referenced
this pull request
Nov 30, 2022
…che#2258) * Fix escaped like wildcards Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character. This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not. * Rewrite logic with peek after PR feedback * Simplifly logic * Add documentation and refactor string creation in tests * Add small fix and cargo fmt
mcheshkov
pushed a commit
to cube-js/arrow-rs
that referenced
this pull request
Aug 21, 2024
…che#2258) * Fix escaped like wildcards Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character. This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not. * Rewrite logic with peek after PR feedback * Simplifly logic * Add documentation and refactor string creation in tests * Add small fix and cargo fmt Can drop this after rebase on commit f78d2e6 "Fix escaped like wildcards in like_utf8 / nlike_utf8 kernels (apache#2258)", first released in 20.0.0
mcheshkov
pushed a commit
to cube-js/arrow-rs
that referenced
this pull request
Aug 21, 2024
…che#2258) * Fix escaped like wildcards Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character. This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not. * Rewrite logic with peek after PR feedback * Simplifly logic * Add documentation and refactor string creation in tests * Add small fix and cargo fmt Can drop this after rebase on commit f78d2e6 "Fix escaped like wildcards in like_utf8 / nlike_utf8 kernels (apache#2258)", first released in 20.0.0
mcheshkov
pushed a commit
to cube-js/arrow-rs
that referenced
this pull request
Aug 21, 2024
…che#2258) * Fix escaped like wildcards Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character. This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not. * Rewrite logic with peek after PR feedback * Simplifly logic * Add documentation and refactor string creation in tests * Add small fix and cargo fmt Can drop this after rebase on commit f78d2e6 "Fix escaped like wildcards in like_utf8 / nlike_utf8 kernels (apache#2258)", first released in 20.0.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #415
Rationale for this change
It is explained in the Issues linked above.
What changes are included in this PR?
Added a new function that replaces the like wildcards '%' and '_' for the regex counterparts before executing them. It also takes into account that the wildcards can be escaped, in that case, it does remove the escape characters and leaves the wildcards so that they are matched against the raw character.
This is implemented iterating over all the characters of the pattern to figure out when it needs to be transformed or not.
Are there any user-facing changes?
N/A