Jefffrey commented on code in PR #22286:
URL: https://github.com/apache/datafusion/pull/22286#discussion_r3339761870
##########
datafusion/functions/src/regex/regexpinstr.rs:
##########
@@ -412,8 +408,22 @@ where
));
}
- // --- Simplified byte_start_offset calculation ---
Review Comment:
bit curious why we're removing these comments?
##########
datafusion/functions/src/regex/regexpinstr.rs:
##########
@@ -412,8 +408,22 @@ where
));
}
- // --- Simplified byte_start_offset calculation ---
let total_chars = value.chars().count() as i64;
+ if pattern.is_empty() {
+ compile_and_cache_regex(pattern, flags, regex_cache)?;
Review Comment:
is there a need for this compile+cache regex call here if the pattern is
empty and not being used?
##########
datafusion/functions/src/regex/regexpinstr.rs:
##########
@@ -762,6 +773,22 @@ mod tests {
});
}
+ fn test_regexp_instr_empty_pattern_global_flag() {
+ let args = [
+ ScalarValue::Utf8(Some("abc".to_string())),
+ ScalarValue::Utf8(Some("".to_string())),
+ ScalarValue::Int64(Some(1)),
+ ScalarValue::Int64(Some(1)),
+ ScalarValue::Utf8(Some("g".to_string())),
+ ];
+ let err = regexp_instr_with_scalar_values(&args)
+ .expect_err("global flag should be rejected for empty patterns");
Review Comment:
we reject global flag regardless of empty pattern, so not sure why this is
being added like so
--
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]
