-
Notifications
You must be signed in to change notification settings - Fork 915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cudf::strings::replace_with_backrefs hang on empty match result #13418
Fix cudf::strings::replace_with_backrefs hang on empty match result #13418
Conversation
@andygrove verified this fixes the bug he reported in #13404 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you David for addressing this failure!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks!
@@ -61,14 +61,15 @@ struct backrefs_fn { | |||
auto out_ptr = d_chars ? (d_chars + d_offsets[idx]) : nullptr; | |||
size_type lpos = 0; // last byte position processed in d_str | |||
size_type begin = 0; // first character position matching regex | |||
size_type end = nchars; // last character position (exclusive) | |||
size_type end = -1; // last character position (exclusive) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size_type end = -1; // last character position (exclusive) | |
size_type end = -1; // last character position |
no longer relevant
/merge |
Description
Fixes bug where the
cudf::strings::replace_with_backrefs
goes into an infinite loop when an match results in an empty string. After each replace occurs, the logic continues to search for matches on the remainder of the string. Each new starting point must account for the previous match being empty.Also included a gtest for this case.
Closes #13404
Checklist