github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4001183393
##########
be/src/storage/index/inverted/token_filter/pinyin_filter.cpp:
##########
@@ -421,8 +434,18 @@ void PinyinFilter::setTokenAttributes(Token* token, const
std::string& term, int
int end_offset, int position) {
set_text(token, term);
- token->setStartOffset(start_offset);
- token->setEndOffset(end_offset);
+ int absolute_start = current_start_offset_;
+ int absolute_end = current_end_offset_;
+ const bool is_whole_token =
+ start_offset == 0 && std::cmp_equal(end_offset,
current_source_.length());
+ if (!config_->ignorePinyinOffset && !is_whole_token && start_offset >= 0
&& end_offset > 0 &&
+ std::cmp_less(start_offset, current_runes_.size()) &&
+ std::cmp_less_equal(end_offset, current_runes_.size())) {
+ absolute_start += current_runes_[start_offset].byte_start;
Review Comment:
[P1] Preserve gaps when mapping Pinyin's ASCII splits
Direct IK can emit a connector-containing Latin run such as `liu-de` as one
token. Pinyin compacts it to `liude`, and the alphabet tokenizer assigns `de`
the compacted span `[3,5)`, but this new lookup applies that span to the
original rune array, so `de` covers the hyphen and `d` rather than `de`. For
full-width `LIU-DE`, that becomes source `[9,13)` instead of `[10,16)`. Please
retain each buffered character's original source-rune range while preserving
the intended compaction semantics, and cover connector-containing IK-to-Pinyin
offsets in both modes.
##########
be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp:
##########
@@ -31,27 +68,57 @@ Token* IKTokenizer::next(Token* token) {
return nullptr;
}
- std::string& token_text = tokens_text_[buffer_index_++];
+ TokenData& token_data = tokens_[buffer_index_++];
// full-width to half-width, and lowercase
// TODO(ryan19929): do regularizeString in fillBuffer.
- CharacterUtil::regularizeString(token_text, this->lowercase);
- size_t size = std::min(token_text.size(),
static_cast<size_t>(LUCENE_MAX_WORD_LEN));
- token->setNoCopy(token_text.data(), 0, static_cast<int32_t>(size));
+ if (source_byte_offsets_enabled_) {
+ current_source_byte_offsets_ =
+ regularize_with_source_byte_offsets(token_data.text,
this->lowercase);
+ } else {
+ CharacterUtil::regularizeString(token_data.text, this->lowercase);
+ current_source_byte_offsets_.clear();
+ }
+ current_token_ = &token_data;
+ size_t size = std::min(token_data.text.size(),
static_cast<size_t>(LUCENE_MAX_WORD_LEN));
Review Comment:
[P1] Keep provenance aligned with the published IK term
When this lexeme exceeds `LUCENE_MAX_WORD_LEN`, `set()` publishes only the
prefix, but `current_source_byte_offsets_` still contains boundaries for the
full normalized lexeme. Pinyin requires exactly one boundary per published
rune, so it discards this mismatched map and falls back to normalized-byte
positions. For example, a valid IK-to-Pinyin analyzer over repetitions of
full-width `LIUDE` past the limit emits the first `liu` at `[0,3)` instead of
its source `[0,9)`. Please choose a UTF-8/rune-safe published prefix, slice
provenance to the same boundary, make the token end-offset convention explicit,
and cover long full-width input plus reset/reuse.
--
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]