[feature](functions) impl scalar functions trim_in、ltrim_in and rtrim_in #41681
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
liujiwen-up
changed the title
Contributor
Author
|
run buildall |
|
TeamCity be ut coverage result: |
yiguolei
reviewed
Oct 11, 2024
| order_qt_52 "SELECT rtrim_in(' hello ', 'lo ');" | ||
| order_qt_53 "SELECT rtrim_in('hello ', ' ');" | ||
| order_qt_54 "SELECT rtrim_in('hello ', 'l o');" | ||
| order_qt_55 "SELECT rtrim_in('hello ', 'l');" |
Contributor
There was a problem hiding this comment.
add more test
- create a table and then insert the values to a table, and use select func(col) from table
- the column in the table, could be a nullable column and not null column.
yiguolei
reviewed
Oct 11, 2024
| }; | ||
| template <bool is_ltrim_in, bool is_rtrim_in, bool trim_single> | ||
| struct TrimInUtil { | ||
| static Status vector(const ColumnString::Chars& str_data, |
Contributor
There was a problem hiding this comment.
为什么不直接使用column string，而是把chars和offsets 分开
Contributor
Author
There was a problem hiding this comment.
In order to maintain the same processing method as the TrimUtil method, so no changes are made here.
struct TrimUtil { static Status vector(const ColumnString::Chars& str_data, const ColumnString::Offsets& str_offsets, const StringRef& remove_str, ColumnString::Chars& res_data, ColumnString::Offsets& res_offsets)
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
| const size_t offset_size = str_offsets.size(); | ||
| res_offsets.resize(offset_size); | ||
| res_data.reserve(str_data.size()); | ||
| std::bitset<256> char_lookup; |
Contributor
There was a problem hiding this comment.
Add comment to explain magic number 256
Contributor
Author
There was a problem hiding this comment.
Currently, there are two processing logics.
- The
simd::VStringFunctions::is_asciimethod is used to judge that when the string is all ascii, bitset<128> will be used for processing. The character range of the standard ASCII table is 0 to 127. Bitset<128> has exactly 128 bits, which is enough to represent all standard ASCII characters. - When the string is not all standard ascii, the utf-8 logic will be used for processing.It is especially important to note that when trimming on the right, according to the rules of UTF-8, the format of the UTF-8 trailing byte is 10xxxxxx. Use
(*prev_char_pos & 0xC0) == 0x80to determine whether the current byte is a trailing byte. When a byte that is not a trailing byte is found, this byte is the starting byte of the current character.
| res_data.reserve(str_data.size()); | ||
| std::bitset<256> char_lookup; | ||
| for (size_t i = 0; i < remove_str.size; ++i) { | ||
| char_lookup[static_cast<unsigned char>(remove_str.data[i])] = true; |
Contributor
Author
There was a problem hiding this comment.
Added utf-8 processing logic
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
zhiqiang-hhhh
approved these changes
Oct 15, 2024
|
|
||
| for (size_t i = 0; i < offset_size; ++i) { | ||
| const char* str_begin = reinterpret_cast<const char*>( | ||
| str_data.data() + (i == 0 ? 0 : str_offsets[i - 1])); |
Contributor
There was a problem hiding this comment.
str_data.data() + str_offsets[i] is better, since PODArray guarantees access to -1 is safe.
| res_offsets.resize(offset_size); | ||
| res_data.reserve(str_data.size()); | ||
|
|
||
| std::unordered_set<std::string> char_lookup; |
Contributor
There was a problem hiding this comment.
potential large momory comsuption
Contributor
Author
There was a problem hiding this comment.
Change to string_view
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 41467 ms |
TPC-DS: Total hot run time: 191690 ms |
ClickBench: Total hot run time: 33.23 s |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
Contributor
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 41520 ms |
TPC-DS: Total hot run time: 192329 ms |
ClickBench: Total hot run time: 32.76 s |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 41398 ms |
TPC-DS: Total hot run time: 192384 ms |
ClickBench: Total hot run time: 32.53 s |
Contributor
Author
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 41202 ms |
TPC-DS: Total hot run time: 191866 ms |
ClickBench: Total hot run time: 33.81 s |
6 tasks
Contributor
Author
Contributor
Author
|
run external |
liujiwen-up
added a commit
to liujiwen-up/doris
that referenced
this pull request
Oct 28, 2024
…_in (apache#41681) trim_in is different from trim Find and remove any characters in a set of characters at both ends of a string (regardless of order) mysql> SELECT TRIM('abcd', 'cde'); +---------------------+ | trim('abcd', 'cde') | +---------------------+ | abcd | +---------------------+ 1 row in set (0.02 sec) mysql> SELECT TRIM_IN('abcd', 'cde'); +------------------------+ | trim_in('abcd', 'cde') | +------------------------+ | ab | +------------------------+ 1 row in set (0.02 sec)
liujiwen-up
added a commit
to liujiwen-up/doris
that referenced
this pull request
Oct 28, 2024
…_in (apache#41681) trim_in is different from trim Find and remove any characters in a set of characters at both ends of a string (regardless of order) mysql> SELECT TRIM('abcd', 'cde'); +---------------------+ | trim('abcd', 'cde') | +---------------------+ | abcd | +---------------------+ 1 row in set (0.02 sec) mysql> SELECT TRIM_IN('abcd', 'cde'); +------------------------+ | trim_in('abcd', 'cde') | +------------------------+ | ab | +------------------------+ 1 row in set (0.02 sec)
yiguolei
pushed a commit
that referenced
this pull request
Nov 1, 2024
KassieZ
added a commit
to apache/doris-website
that referenced
this pull request
Nov 28, 2024
apache/doris#41681 # Versions - [x] dev - [x] 3.0 - [x] 2.1 - [ ] 2.0 # Languages - [x] Chinese - [x] English --------- Co-authored-by: KassieZ <139741991+KassieZ@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
trim_in is different from trim
Find and remove any characters in a set of characters at both ends of a string (regardless of order)
mysql> SELECT TRIM('abcd', 'cde');
+---------------------+
| trim('abcd', 'cde') |
+---------------------+
| abcd |
+---------------------+
1 row in set (0.02 sec)
mysql> SELECT TRIM_IN('abcd', 'cde');
+------------------------+
| trim_in('abcd', 'cde') |
+------------------------+
| ab |
+------------------------+
1 row in set (0.02 sec)