wiedld commented on code in PR #6849:
URL: https://github.com/apache/arrow-rs/pull/6849#discussion_r1893499529


##########
arrow-string/src/regexp.rs:
##########
@@ -497,114 +559,292 @@ pub fn regexp_match(
 mod tests {
     use super::*;
 
-    #[test]
-    fn match_single_group() {
-        let values = vec![
+    macro_rules! test_match_single_group {
+        ($test_name:ident, $values:expr, $patterns:expr, $arr_type:ty, 
$builder_type:ty, $expected:expr) => {
+            #[test]
+            fn $test_name() {
+                let array: $arr_type = <$arr_type>::from($values);
+                let pattern: $arr_type = <$arr_type>::from($patterns);
+
+                let actual = regexp_match(&array, &pattern, None).unwrap();
+
+                let elem_builder: $builder_type = <$builder_type>::new();
+                let mut expected_builder = ListBuilder::new(elem_builder);
+
+                for val in $expected {
+                    match val {
+                        Some(v) => {
+                            expected_builder.values().append_value(v);
+                            expected_builder.append(true);
+                        }
+                        None => expected_builder.append(false),
+                    }
+                }
+
+                let expected = expected_builder.finish();
+                let result = 
actual.as_any().downcast_ref::<ListArray>().unwrap();
+                assert_eq!(&expected, result);
+            }
+        };
+    }
+
+    test_match_single_group!(
+        match_single_group_string,
+        vec![
             Some("abc-005-def"),
             Some("X-7-5"),
             Some("X545"),
             None,
             Some("foobarbequebaz"),
             Some("foobarbequebaz"),
-        ];
-        let array = StringArray::from(values);
-        let mut pattern_values = vec![r".*-(\d*)-.*"; 4];
-        pattern_values.push(r"(bar)(bequ1e)");
-        pattern_values.push("");
-        let pattern = GenericStringArray::<i32>::from(pattern_values);
-        let actual = regexp_match(&array, &pattern, None).unwrap();
-        let elem_builder: GenericStringBuilder<i32> = 
GenericStringBuilder::new();
-        let mut expected_builder = ListBuilder::new(elem_builder);
-        expected_builder.values().append_value("005");
-        expected_builder.append(true);
-        expected_builder.values().append_value("7");
-        expected_builder.append(true);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        expected_builder.values().append_value("");
-        expected_builder.append(true);
-        let expected = expected_builder.finish();
-        let result = actual.as_any().downcast_ref::<ListArray>().unwrap();
-        assert_eq!(&expected, result);
-    }
+        ],
+        vec![
+            r".*-(\d*)-.*",
+            r".*-(\d*)-.*",
+            r".*-(\d*)-.*",
+            r".*-(\d*)-.*",
+            r"(bar)(bequ1e)",
+            ""
+        ],
+        StringArray,
+        GenericStringBuilder<i32>,
+        [Some("005"), Some("7"), None, None, None, Some("")]
+    );
+    test_match_single_group!(
+        match_single_group_string_view,
+        vec![
+            Some("abc-005-def"),
+            Some("X-7-5"),
+            Some("X545"),
+            None,
+            Some("foobarbequebaz"),
+            Some("foobarbequebaz"),
+        ],
+        vec![
+            r".*-(\d*)-.*",
+            r".*-(\d*)-.*",
+            r".*-(\d*)-.*",
+            r".*-(\d*)-.*",
+            r"(bar)(bequ1e)",
+            ""
+        ],
+        StringViewArray,
+        StringViewBuilder,
+        [Some("005"), Some("7"), None, None, None, Some("")]
+    );
+
+    macro_rules! test_match_single_group_with_flags {
+        ($test_name:ident, $values:expr, $patterns:expr, $flags:expr, 
$array_type:ty, $builder_type:ty, $expected:expr) => {
+            #[test]
+            fn $test_name() {
+                let array: $array_type = <$array_type>::from($values);
+                let pattern: $array_type = <$array_type>::from($patterns);
+                let flags: $array_type = <$array_type>::from($flags);
+
+                let actual = regexp_match(&array, &pattern, 
Some(&flags)).unwrap();
+
+                let elem_builder: $builder_type = <$builder_type>::new();
+                let mut expected_builder = ListBuilder::new(elem_builder);
+
+                for val in $expected {
+                    match val {
+                        Some(v) => {
+                            expected_builder.values().append_value(v);
+                            expected_builder.append(true);
+                        }
+                        None => {
+                            expected_builder.append(false);
+                        }
+                    }
+                }
 
-    #[test]
-    fn match_single_group_with_flags() {
-        let values = vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), 
None];
-        let array = StringArray::from(values);
-        let pattern = StringArray::from(vec![r"x.*-(\d*)-.*"; 4]);
-        let flags = StringArray::from(vec!["i"; 4]);
-        let actual = regexp_match(&array, &pattern, Some(&flags)).unwrap();
-        let elem_builder: GenericStringBuilder<i32> = 
GenericStringBuilder::with_capacity(0, 0);
-        let mut expected_builder = ListBuilder::new(elem_builder);
-        expected_builder.append(false);
-        expected_builder.values().append_value("7");
-        expected_builder.append(true);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        let expected = expected_builder.finish();
-        let result = actual.as_any().downcast_ref::<ListArray>().unwrap();
-        assert_eq!(&expected, result);
+                let expected = expected_builder.finish();
+                let result = 
actual.as_any().downcast_ref::<ListArray>().unwrap();
+                assert_eq!(&expected, result);
+            }
+        };
     }
 
-    #[test]
-    fn match_scalar_pattern() {
-        let values = vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), 
None];
-        let array = StringArray::from(values);
-        let pattern = Scalar::new(StringArray::from(vec![r"x.*-(\d*)-.*"; 1]));
-        let flags = Scalar::new(StringArray::from(vec!["i"; 1]));
-        let actual = regexp_match(&array, &pattern, Some(&flags)).unwrap();
-        let elem_builder: GenericStringBuilder<i32> = 
GenericStringBuilder::with_capacity(0, 0);
-        let mut expected_builder = ListBuilder::new(elem_builder);
-        expected_builder.append(false);
-        expected_builder.values().append_value("7");
-        expected_builder.append(true);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        let expected = expected_builder.finish();
-        let result = actual.as_any().downcast_ref::<ListArray>().unwrap();
-        assert_eq!(&expected, result);
-
-        // No flag
-        let values = vec![Some("abc-005-def"), Some("x-7-5"), Some("X545"), 
None];
-        let array = StringArray::from(values);
-        let actual = regexp_match(&array, &pattern, None).unwrap();
-        let result = actual.as_any().downcast_ref::<ListArray>().unwrap();
-        assert_eq!(&expected, result);
+    test_match_single_group_with_flags!(
+        match_single_group_with_flags_string,
+        vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), None],
+        vec![r"x.*-(\d*)-.*"; 4],
+        vec!["i"; 4],
+        StringArray,
+        GenericStringBuilder<i32>,
+        [None, Some("7"), None, None]
+    );
+    test_match_single_group_with_flags!(
+        match_single_group_with_flags_stringview,
+        vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), None],
+        vec![r"x.*-(\d*)-.*"; 4],
+        vec!["i"; 4],
+        StringViewArray,
+        StringViewBuilder,
+        [None, Some("7"), None, None]
+    );
+
+    macro_rules! test_match_scalar_pattern {
+        ($test_name:ident, $values:expr, $pattern:expr, $flag:expr, 
$array_type:ty, $builder_type:ty, $expected:expr) => {
+            #[test]
+            fn $test_name() {
+                let array: $array_type = <$array_type>::from($values);
+
+                let pattern_scalar = 
Scalar::new(<$array_type>::from(vec![$pattern; 1]));
+                let flag_scalar = Scalar::new(<$array_type>::from(vec![$flag; 
1]));
+
+                let actual = regexp_match(&array, &pattern_scalar, 
Some(&flag_scalar)).unwrap();
+
+                let elem_builder: $builder_type = <$builder_type>::new();
+                let mut expected_builder = ListBuilder::new(elem_builder);
+
+                for val in $expected {
+                    match val {
+                        Some(v) => {
+                            expected_builder.values().append_value(v);
+                            expected_builder.append(true);
+                        }
+                        None => expected_builder.append(false),
+                    }
+                }
+
+                let expected = expected_builder.finish();
+                let result = 
actual.as_any().downcast_ref::<ListArray>().unwrap();
+                assert_eq!(&expected, result);
+            }
+        };
     }
 
