pravindra commented on a change in pull request #11471:
URL: https://github.com/apache/arrow/pull/11471#discussion_r757260350



##########
File path: cpp/src/gandiva/like_holder.cc
##########
@@ -22,10 +22,20 @@
 #include "gandiva/regex_util.h"
 
 namespace gandiva {
+RE2 LikeHolder::starts_with_regex_(R"((\w|\s|\\-)*\.\*)");

Review comment:
       maybe, we should make this more generic. Allow any characters other than 
. and * ?
   
   `RE2 LikeHolder::starts_with_regex_(R"([^\.\*]*\.\*)");
   `
   same for ends_with & is_substr ?

##########
File path: cpp/src/gandiva/like_holder.cc
##########
@@ -22,10 +22,20 @@
 #include "gandiva/regex_util.h"
 
 namespace gandiva {
+RE2 LikeHolder::starts_with_regex_(R"((\w|\s|\\-)*\.\*)");
+RE2 LikeHolder::ends_with_regex_(R"(\.\*(\w|\s|\\-)*)");
+RE2 LikeHolder::is_substr_regex_(R"(\.\*(\w|\s|\\-)*\.\*)");
 
-RE2 LikeHolder::starts_with_regex_(R"((\w|\s)*\.\*)");
-RE2 LikeHolder::ends_with_regex_(R"(\.\*(\w|\s)*)");
-RE2 LikeHolder::is_substr_regex_(R"(\.\*(\w|\s)*\.\*)");
+std::string& RemovePatternEscapeChars(const FunctionNode& node, std::string& 
pattern) {
+  if (node.children().size() != 2) {

Review comment:
       why is this required ? the pattern that is present here is the pcre 
pattern and any escapes should have already been removed as part of 
SqlLikePatternToPcre().
   
   can you add a test which would fail without this change 
(RemovePatternEscapeChars part) and passes with the change ?




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