[GitHub] [arrow] js8544 commented on a diff in pull request #33806: GH-33723: [C++] re2::RE2::RE2() result must be checked

[GitBox]

js8544 commented on code in PR #33806:
URL: https://github.com/apache/arrow/pull/33806#discussion_r1082609589


##########
cpp/src/arrow/compute/kernels/scalar_string_ascii.cc:
##########
@@ -1505,6 +1508,13 @@ struct MatchLike {
     static const RE2 kLikePatternIsStartsWith(R"(([^%_]*[^\\%_])?%+)", 
kRE2Options);
     // A LIKE pattern matching this regex can be translated into a suffix 
search.
     static const RE2 kLikePatternIsEndsWith(R"(%+([^%_]*))", kRE2Options);
+    static bool global_checked = false;
+    if (ARROW_PREDICT_FALSE(!global_checked)) {
+      RETURN_NOT_OK(RegexStatus(kLikePatternIsSubstringMatch));
+      RETURN_NOT_OK(RegexStatus(kLikePatternIsStartsWith));
+      RETURN_NOT_OK(RegexStatus(kLikePatternIsEndsWith));
+      global_checked = true;
+    }

Review Comment:
   Are there better ways to check global RE2 objects?



##########
cpp/src/gandiva/regex_functions_holder.cc:
##########
@@ -34,27 +31,44 @@ std::string& RemovePatternEscapeChars(const FunctionNode& 
node, std::string& pat
 // Short-circuit pattern matches for the following common sub cases :
 // - starts_with, ends_with and is_substr
 const FunctionNode LikeHolder::TryOptimize(const FunctionNode& node) {
+  // NOTE: avoid making those constants global to avoid compiling regexes at 
startup
+  // pre-compiled pattern for matching starts_with
+  static const RE2 starts_with_regex(R"(([^\.\*])*\.\*)");
+  // pre-compiled pattern for matching ends_with
+  static const RE2 ends_with_regex(R"(\.\*([^\.\*])*)");
+  // pre-compiled pattern for matching is_substr
+  static const RE2 is_substr_regex(R"(\.\*([^\.\*])*\.\*)");
+
+  static bool global_checked = false;
+  if (ARROW_PREDICT_FALSE(!global_checked)) {
+    if (ARROW_PREDICT_FALSE(
+            !(starts_with_regex.ok() && ends_with_regex.ok() && 
is_substr_regex.ok()))) {
+      return node;
+    }
+    global_checked = true;
+  }

Review Comment:
   Are there better ways to check global RE2 objects?



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