pitrou commented on code in PR #12391:
URL: https://github.com/apache/arrow/pull/12391#discussion_r936772777


##########
cpp/src/gandiva/precompiled/string_ops.cc:
##########
@@ -3034,4 +3034,45 @@ int32_t instr_utf8(const char* string, int32_t 
string_len, const char* substring
   }
   return 0;
 }
+
+FORCE_INLINE
+int32_t find_in_set_utf8_utf8(int64_t context, const char* to_find, int32_t 
to_find_len,
+                              const char* string_list, int32_t 
string_list_len) {
+  // Return 0 if to search entry have commas
+  if (is_substr_utf8_utf8(to_find, to_find_len, reinterpret_cast<const 
char*>(","), 1)) {
+    return 0;
+  }
+
+  int32_t cur_pos_in_array = 0;
+  int32_t cur_length = 0;
+  bool matching = true;
+
+  for (int i = 0; i < string_list_len; i++) {
+    if (string_list[i] == ',') {
+      cur_pos_in_array++;
+      if (matching && cur_length == to_find_len) {
+        return cur_pos_in_array;
+      } else {
+        matching = true;
+        cur_length = 0;
+      }
+    } else {
+      if (cur_length + 1 <= string_list_len) {

Review Comment:
   Why this condition? In which situation is it false?



##########
cpp/src/gandiva/precompiled/string_ops.cc:
##########
@@ -3034,4 +3034,45 @@ int32_t instr_utf8(const char* string, int32_t 
string_len, const char* substring
   }
   return 0;
 }
+
+FORCE_INLINE
+int32_t find_in_set_utf8_utf8(int64_t context, const char* to_find, int32_t 
to_find_len,
+                              const char* string_list, int32_t 
string_list_len) {
+  // Return 0 if to search entry have commas
+  if (is_substr_utf8_utf8(to_find, to_find_len, reinterpret_cast<const 
char*>(","), 1)) {

Review Comment:
   Since you are looking for a single unicode codepoint below 128, you can 
probably do this faster using `memchr`.



##########
cpp/src/gandiva/precompiled/string_ops.cc:
##########
@@ -3034,4 +3034,45 @@ int32_t instr_utf8(const char* string, int32_t 
string_len, const char* substring
   }
   return 0;
 }
+
+FORCE_INLINE
+int32_t find_in_set_utf8_utf8(int64_t context, const char* to_find, int32_t 
to_find_len,
+                              const char* string_list, int32_t 
string_list_len) {
+  // Return 0 if to search entry have commas
+  if (is_substr_utf8_utf8(to_find, to_find_len, reinterpret_cast<const 
char*>(","), 1)) {
+    return 0;
+  }
+
+  int32_t cur_pos_in_array = 0;
+  int32_t cur_length = 0;
+  bool matching = true;
+
+  for (int i = 0; i < string_list_len; i++) {
+    if (string_list[i] == ',') {
+      cur_pos_in_array++;
+      if (matching && cur_length == to_find_len) {
+        return cur_pos_in_array;
+      } else {
+        matching = true;
+        cur_length = 0;
+      }
+    } else {
+      if (cur_length + 1 <= string_list_len) {
+        if (!matching || (memcmp(string_list + i, to_find + cur_length, 1))) {

Review Comment:
   Why call `memcmp` if you are only comparing a single byte?



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