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 2020/07/06 01:56:39 UTC
[GitHub] [arrow] sagnikc-dremio opened a new pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
sagnikc-dremio opened a new pull request #7641:
URL: https://github.com/apache/arrow/pull/7641
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457847628
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // base text has some characters which are not present in the trim text
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 end_cnt, char_len, end_ptr = 0;
+ // scan the base text from left to right and increment the end pointer to the current
+ // position when there is a character which is not present in the trim text
+ for (end_cnt = 0; end_cnt < basetext_len; end_cnt += char_len) {
Review comment:
You can go back byte by byte and check in the trim set if the current byte is the first byte in utf8 char.
```
while (end >= start) {
int char_len = utf8_char_len(end);
if (char_len == 0) {end—; break;} // trailing byte in mutlitbyte char
if (char, char_len) present in (trim, trim_len) break;
end—;```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r458053012
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,104 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 char_len, end_ptr;
+ // scan the base text from right to left and decrement the end pointer till
+ // there is a character which is not present in the trim text
+ for (end_ptr = basetext_len - 1; end_ptr >= 0; --end_ptr) {
+ char_len = utf8_char_length(basetext[end_ptr]);
+ if (char_len == 0) { // trailing bytes of multibyte character
+ continue;
+ }
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + end_ptr, char_len)) {
+ break;
+ }
}
- // string with all spaces
- if (start > end) {
+ end_ptr += utf8_char_length(basetext[end_ptr]); // point to the next character
Review comment:
here end_ptr can be -1 and will be buffer overflow. You need to separately handle that case.
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,104 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
Review comment:
You need to handle the cases when the input is a malformed utf8 string. Currently the code might give buffer overflow if last character is incomplete multibyte character or the ltrim() might go into an infinite loop if there is an invalid byte in the middle.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#issuecomment-655228953
@pprudhvi @projjal Can you please review this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457847628
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // base text has some characters which are not present in the trim text
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 end_cnt, char_len, end_ptr = 0;
+ // scan the base text from left to right and increment the end pointer to the current
+ // position when there is a character which is not present in the trim text
+ for (end_cnt = 0; end_cnt < basetext_len; end_cnt += char_len) {
Review comment:
You can go back byte by byte and check in the trim set if the current byte is the first byte in utf8 char.
```
while (end >= start) {
int char_len = utf8_char_len(end);
if (char_len == 0) {end--; continue;} // trailing byte in mutlitbyte char
if basetext(end, char_len) present in trim(0, trim_len) break;
end--;```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457316490
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // base text has some characters which are not present in the trim text
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 end_cnt, char_len, end_ptr = 0;
+ // scan the base text from left to right and increment the end pointer to the current
+ // position when there is a character which is not present in the trim text
+ for (end_cnt = 0; end_cnt < basetext_len; end_cnt += char_len) {
Review comment:
Why are you moving from left to right. You will have to pass and decode the entire string even when there is no trim text or only a single trim character in the basetext. Better move from right to left and break when not found.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r459302381
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,161 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (char_len == 0 || start_ptr + char_len > basetext_len) {
+ // invalid byte or incomplete glyph
+ set_error_for_invalid_utf(context, basetext[start_ptr]);
+ *out_len = 0;
+ return "";
+ }
+ for (gdv_int32 char_ptr = 1; char_ptr < char_len; ++char_ptr) {
+ if ((basetext[start_ptr + char_ptr] & 0xC0) != 0x80) {
+ // bytes following head-byte of glyph
+ set_error_for_invalid_utf(context, basetext[start_ptr + char_ptr]);
+ *out_len = 0;
+ return "";
+ }
+ }
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
}
- // string with all spaces
- if (start > end) {
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
*out_len = 0;
return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 char_len, end_ptr, char_cnt = 1;
+ // scan the base text from right to left and decrement the end pointer till
+ // there is a character which is not present in the trim text
+ for (end_ptr = basetext_len - 1; end_ptr >= 0; --end_ptr) {
+ char_len = utf8_char_length(basetext[end_ptr]);
+ if (char_len == 0) { // trailing bytes of multibyte character
+ ++char_cnt;
Review comment:
byte_cnt would be a better name
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,161 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (char_len == 0 || start_ptr + char_len > basetext_len) {
+ // invalid byte or incomplete glyph
+ set_error_for_invalid_utf(context, basetext[start_ptr]);
+ *out_len = 0;
+ return "";
+ }
+ for (gdv_int32 char_ptr = 1; char_ptr < char_len; ++char_ptr) {
Review comment:
I am not sure if this check is really needed. Verifying all the bytes seems extra overhead unless this function really means to validate the entire utf8 string. The check at L388 is required because otherwise it may crash.
If you do think it is required we should also do this in rtrim to have consistent behavior in both.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r459372973
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,161 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (char_len == 0 || start_ptr + char_len > basetext_len) {
+ // invalid byte or incomplete glyph
+ set_error_for_invalid_utf(context, basetext[start_ptr]);
+ *out_len = 0;
+ return "";
+ }
+ for (gdv_int32 char_ptr = 1; char_ptr < char_len; ++char_ptr) {
+ if ((basetext[start_ptr + char_ptr] & 0xC0) != 0x80) {
+ // bytes following head-byte of glyph
+ set_error_for_invalid_utf(context, basetext[start_ptr + char_ptr]);
+ *out_len = 0;
+ return "";
+ }
+ }
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
}
- // string with all spaces
- if (start > end) {
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
*out_len = 0;
return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 char_len, end_ptr, char_cnt = 1;
+ // scan the base text from right to left and decrement the end pointer till
+ // there is a character which is not present in the trim text
+ for (end_ptr = basetext_len - 1; end_ptr >= 0; --end_ptr) {
+ char_len = utf8_char_length(basetext[end_ptr]);
+ if (char_len == 0) { // trailing bytes of multibyte character
+ ++char_cnt;
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r452613034
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -320,6 +385,143 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
Review comment:
The utf8 handling seems incorrect. You need to decode the utf8 char and match against the target string instead of matching individual bytes. In this case a byte of multibyte char in trim_text might match against a byte of another multibyte char in target string
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#issuecomment-662958111
Thanks @sagnikc-dremio
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457838825
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -16,6 +16,7 @@
// under the License.
// String functions
+#include <unordered_set>
Review comment:
Removed it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r459373149
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,161 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (char_len == 0 || start_ptr + char_len > basetext_len) {
+ // invalid byte or incomplete glyph
+ set_error_for_invalid_utf(context, basetext[start_ptr]);
+ *out_len = 0;
+ return "";
+ }
+ for (gdv_int32 char_ptr = 1; char_ptr < char_len; ++char_ptr) {
Review comment:
Removed the check.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] praveenbingo commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
praveenbingo commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r451391005
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -284,10 +285,74 @@ const char* reverse_utf8(gdv_int64 context, const char* data, gdv_int32 data_len
return ret;
}
-// Trim a utf8 sequence
+// Trims whitespaces from the left end of the input utf8 sequence
FORCE_INLINE
-const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
- int32_t* out_len) {
+const char* ltrim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
+ int32_t* out_len) {
+ if (data_len == 0) {
+ *out_len = 0;
+ return "";
+ }
+
+ gdv_int32 start = 0;
+ // start denotes the first position of non-space characters in the input string
+ while (start < data_len && data[start] == ' ') {
+ ++start;
+ }
+
+ // string with no leading spaces, return original string
+ if (start == 0) {
+ *out_len = data_len;
+ return data;
+ }
+
+ // string with all spaces
+ if (start == data_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // string has some leading spaces and some non-space characters
+ *out_len = data_len - start;
+ return data + start;
+}
+
+// Trims whitespaces from the right end of the input utf8 sequence
+FORCE_INLINE
+const char* rtrim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
+ int32_t* out_len) {
+ if (data_len == 0) {
+ *out_len = 0;
+ return "";
+ }
+
+ gdv_int32 end = data_len - 1;
+ // end denotes the last position of non-space characters in the input string
+ while (end >= 0 && data[end] == ' ') {
Review comment:
is this the only utf8 space character?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#issuecomment-653977929
https://issues.apache.org/jira/browse/ARROW-9328
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457847628
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // base text has some characters which are not present in the trim text
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 end_cnt, char_len, end_ptr = 0;
+ // scan the base text from left to right and increment the end pointer to the current
+ // position when there is a character which is not present in the trim text
+ for (end_cnt = 0; end_cnt < basetext_len; end_cnt += char_len) {
Review comment:
You can go back byte by byte and check in the trim set if the current byte is the first byte in utf8 char.
```
while (end >= start) {
int char_len = utf8_char_len(end);
if (char_len == 0) {end--; break;} // trailing byte in mutlitbyte char
if basetext(end, char_len) present in trim(0, trim_len) break;
end--;```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457840242
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
Review comment:
Yes, thanks for pointing out. Removed the redundant checks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457843985
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // base text has some characters which are not present in the trim text
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 end_cnt, char_len, end_ptr = 0;
+ // scan the base text from left to right and increment the end pointer to the current
+ // position when there is a character which is not present in the trim text
+ for (end_cnt = 0; end_cnt < basetext_len; end_cnt += char_len) {
Review comment:
I did consider that option previously and it worked well and good for single-byte character strings, but I was facing some issues with multibyte character strings.
So, we need to decode the utf8 char and compute its length before we can move to the subsequent character. So if we are traversing from right to left, we need to find the byte position of the last character and check if that character belongs to the trimtext character set, then find the byte position of the second-to-last character and so on. It can be done, but it looks a little complex to me, whereas while moving from left to right, we don't come across this.
Maybe there is a better way to do it. Please suggest if you have anything in mind.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sagnikc-dremio commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
sagnikc-dremio commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r459212543
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -305,21 +342,104 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
--end;
}
- // string with no leading/trailing spaces, return original string
- if (start == 0 && end == data_len - 1) {
- *out_len = data_len;
- return data;
+ // string has some leading/trailing spaces and some non-space characters
+ *out_len = end - start + 1;
+ return data + start;
+}
+
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
Review comment:
Added logic which will sniff out an invalid byte or an incomplete multibyte character and will throw an error, as was discussed offline. Also, added unit tests for the same.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r457311111
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -16,6 +16,7 @@
// under the License.
// String functions
+#include <unordered_set>
Review comment:
Not being used now
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // base text has some characters which are not present in the trim text
+ *out_len = basetext_len - start_ptr;
+ return basetext + start_ptr;
+}
+
+// Trims characters present in the trim text from the right end of the base text
+FORCE_INLINE
+const char* rtrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 end_cnt, char_len, end_ptr = 0;
+ // scan the base text from left to right and increment the end pointer to the current
+ // position when there is a character which is not present in the trim text
+ for (end_cnt = 0; end_cnt < basetext_len; end_cnt += char_len) {
Review comment:
Why are you moving from left to right. You will have to pass and decode the entire string even when there is no trim text or only a single character in the basetext. Better move from right to left and break when not found.
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -322,6 +387,138 @@ const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
return data + start;
}
+// Trims characters present in the trim text from the left end of the base text
+FORCE_INLINE
+const char* ltrim_utf8_utf8(gdv_int64 context, const char* basetext,
+ gdv_int32 basetext_len, const char* trimtext,
+ gdv_int32 trimtext_len, int32_t* out_len) {
+ if (basetext_len == 0) {
+ *out_len = 0;
+ return "";
+ } else if (trimtext_len == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ gdv_int32 start_ptr, char_len;
+ // scan the base text from left to right and increment the start pointer till
+ // there is a character which is not present in the trim text
+ for (start_ptr = 0; start_ptr < basetext_len; start_ptr += char_len) {
+ char_len = utf8_char_length(basetext[start_ptr]);
+ if (!is_substr_utf8_utf8(trimtext, trimtext_len, basetext + start_ptr, char_len)) {
+ break;
+ }
+ }
+
+ // the first character from the left is not present in the trim text,
+ // hence there is nothing to be trimmed, return original string
+ if (start_ptr == 0) {
+ *out_len = basetext_len;
+ return basetext;
+ }
+
+ // all the characters in the base text are present in the trim text,
+ // hence trim the entire string, return empty string
+ if (start_ptr == trimtext_len) {
Review comment:
start_ptr == _basetext_len_
Also, you don't need these checks to return empty string and original string, these add unnecessary branching. The final calculations in Line428 will take care of them.
Same for the other trim functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #7641:
URL: https://github.com/apache/arrow/pull/7641#discussion_r452615768
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -284,10 +285,74 @@ const char* reverse_utf8(gdv_int64 context, const char* data, gdv_int32 data_len
return ret;
}
-// Trim a utf8 sequence
+// Trims whitespaces from the left end of the input utf8 sequence
FORCE_INLINE
-const char* trim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
- int32_t* out_len) {
+const char* ltrim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
+ int32_t* out_len) {
+ if (data_len == 0) {
+ *out_len = 0;
+ return "";
+ }
+
+ gdv_int32 start = 0;
+ // start denotes the first position of non-space characters in the input string
+ while (start < data_len && data[start] == ' ') {
+ ++start;
+ }
+
+ // string with no leading spaces, return original string
+ if (start == 0) {
+ *out_len = data_len;
+ return data;
+ }
+
+ // string with all spaces
+ if (start == data_len) {
+ *out_len = 0;
+ return "";
+ }
+
+ // string has some leading spaces and some non-space characters
+ *out_len = data_len - start;
+ return data + start;
+}
+
+// Trims whitespaces from the right end of the input utf8 sequence
+FORCE_INLINE
+const char* rtrim_utf8(gdv_int64 context, const char* data, gdv_int32 data_len,
+ int32_t* out_len) {
+ if (data_len == 0) {
+ *out_len = 0;
+ return "";
+ }
+
+ gdv_int32 end = data_len - 1;
+ // end denotes the last position of non-space characters in the input string
+ while (end >= 0 && data[end] == ' ') {
Review comment:
there is only one unicode codepoint for each character
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] praveenbingo closed pull request #7641: ARROW-9328: [C++][Gandiva] Add LTRIM, RTRIM, BTRIM functions for string
Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #7641:
URL: https://github.com/apache/arrow/pull/7641
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org