[GitHub] [arrow] westonpace commented on a change in pull request #11233: ARROW-13879: [C++] Mixed support for binary types in regex functions

[GitBox]

westonpace commented on a change in pull request #11233:
URL: https://github.com/apache/arrow/pull/11233#discussion_r734096663



##########
File path: cpp/src/arrow/compute/kernels/scalar_string.cc
##########
@@ -1303,21 +1336,22 @@ struct FindSubstring {
 struct FindSubstringRegex {
   std::unique_ptr<RE2> regex_match_;
 
-  explicit FindSubstringRegex(const MatchSubstringOptions& options,
+  explicit FindSubstringRegex(const MatchSubstringOptions& options, bool 
is_utf8 = true,
                               bool literal = false) {
     std::string regex = "(";
     regex.reserve(options.pattern.length() + 2);
     regex += literal ? RE2::QuoteMeta(options.pattern) : options.pattern;
     regex += ")";
-    regex_match_.reset(new RE2(std::move(regex), 
RegexSubstringMatcher::MakeRE2Options(
-                                                     options, 
/*literal=*/false)));
+    regex_match_.reset(
+        new RE2(std::move(regex),

Review comment:
       ```suggestion
           new RE2(regex,
   ```

##########
File path: cpp/src/arrow/compute/kernels/scalar_string.cc
##########
@@ -1235,52 +1267,53 @@ void AddMatchSubstring(FunctionRegistry* registry) {
   {
     auto func = std::make_shared<ScalarFunction>("match_substring", 
Arity::Unary(),
                                                  &match_substring_doc);
-    auto exec_32 = MatchSubstring<StringType, PlainSubstringMatcher>::Exec;
-    auto exec_64 = MatchSubstring<LargeStringType, 
PlainSubstringMatcher>::Exec;
-    DCHECK_OK(func->AddKernel({utf8()}, boolean(), exec_32, 
MatchSubstringState::Init));
-    DCHECK_OK(
-        func->AddKernel({large_utf8()}, boolean(), exec_64, 
MatchSubstringState::Init));
+    for (const auto& ty : BaseBinaryTypes()) {
+      auto exec = GenerateVarBinaryToVarBinary<MatchSubstring, 
PlainSubstringMatcher>(ty);
+      DCHECK_OK(
+          func->AddKernel({ty}, boolean(), std::move(exec), 
MatchSubstringState::Init));
+    }
     DCHECK_OK(registry->AddFunction(std::move(func)));
   }
   {
     auto func = std::make_shared<ScalarFunction>("starts_with", Arity::Unary(),
                                                  &match_substring_doc);

Review comment:
       ```suggestion
                                                    &starts_with_doc);
   ```

##########
File path: cpp/src/arrow/compute/kernels/scalar_string.cc
##########
@@ -1235,52 +1267,53 @@ void AddMatchSubstring(FunctionRegistry* registry) {
   {
     auto func = std::make_shared<ScalarFunction>("match_substring", 
Arity::Unary(),
                                                  &match_substring_doc);
-    auto exec_32 = MatchSubstring<StringType, PlainSubstringMatcher>::Exec;
-    auto exec_64 = MatchSubstring<LargeStringType, 
PlainSubstringMatcher>::Exec;
-    DCHECK_OK(func->AddKernel({utf8()}, boolean(), exec_32, 
MatchSubstringState::Init));
-    DCHECK_OK(
-        func->AddKernel({large_utf8()}, boolean(), exec_64, 
MatchSubstringState::Init));
+    for (const auto& ty : BaseBinaryTypes()) {
+      auto exec = GenerateVarBinaryToVarBinary<MatchSubstring, 
PlainSubstringMatcher>(ty);
+      DCHECK_OK(
+          func->AddKernel({ty}, boolean(), std::move(exec), 
MatchSubstringState::Init));
+    }
     DCHECK_OK(registry->AddFunction(std::move(func)));
   }
   {
     auto func = std::make_shared<ScalarFunction>("starts_with", Arity::Unary(),
                                                  &match_substring_doc);
-    auto exec_32 = MatchSubstring<StringType, PlainStartsWithMatcher>::Exec;
-    auto exec_64 = MatchSubstring<LargeStringType, 
PlainStartsWithMatcher>::Exec;
-    DCHECK_OK(func->AddKernel({utf8()}, boolean(), exec_32, 
MatchSubstringState::Init));
-    DCHECK_OK(
-        func->AddKernel({large_utf8()}, boolean(), exec_64, 
MatchSubstringState::Init));
+    for (const auto& ty : BaseBinaryTypes()) {
+      auto exec =
+          GenerateVarBinaryToVarBinary<MatchSubstring, 
PlainStartsWithMatcher>(ty);
+      DCHECK_OK(
+          func->AddKernel({ty}, boolean(), std::move(exec), 
MatchSubstringState::Init));
+    }
     DCHECK_OK(registry->AddFunction(std::move(func)));
   }
   {
     auto func = std::make_shared<ScalarFunction>("ends_with", Arity::Unary(),
                                                  &match_substring_doc);

Review comment:
       ```suggestion
                                                    &ends_with_doc);
   ```

##########
File path: cpp/src/arrow/compute/kernels/scalar_string.cc
##########
@@ -4173,48 +4238,45 @@ void MakeUnaryStringUTF8TransformKernel(std::string 
name, FunctionRegistry* regi
 using StringPredicate =
     std::function<bool(KernelContext*, const uint8_t*, size_t, Status*)>;
 

Review comment:
       ```suggestion
   ```

##########
File path: cpp/src/arrow/compute/kernels/scalar_string_test.cc
##########
@@ -50,6 +51,19 @@ class BaseTestStringKernels : public ::testing::Test {
     CheckScalarUnary(func_name, type(), json_input, out_ty, json_expected, 
options);
   }
 
+  void CheckUnary(std::string func_name, const std::shared_ptr<Array>& input,
+                  std::shared_ptr<DataType> out_ty, std::string json_expected,
+                  const FunctionOptions* options = nullptr) {
+    CheckScalar(func_name, {Datum(input)}, Datum(ArrayFromJSON(out_ty, 
json_expected)),
+                options);
+  }
+
+  void CheckUnary(std::string func_name, const std::shared_ptr<Array>& input,
+                  const std::shared_ptr<Array> expected,

Review comment:
       ```suggestion
                     const std::shared_ptr<Array>& expected,
   ```




-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org