-    #[test]
-    fn match_scalar_no_pattern() {
-        let values = vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), 
None];
-        let array = StringArray::from(values);
-        let pattern = Scalar::new(new_null_array(&DataType::Utf8, 1));
-        let actual = regexp_match(&array, &pattern, None).unwrap();
-        let elem_builder: GenericStringBuilder<i32> = 
GenericStringBuilder::with_capacity(0, 0);
-        let mut expected_builder = ListBuilder::new(elem_builder);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        expected_builder.append(false);
-        let expected = expected_builder.finish();
-        let result = actual.as_any().downcast_ref::<ListArray>().unwrap();
-        assert_eq!(&expected, result);
+    test_match_scalar_pattern!(
+        match_scalar_pattern_string_with_flags,
+        vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), None],
+        r"x.*-(\d*)-.*",
+        Some("i"),
+        StringArray,
+        GenericStringBuilder<i32>,
+        [None, Some("7"), None, None]
+    );
+    test_match_scalar_pattern!(
+        match_scalar_pattern_stringview_with_flags,
+        vec![Some("abc-005-def"), Some("X-7-5"), Some("X545"), None],
+        r"x.*-(\d*)-.*",
+        Some("i"),
+        StringViewArray,
+        StringViewBuilder,
+        [None, Some("7"), None, None]
+    );
+
+    test_match_scalar_pattern!(
+        match_scalar_pattern_string_no_flags,
+        vec![Some("abc-005-def"), Some("x-7-5"), Some("X545"), None],
+        r"x.*-(\d*)-.*",
+        None::<&str>,
+        StringArray,
+        GenericStringBuilder<i32>,
+        [None, Some("7"), None, None]
+    );

Review Comment:
   There is no difference in the match results with, or without, flags. Might 
be nice to have test cases demonstrating this difference. 🙏🏼 



-- 
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]

Reply via email to