github-actions[bot] commented on code in PR #68661:
URL: https://github.com/apache/doris/pull/68661#discussion_r4193752955
##########
be/src/exprs/function/match.cpp:
##########
@@ -382,42 +503,15 @@ Status
FunctionMatchPhrase::execute_match(FunctionContext* context, const std::s
auto current_src_array_offset = 0;
for (int i = 0; i < input_rows_count; i++) {
- auto data_tokens = analyse_data_token(column_name, analyzer_ctx,
string_col, i,
- array_offsets,
current_src_array_offset);
-
- // TODO: more efficient impl
- bool matched = false;
- auto data_it = data_tokens.begin();
- while (data_it != data_tokens.end()) {
- // find position of first token
- data_it =
- std::find_if(data_it, data_tokens.end(), [&](const
segment_v2::TermInfo& info) {
- return info.get_single_term() ==
query_tokens[0].get_single_term();
- });
- if (data_it != data_tokens.end()) {
- matched = true;
- auto data_it_next = ++data_it;
- auto query_it = query_tokens.begin() + 1;
- // compare query_tokens after the first to data_tokens one by
one
- while (query_it != query_tokens.end()) {
- if (data_it_next == data_tokens.end() ||
- data_it_next->get_single_term() !=
query_it->get_single_term()) {
- matched = false;
- break;
- }
- query_it++;
- data_it_next++;
- }
-
- if (matched) {
- break;
- }
+ const auto element_begin = array_offsets ? (*array_offsets)[i - 1] : i;
Review Comment:
[P1] Keep array phrase fallback consistent with indexed positions. With an
English-analyzed `ARRAY<STRING>` containing `["hello", "world"]` and
`support_phrase=true`, the index stores successive same-name fields at adjacent
positions (the pinned CLucene analyzer has a [default gap of
zero](https://github.com/apache/doris-thirdparty/blob/08d45e541fef1e48dc4ab00d2d5fde9569b49ee3/src/core/CLucene/analysis/AnalysisHeader.h#L349-L357)),
so indexed `MATCH_PHRASE "hello world"` matches while this loop returns false.
Prefix and edge likewise diverge for `hello wor` and `llo wor`. The old
fallback already missed this cross-element case; these new per-element loops
and negative tests preserve the mismatch as array phrase support expands.
Queries can return different rows depending on index use. Align fallback and
indexed semantics, including already-written segments.
##########
be/src/exprs/function/match.cpp:
##########
@@ -241,9 +355,12 @@ inline std::vector<segment_v2::TermInfo>
FunctionMatchBase::analyse_data_token(
auto reader =
doris::segment_v2::inverted_index::InvertedIndexAnalyzer::create_reader(
analyzer_ctx->char_filter_map);
reader->init(str_ref.data, (int)str_ref.size, true);
- data_tokens =
+ auto element_tokens =
doris::segment_v2::inverted_index::InvertedIndexAnalyzer::get_analyse_result(
reader, analyzer_ctx->analyzer.get());
+ for (auto& token : element_tokens) {
Review Comment:
[P2] Avoid retaining every analyzed token in an array row. This append keeps
all element tokens until `MATCH_ANY`, `MATCH_ALL`, or `MATCH_REGEXP` evaluates
the row. A feasible one-million-element English-analyzed array builds roughly
one million `TermInfo` values even when `MATCH_ANY "alpha"` is satisfied by
element zero; the old analyzed branch retained only one element's tokens at a
time. This creates a large new per-row memory spike in fallback queries.
Evaluate elements as they are tokenized and retain only the state each operator
needs.
--
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]
