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]

Reply via email to