pitrou commented on code in PR #50479:
URL: https://github.com/apache/arrow/pull/50479#discussion_r3912102846
##########
cpp/src/arrow/compute/kernels/scalar_string_ascii.cc:
##########
@@ -1645,6 +1646,7 @@ void AddAsciiStringMatchSubstring(FunctionRegistry*
registry) {
DCHECK_OK(
func->AddKernel({ty}, boolean(), std::move(exec),
MatchSubstringState::Init));
}
+ AddMatchSubstringViewKernels<MatchSubstring,
PlainSubstringMatcher>(func.get());
Review Comment:
Can we perhaps add a `GenerateTypeAgnosticVarBinaryViewBase` so that we can
write:
```c++
for (const auto& ty : BinaryViewTypes()) {
auto exec =
GenerateTypeAgnosticVarBinaryViewBase<MatchSubstring,
PlainSubstringMatcher>(ty);
DCHECK_OK(
func->AddKernel({ty}, boolean(), std::move(exec),
MatchSubstringState::Init));
}
```
##########
cpp/src/arrow/compute/kernels/scalar_string_test.cc:
##########
@@ -2837,4 +2841,146 @@ TEST(TestStringKernels, UnicodeLibraryAssumptions) {
}
#endif
+// ----------------------------------------------------------------------
+// View type support for scalar string predicate/measurement kernels.
+
+// Mixes empty, inlined (<= 12 bytes), out-of-line (> 12 bytes), and null
values
+// to cover the view layout.
+constexpr auto kViewInput =
+ R"(["", "cat", null, "the quick brown fox", "concatenation", "a cat sat",
"CAT"])";
+
+TEST(TestStringViewPredicates, MatchSubstring) {
+ MatchSubstringOptions options{"cat"};
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("match_substring", ty, kViewInput, boolean(),
+ "[false, true, null, false, true, true, false]",
&options);
+ }
+}
+
+TEST(TestStringViewPredicates, StartsWithEndsWith) {
+ MatchSubstringOptions starts{"c"};
+ MatchSubstringOptions ends{"t"};
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("starts_with", ty, kViewInput, boolean(),
+ "[false, true, null, false, true, false, false]",
&starts);
+ CheckScalarUnary("ends_with", ty, kViewInput, boolean(),
+ "[false, true, null, false, false, true, false]", &ends);
+ }
+}
+
+TEST(TestStringViewPredicates, FindSubstring) {
+ MatchSubstringOptions options{"cat"};
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("find_substring", ty, kViewInput, int32(),
+ "[-1, 0, null, -1, 3, 2, -1]", &options);
+ }
+}
+
+TEST(TestStringViewPredicates, CountSubstring) {
+ MatchSubstringOptions options{"cat"};
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("count_substring", ty, kViewInput, int32(),
+ "[0, 1, null, 0, 1, 1, 0]", &options);
+ }
+}
+
+TEST(TestStringViewPredicates, BinaryLength) {
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("binary_length", ty, kViewInput, int32(),
+ "[0, 3, null, 19, 13, 9, 3]");
+ }
+}
+
+#ifdef ARROW_WITH_RE2
+TEST(TestStringViewPredicates, MatchSubstringRegex) {
+ MatchSubstringOptions options{"a+"};
+ const auto* input = R"(["", "cat", null, "aaa banana", "concatenation"])";
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("match_substring_regex", ty, input, boolean(),
+ "[false, true, null, true, true]", &options);
+ CheckScalarUnary("find_substring_regex", ty, input, int32(), "[-1, 1,
null, 0, 4]",
+ &options);
+ CheckScalarUnary("count_substring_regex", ty, input, int32(), "[0, 1,
null, 4, 2]",
+ &options);
+ }
+}
+
+TEST(TestStringViewPredicates, MatchLike) {
+ MatchSubstringOptions contains{"%cat%"};
+ MatchSubstringOptions prefix{"c%"};
+ const auto* input = R"(["cat", "concatenation", null, "dog", "a cat sat"])";
+ for (const auto& ty : {binary_view(), utf8_view()}) {
+ CheckScalarUnary("match_like", ty, input, boolean(),
+ "[true, true, null, false, true]", &contains);
+ CheckScalarUnary("match_like", ty, input, boolean(),
+ "[true, true, null, false, false]", &prefix);
+ }
+}
+
+// utf8_view registers as StringViewType, so ignore_case folds the full Unicode
+// range (É matches é), not just ASCII. This would fail if utf8_view were
+// dispatched through the generic BinaryViewType path and lost that
distinction.
+TEST(TestStringViewPredicates, MatchSubstringIgnoreCase) {
+ MatchSubstringOptions options{"aé(", /*ignore_case=*/true};
+ CheckScalarUnary("match_substring", utf8_view(),
Review Comment:
Can we add the same test for BinaryView to check and showcase the different
results?
--
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]
