novain-shibu opened a new pull request, #55442: URL: https://github.com/apache/arrow/pull/55442
### Rationale for this change `replace_substring` with an empty pattern never returns. With an empty pattern, the search matches at the current position, the cursor advances by the pattern length (0), and with the default unlimited `max_replacements` the loop appends the replacement forever. Memory grows until the process is killed (about 100 MB/s on a one-element array). This still happens on main. `replace_substring_regex` has a related hang: with a pattern that can match empty text and a negative `max_replacements` other than -1 (for example -3), the counter never reaches 0 and the position doesn't advance. pandas worked around the first case on its side (pandas-dev/pandas#64941, pandas-dev/pandas#65147). ### What changes are included in this PR? - `replace_substring`: return `Invalid` when the pattern is empty. This follows the suggestion in #39264 to start with an error, since other systems disagree on the expected result: Python, Java and R's `gsub` insert the replacement at every position, Postgres leaves the string unchanged, and stringr raises. - `replace_substring_regex`: treat any negative `max_replacements` as unlimited, using the existing global-replace path that -1 already takes. - Tests for both cases. ### Are these changes tested? Yes. New cases in `scalar_string_test.cc`. On a Linux Release build of main (7299fdcc): - `arrow-compute-scalar-type-test`: 361/361 pass before and after the change - `*ReplaceSubstring*`: 12/12 pass with the change - with the new tests but the old kernel, the run hangs at the first empty-pattern case (killed after 240 s) ### Are there any user-facing changes? Yes. `replace_substring` with an empty pattern now raises `ArrowInvalid`. Before, it hung with unlimited replacements, and with a positive `max_replacements` it returned the replacement repeated at the start (for example `"-ab"`). Negative `max_replacements` other than -1 in `replace_substring_regex` now behaves like -1 instead of hanging. **This PR contains a "Critical Fix".** The empty-pattern case loops forever and allocates memory without bound, which can bring down the process. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed * GitHub Issue: #39149 -- 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]
