github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4057324012
##########
be/src/storage/index/inverted/token_filter/pinyin_filter.cpp:
##########
@@ -79,6 +80,11 @@ Token* PinyinFilter::next(Token* token) {
current_token_text_ = std::string(token->termBuffer<char>(),
token->termLength<char>());
current_start_offset_ = token->startOffset();
current_end_offset_ = token->endOffset();
+ auto source_byte_offsets = get_source_byte_offsets();
+ current_source_byte_offsets_.assign(source_byte_offsets.begin(),
source_byte_offsets.end());
Review Comment:
This source-map snapshot is empty for several FE-valid upstream tokenizers
(`basic`, `icu`, `char_group`, `ngram`, `empty`): their `next()` methods still
call `set`/`setNoCopy` without publishing byte boundaries. With
`pinyin(ignore_pinyin_offset=false)`, Basic tokenizes `刘德华` as three `[0,len)`
tokens, so the Pinyin alternatives for later characters reuse local offsets
instead of document spans; the same happens after reset/multiple tokens. Please
make every supported upstream publish corrected source boundaries when offset
tracking is enabled, or reject this Pinyin setting for unsupported tokenizers,
and add a multi-token regression beyond Keyword/Standard.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/InvertedIndexUtil.java:
##########
@@ -423,17 +445,27 @@ public static boolean isAnalyzerMatched(Map<String,
String> properties, String a
buildAnalyzerIdentity(properties));
}
+ String resolvedAnalyzer = resolveAnalyzerName(normalizedAnalyzer);
String preferredAnalyzer =
InvertedIndexProperties.getPreferredAnalyzer(properties);
if (!Strings.isNullOrEmpty(preferredAnalyzer)) {
- return normalizedAnalyzer.equalsIgnoreCase(preferredAnalyzer);
Review Comment:
Two valid non-IK indexes can now differ only by the outer `char_replace`
settings because `AnalyzerIdentityBuilder` appends those settings to the
identity, but this matcher still compares only the resolved analyzer name. For
example, `standard` with `a->b` and `standard` with `x->y` both satisfy this
branch; `OlapTable.getInvertedIndex(..., "standard")` then keeps both and
chooses the first analyzed index. `MatchPredicate` serializes that arbitrary
index's char-filter map, so `USING ANALYZER standard` can analyze with the
wrong replacement and the other index is unreachable. Please carry the
effective outer-filter identity into FE selection (or reject same-name indexes
that cannot be disambiguated by the query syntax) and add a regression proving
the selected Thrift properties.
--
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]