alamb commented on code in PR #21486:
URL: https://github.com/apache/datafusion/pull/21486#discussion_r3054196545
##########
datafusion/functions/src/regex/regexpreplace.rs:
##########
@@ -499,37 +568,13 @@ fn _regexp_replace_static_pattern_replace<T:
OffsetSizeTrait>(
let mut new_offsets = BufferBuilder::<T>::new(string_array.len() +
1);
new_offsets.append(T::zero());
- if let Some(ref short_re) = short_re {
Review Comment:
The idea was to encapsulate this logic so it was easer to understand and not
repeated in the two loops
##########
datafusion/functions/src/regex/regexpreplace.rs:
##########
@@ -201,6 +202,80 @@ fn regex_replace_posix_groups(replacement: &str) -> String
{
.into_owned()
}
+struct ShortRegex {
Review Comment:
The idea is to consolidate the logic for the special case short regexp
optimization so
1. It is easier to understand
2. It is not duplicated (and thus improve test coverage)
##########
datafusion/functions/src/regex/regexpreplace.rs:
##########
@@ -440,7 +515,7 @@ macro_rules! fetch_string_arg {
/// hold a single Regex object for the replace operation. This also speeds
/// up the pre-processing time of the replacement string, since it only
/// needs to processed once.
-fn _regexp_replace_static_pattern_replace<T: OffsetSizeTrait>(
+fn regexp_replace_static_pattern_replace<T: OffsetSizeTrait>(
Review Comment:
As a drive by fix, this function randomly was named starting with `_` which
is unecessary and unconventional
--
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]
