diff --git a/datafusion/functions/src/regex/regexpreplace.rs b/datafusion/functions/src/regex/regexpreplace.rs index 8b6dc997a9166..f5bd6f182ac4b 100644 --- a/datafusion/functions/src/regex/regexpreplace.rs +++ b/datafusion/functions/src/regex/regexpreplace.rs @@ -409,19 +409,12 @@ where let replacement = regex_replace_posix_groups(replacement); // format flags into rust pattern - let (pattern, replace_all) = if flags == "g" { - (pattern.to_string(), true) - } else if flags.contains('g') { - ( - format!( - "(?{}){}", - flags.to_string().replace('g', ""), - pattern - ), - true, - ) + let replace_all = flags.contains('g'); + let flags = flags.replace('g', ""); + let pattern = if flags.is_empty() { + pattern.to_string() } else { - (format!("(?{flags}){pattern}"), false) + format!("(?{flags}){pattern}") }; // if patterns hashmap already has regexp then use else create and return @@ -531,11 +524,16 @@ fn regexp_replace_static_pattern_replace( // whether this is a global match (as in replace all) or just a single // replace operation. let (pattern, limit) = match flags { - Some("g") => (pattern.to_string(), 0), - Some(flags) => ( - format!("(?{}){}", flags.to_string().replace('g', ""), pattern), - !flags.contains('g') as usize, - ), + Some(flags) => { + let limit = !flags.contains('g') as usize; + let flags = flags.replace('g', ""); + let pattern = if flags.is_empty() { + pattern.to_string() + } else { + format!("(?{flags}){pattern}") + }; + (pattern, limit) + } None => (pattern.to_string(), 1), }; @@ -924,6 +922,44 @@ mod tests { assert_eq!(re.as_ref(), &expected); } + #[test] + fn test_static_pattern_regexp_replace_empty_flags() { + // An empty flags string must behave like no flags at all, matching + // `compile_regex` in this module, which is used by the other regexp + // functions. + let values = StringArray::from(vec!["abc"; 3]); + let patterns = StringArray::from(vec!["b"; 3]); + let replacements = StringArray::from(vec!["X"; 3]); + // `gg` collapses to an empty flags string once `g` is stripped. + let flags = StringArray::from(vec![Some(""), Some("g"), Some("gg")]); + let expected = StringArray::from(vec!["aXc"; 3]); + + let re = regexp_replace_static_pattern_replace::(&[ + Arc::new(values), + Arc::new(patterns), + Arc::new(replacements), + Arc::new(flags), + ]) + .unwrap(); + + assert_eq!(re.as_ref(), &expected); + } + + #[test] + fn test_regexp_replace_empty_flags() { + let values = StringArray::from(vec!["abc"; 3]); + let patterns = StringArray::from(vec!["b"; 3]); + let replacements = StringArray::from(vec!["X"; 3]); + let flags = StringArray::from(vec![Some(""), Some("g"), Some("gg")]); + let expected = StringArray::from(vec!["aXc"; 3]); + + let re = + regexp_replace::(&values, &patterns, &replacements, Some(&flags)) + .unwrap(); + + assert_eq!(re.as_ref(), &expected); + } + #[test] fn test_static_pattern_regexp_replace_pattern_error() { let values = StringArray::from(vec!["abc"; 5]); diff --git a/datafusion/sqllogictest/test_files/regexp/regexp_replace.slt b/datafusion/sqllogictest/test_files/regexp/regexp_replace.slt index e27ff1e9c1a00..e3b08d075b497 100644 --- a/datafusion/sqllogictest/test_files/regexp/regexp_replace.slt +++ b/datafusion/sqllogictest/test_files/regexp/regexp_replace.slt @@ -78,6 +78,26 @@ SELECT regexp_replace('foobarbaz', 'b..', 'X'); ---- fooXbaz +# An empty flags string behaves like no flags at all, as it does for the other +# regexp functions. `gg` leaves an empty flags string once `g` is stripped. +query T +SELECT regexp_replace('foobarbaz', 'b..', 'X', ''); +---- +fooXbaz + +query T +SELECT regexp_replace('foobarbaz', 'b..', 'X', 'gg'); +---- +fooXX + +query TI +SELECT + regexp_replace('foobarbaz', 'b..', 'X', flags), + regexp_count('foobarbaz', 'b..', 1, flags) +FROM (VALUES ('')) t(flags); +---- +fooXbaz 2 + query T SELECT regexp_replace('foobarbaz', 'b(..)', 'X\\1Y', 'g'); ----