github-actions[bot] commented on code in PR #66050:
URL: https://github.com/apache/doris/pull/66050#discussion_r3827220095
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -136,52 +137,93 @@ struct RegexpExtractEngine {
return false;
}
- // Match all occurrences and extract the first capturing group
- void match_all_and_extract(const char* data, size_t size,
- std::vector<std::string>& results) const {
+ // Match all occurrences and extract the capturing group with the given
index.
+ // index 0 means the whole match, 1 means the first capturing group (the
default), and so on.
+ // Groups that did not participate in a match (or matched an empty string)
contribute an
+ // empty string to the result, consistent with Spark.
+ // emit_empty_matches selects Spark semantics (explicit group index):
successful
+ // zero-width matches are emitted and one terminal-position search is
allowed,
+ // with UTF-8-safe advancement. The legacy two-argument form keeps skipping
+ // zero-width matches and never searches the terminal position.
+ void match_all_and_extract(const char* data, size_t size, int index,
+ std::vector<std::string>& results, bool
emit_empty_matches) const {
+ if (index < 0) {
+ return;
+ }
if (is_re2()) {
int max_matches = 1 + re2_regex->NumberOfCapturingGroups();
- if (max_matches < 2) {
- return; // No capturing groups
+ if (index >= max_matches) {
+ return;
}
size_t pos = 0;
- while (pos < size) {
- const char* str_pos = data + pos;
- size_t str_size = size - pos;
+ while (pos <= size) {
std::vector<re2::StringPiece> matches(max_matches);
- bool success = re2_regex->Match(re2::StringPiece(str_pos,
str_size), 0, str_size,
+ // Search within the original subject starting from pos, so
`^` stays
+ // anchored to the beginning of the original string.
+ bool success = re2_regex->Match(re2::StringPiece(data, size),
pos, size,
Review Comment:
[P1] Preserve suffix anchoring for the legacy arity
Searching the original subject is needed for the new explicit-index
semantics, but this shared call also changes existing two-argument results.
Before this PR, `regexp_extract_all('aa', '^(a)')` searched each remaining
suffix as a new subject and returned `['a','a']`; this now keeps `^` tied to
offset 0 and returns only `['a']` (the array form changes too). The existing
original-subject thread covered an explicitly supplied index, while the PR
promises the no-index form remains unchanged. Please split the cursor semantics
by arity, or explicitly declare and test the legacy behavior change.
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -136,52 +137,93 @@ struct RegexpExtractEngine {
return false;
}
- // Match all occurrences and extract the first capturing group
- void match_all_and_extract(const char* data, size_t size,
- std::vector<std::string>& results) const {
+ // Match all occurrences and extract the capturing group with the given
index.
+ // index 0 means the whole match, 1 means the first capturing group (the
default), and so on.
+ // Groups that did not participate in a match (or matched an empty string)
contribute an
+ // empty string to the result, consistent with Spark.
+ // emit_empty_matches selects Spark semantics (explicit group index):
successful
+ // zero-width matches are emitted and one terminal-position search is
allowed,
+ // with UTF-8-safe advancement. The legacy two-argument form keeps skipping
+ // zero-width matches and never searches the terminal position.
+ void match_all_and_extract(const char* data, size_t size, int index,
+ std::vector<std::string>& results, bool
emit_empty_matches) const {
+ if (index < 0) {
+ return;
+ }
if (is_re2()) {
int max_matches = 1 + re2_regex->NumberOfCapturingGroups();
- if (max_matches < 2) {
- return; // No capturing groups
+ if (index >= max_matches) {
+ return;
}
size_t pos = 0;
- while (pos < size) {
- const char* str_pos = data + pos;
- size_t str_size = size - pos;
+ while (pos <= size) {
std::vector<re2::StringPiece> matches(max_matches);
- bool success = re2_regex->Match(re2::StringPiece(str_pos,
str_size), 0, str_size,
+ // Search within the original subject starting from pos, so
`^` stays
+ // anchored to the beginning of the original string.
+ bool success = re2_regex->Match(re2::StringPiece(data, size),
pos, size,
re2::RE2::UNANCHORED,
matches.data(), max_matches);
if (!success) {
break;
}
- if (matches[0].empty()) {
+ const re2::StringPiece& whole = matches[0];
+ if (whole.empty() && !emit_empty_matches) {
+ if (pos >= size) {
+ break;
+ }
pos += 1;
continue;
}
- // Extract first capturing group
- if (matches.size() > 1 && !matches[1].empty()) {
- results.emplace_back(matches[1].data(), matches[1].size());
+ // Extract the capturing group with the given index
+ if (static_cast<size_t>(index) < matches.size()) {
+ const re2::StringPiece& group = matches[index];
+ if (group.data() != nullptr) {
+ results.emplace_back(group.data(), group.size());
+ } else {
+ results.emplace_back();
+ }
+ }
+ if (whole.empty()) {
+ if (pos >= size) {
+ break;
+ }
+ // Advance one full UTF-8 character, never into a
continuation byte.
+ pos +=
get_utf8_byte_length(static_cast<uint8_t>(data[pos]));
Review Comment:
[P1] Advance from the returned RE2 empty-match offset
An unanchored RE2 search can return a zero-width match strictly after `pos`,
but this advances from `data[pos]` instead of from `whole.data()`. For example,
`regexp_extract_all_array('ab', '\\b', 0)` should emit the two boundaries at
offsets 0 and 2. After the first match sets `pos` to 1, the next search finds
offset 2; this line advances only to 2, so the following iteration emits that
same boundary again. The earlier progress thread covered the Boost branch,
which now correctly uses `matches[0].first`; please compute the RE2 match
offset from `whole.data() - data` before the UTF-8 step and add this
later-than-origin case for both outputs.
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -136,52 +137,93 @@ struct RegexpExtractEngine {
return false;
}
- // Match all occurrences and extract the first capturing group
- void match_all_and_extract(const char* data, size_t size,
- std::vector<std::string>& results) const {
+ // Match all occurrences and extract the capturing group with the given
index.
+ // index 0 means the whole match, 1 means the first capturing group (the
default), and so on.
+ // Groups that did not participate in a match (or matched an empty string)
contribute an
+ // empty string to the result, consistent with Spark.
+ // emit_empty_matches selects Spark semantics (explicit group index):
successful
+ // zero-width matches are emitted and one terminal-position search is
allowed,
+ // with UTF-8-safe advancement. The legacy two-argument form keeps skipping
+ // zero-width matches and never searches the terminal position.
+ void match_all_and_extract(const char* data, size_t size, int index,
+ std::vector<std::string>& results, bool
emit_empty_matches) const {
+ if (index < 0) {
+ return;
+ }
if (is_re2()) {
int max_matches = 1 + re2_regex->NumberOfCapturingGroups();
- if (max_matches < 2) {
- return; // No capturing groups
+ if (index >= max_matches) {
+ return;
}
size_t pos = 0;
- while (pos < size) {
- const char* str_pos = data + pos;
- size_t str_size = size - pos;
+ while (pos <= size) {
std::vector<re2::StringPiece> matches(max_matches);
- bool success = re2_regex->Match(re2::StringPiece(str_pos,
str_size), 0, str_size,
+ // Search within the original subject starting from pos, so
`^` stays
+ // anchored to the beginning of the original string.
+ bool success = re2_regex->Match(re2::StringPiece(data, size),
pos, size,
re2::RE2::UNANCHORED,
matches.data(), max_matches);
if (!success) {
break;
}
- if (matches[0].empty()) {
+ const re2::StringPiece& whole = matches[0];
+ if (whole.empty() && !emit_empty_matches) {
+ if (pos >= size) {
+ break;
+ }
pos += 1;
continue;
}
- // Extract first capturing group
- if (matches.size() > 1 && !matches[1].empty()) {
- results.emplace_back(matches[1].data(), matches[1].size());
+ // Extract the capturing group with the given index
+ if (static_cast<size_t>(index) < matches.size()) {
Review Comment:
[P1] Preserve nonparticipating groups for the legacy arity
This unconditional append also runs when `emit_empty_matches` is false, so
it changes existing two-argument queries. For example, `regexp_extract_all('a
b', '(a)|(b)')` previously skipped the unmatched default group 1 on the `b`
match and returned `['a']`; it now appends an empty element and returns
`['a','']` (the array variant changes likewise). The existing thread about
preserving empty groups covered an explicitly supplied index, whereas the PR
promises the no-index form remains unchanged. Please retain the former
unmatched/empty-group filtering for the two-argument path and add a
legacy-arity case for both outputs.
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -136,52 +137,93 @@ struct RegexpExtractEngine {
return false;
}
- // Match all occurrences and extract the first capturing group
- void match_all_and_extract(const char* data, size_t size,
- std::vector<std::string>& results) const {
+ // Match all occurrences and extract the capturing group with the given
index.
+ // index 0 means the whole match, 1 means the first capturing group (the
default), and so on.
+ // Groups that did not participate in a match (or matched an empty string)
contribute an
+ // empty string to the result, consistent with Spark.
+ // emit_empty_matches selects Spark semantics (explicit group index):
successful
+ // zero-width matches are emitted and one terminal-position search is
allowed,
+ // with UTF-8-safe advancement. The legacy two-argument form keeps skipping
+ // zero-width matches and never searches the terminal position.
+ void match_all_and_extract(const char* data, size_t size, int index,
+ std::vector<std::string>& results, bool
emit_empty_matches) const {
+ if (index < 0) {
+ return;
+ }
if (is_re2()) {
int max_matches = 1 + re2_regex->NumberOfCapturingGroups();
- if (max_matches < 2) {
- return; // No capturing groups
+ if (index >= max_matches) {
+ return;
}
size_t pos = 0;
- while (pos < size) {
- const char* str_pos = data + pos;
- size_t str_size = size - pos;
+ while (pos <= size) {
std::vector<re2::StringPiece> matches(max_matches);
- bool success = re2_regex->Match(re2::StringPiece(str_pos,
str_size), 0, str_size,
+ // Search within the original subject starting from pos, so
`^` stays
+ // anchored to the beginning of the original string.
+ bool success = re2_regex->Match(re2::StringPiece(data, size),
pos, size,
re2::RE2::UNANCHORED,
matches.data(), max_matches);
if (!success) {
break;
}
- if (matches[0].empty()) {
+ const re2::StringPiece& whole = matches[0];
+ if (whole.empty() && !emit_empty_matches) {
+ if (pos >= size) {
+ break;
+ }
pos += 1;
continue;
}
- // Extract first capturing group
- if (matches.size() > 1 && !matches[1].empty()) {
- results.emplace_back(matches[1].data(), matches[1].size());
+ // Extract the capturing group with the given index
+ if (static_cast<size_t>(index) < matches.size()) {
+ const re2::StringPiece& group = matches[index];
+ if (group.data() != nullptr) {
+ results.emplace_back(group.data(), group.size());
+ } else {
+ results.emplace_back();
+ }
+ }
+ if (whole.empty()) {
+ if (pos >= size) {
+ break;
+ }
+ // Advance one full UTF-8 character, never into a
continuation byte.
+ pos +=
get_utf8_byte_length(static_cast<uint8_t>(data[pos]));
+ } else {
+ // Advance past the match via its pointer into the
original subject.
+ pos = (whole.data() - data) + whole.size();
}
- // Move position forward
- auto offset = std::string(str_pos, str_size)
- .find(std::string(matches[0].data(),
matches[0].size()));
- pos += offset + matches[0].size();
}
} else if (is_boost()) {
const char* search_start = data;
const char* search_end = data + size;
boost::match_results<const char*> matches;
- while (boost::regex_search(search_start, search_end, matches,
*boost_regex)) {
- if (matches.size() > 1 && matches[1].matched) {
- results.emplace_back(matches[1].str());
+ // Keep the original subject start reachable: match_prev_avail lets
+ // look-behind assertions see characters before search_start, and
+ // match_not_bob keeps `\A` anchored to the start of the original
+ // buffer (Boost's documented repeated-regex_search idiom; with
+ // match_prev_avail set, `^` is decided by the preceding
character).
+ while (search_start <= search_end &&
+ boost::regex_search(search_start, search_end, matches,
*boost_regex,
+ search_start == data
+ ? boost::match_default
+ : boost::match_prev_avail |
boost::match_not_bob)) {
+ const bool empty_match = matches[0].length() == 0;
+ if (!empty_match || emit_empty_matches) {
+ if (static_cast<size_t>(index) < matches.size()) {
+ results.emplace_back(matches[index].str());
+ }
}
- if (matches[0].length() == 0) {
- if (search_start == search_end) {
+ if (empty_match) {
+ // Advance past the zero-width match itself (one full UTF-8
+ // character), otherwise a match found after the origin
would be
+ // emitted twice or the next search could split a
multibyte char.
+ if (matches[0].first == search_end) {
break;
}
- search_start += 1;
+ search_start =
Review Comment:
[P2] Apply the mandatory clang-format v16 pass
The required formatter rejects both changed BE files: `clang-format-16
--dry-run --Werror be/src/exprs/function/function_regexp.cpp
be/test/exprs/function/function_like_test.cpp` reports this cursor wrapping
plus the new include/null-map code and several changed unit-test lines. The
base revisions of both files pass the same check, so these violations are
introduced here. Please run `build-support/clang-format.sh` on the changed C++
files so the repository's formatting gate can pass.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
