You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/11/03 20:32:00 UTC

[GitHub] [arrow] kou commented on a diff in pull request #14579: ARROW-18235: [C++][Gandiva] Fix the like function implementation for escape chars

kou commented on code in PR #14579:
URL: https://github.com/apache/arrow/pull/14579#discussion_r1013369863


##########
cpp/src/gandiva/regex_util.cc:
##########
@@ -44,6 +45,10 @@ Status RegexUtil::SqlLikePatternToPcre(const std::string& sql_pattern, char esca
 
       cur = sql_pattern.at(idx);
       if (cur == '_' || cur == '%' || cur == escape_char) {
+        if (cur == '\\') {

Review Comment:
   Is this condition correct? It seems that we need to check whether the `cur` (== `escape_char`) is a special character or not:
   
   ```suggestion
           if (pcre_regex_specials_.find(cur) != pcre_regex_specials_.end()) {
   ```
   
   BTW, can we extract this condition to out of this `for` loop if my suggestion is correct?



##########
cpp/src/gandiva/tests/filter_test.cc:
##########
@@ -414,6 +414,39 @@ TEST_F(TestFilter, TestLike) {
 
   // Validate results
   EXPECT_ARROW_ARRAY_EQUALS(exp, selection_vector->ToArray());
+
+  auto literal_escape_pattern =
+      TreeExprBuilder::MakeStringLiteral("%tu^_security^_freeze%");
+  auto escape_char = TreeExprBuilder::MakeStringLiteral("^");

Review Comment:
   Can we use a PCRE special character that is invalid without suitable escap such as `[` instead?



-- 
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-unsubscribe@arrow.apache.org

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