github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4002844202
##########
be/src/storage/index/inverted/char_filter/icu_normalizer_char_filter.cpp:
##########
@@ -59,33 +60,68 @@ void ICUNormalizerCharFilter::fill() {
input.resize(_reader->size());
_reader->readCopy(input.data(), 0, static_cast<int32_t>(input.size()));
normalize_text(input, _buf);
+ build_source_byte_offset_map();
_transformed_input.init(_buf.data(), static_cast<int32_t>(_buf.size()),
false);
}
void ICUNormalizerCharFilter::normalize_text(const std::string& input,
std::string& output) {
output.clear();
+ _edits.reset();
if (input.empty()) {
return;
}
UErrorCode status = U_ZERO_ERROR;
- icu::UnicodeString src16 = icu::UnicodeString::fromUTF8(input);
- UNormalizationCheckResult quick_result = _normalizer->quickCheck(src16,
status);
- if (U_SUCCESS(status) && quick_result == UNORM_YES) {
- output = input;
- return;
- }
-
- icu::UnicodeString result16;
- status = U_ZERO_ERROR;
- _normalizer->normalize(src16, result16, status);
+ icu::StringByteSink<std::string> sink(&output);
+ _normalizer->normalizeUTF8(0, icu::StringPiece(input), sink, &_edits,
status);
if (U_FAILURE(status)) {
LOG(WARNING) << "ICU normalize failed: " << u_errorName(status) << ",
using original text";
output = input;
+ _edits.reset();
+ _edits.addUnchanged(static_cast<int32_t>(input.size()));
return;
}
+}
+
+void ICUNormalizerCharFilter::build_source_byte_offset_map() {
+ _source_byte_offsets.clear();
+ _source_byte_offsets.reserve(_buf.size() + 1);
+ _source_byte_offsets.push_back(0);
+
+ UErrorCode status = U_ZERO_ERROR;
+ auto iterator = _edits.getFineIterator();
+ while (iterator.next(status)) {
+ if (U_FAILURE(status) ||
+ iterator.destinationIndex() !=
static_cast<int32_t>(_source_byte_offsets.size() - 1)) {
+ _source_byte_offsets.clear();
+ return;
+ }
+
+ const int32_t source_start = iterator.sourceIndex();
+ const int32_t source_end = source_start + iterator.oldLength();
+ if (iterator.hasChange()) {
+ // ICU maps the start of a replacement to the start of its source
span, and every
+ // later destination boundary in that replacement to the end of
the source span.
+ for (int32_t i = 0; i < iterator.newLength(); ++i) {
Review Comment:
[P1] Advance shared boundaries across deletion edits. A changed span
contributes `source_end` exactly `newLength()` times, so an ICU deletion
(`oldLength() > 0`, `newLength() == 0`) updates no table entry and leaves the
shared destination boundary before the deleted bytes; the size check still
passes. Default `nfkc_cf` removes U+00AD, so `liu<U+00AD>de` normalizes to
`liude` but boundary 3 maps to source byte 3 rather than byte 5, and
offset-aware IK -> Pinyin reports `de` as `[3,7)` instead of `[5,7)`. This is
distinct from the prior replacement/scaling threads because it is a
zero-length-edit error in the new table. Please advance the existing boundary
for deletions with ICU-compatible adjacent/trailing semantics and add exact
`nfkc_cf` deletion tests through ICU -> IK -> Pinyin, including reset and
nesting.
##########
be/src/storage/index/inverted/char_filter/icu_normalizer_char_filter.cpp:
##########
@@ -59,33 +60,68 @@ void ICUNormalizerCharFilter::fill() {
input.resize(_reader->size());
_reader->readCopy(input.data(), 0, static_cast<int32_t>(input.size()));
normalize_text(input, _buf);
+ build_source_byte_offset_map();
_transformed_input.init(_buf.data(), static_cast<int32_t>(_buf.size()),
false);
}
void ICUNormalizerCharFilter::normalize_text(const std::string& input,
std::string& output) {
output.clear();
+ _edits.reset();
if (input.empty()) {
return;
}
UErrorCode status = U_ZERO_ERROR;
- icu::UnicodeString src16 = icu::UnicodeString::fromUTF8(input);
- UNormalizationCheckResult quick_result = _normalizer->quickCheck(src16,
status);
- if (U_SUCCESS(status) && quick_result == UNORM_YES) {
- output = input;
- return;
- }
-
- icu::UnicodeString result16;
- status = U_ZERO_ERROR;
- _normalizer->normalize(src16, result16, status);
+ icu::StringByteSink<std::string> sink(&output);
+ _normalizer->normalizeUTF8(0, icu::StringPiece(input), sink, &_edits,
status);
if (U_FAILURE(status)) {
LOG(WARNING) << "ICU normalize failed: " << u_errorName(status) << ",
using original text";
output = input;
+ _edits.reset();
+ _edits.addUnchanged(static_cast<int32_t>(input.size()));
return;
}
+}
+
+void ICUNormalizerCharFilter::build_source_byte_offset_map() {
+ _source_byte_offsets.clear();
+ _source_byte_offsets.reserve(_buf.size() + 1);
Review Comment:
[P1] Avoid a dense offset entry for every normalized byte. `fill()` now
unconditionally builds this vector and reserves one `int32_t` per output byte,
including unchanged/sparse-edit text and analyzer chains whose tokenizer never
consumes `correct_offset()`. This is in addition to the full input copy,
normalized buffer, ICU `Edits`, and tokenizer state. Analyzed strings are not
bounded by `ignore_above`, so a 100 MiB value requests about 400 MiB for this
table alone and concurrent indexing can hit the BE memory limit. This is a new
resource consequence of the fix for the earlier quadratic lookup thread. Please
retain compact edit spans/checkpoints with bounded lookup cost instead of
materializing every byte, with a large sparse-edit memory regression or
benchmark.
##########
be/src/storage/index/inverted/token_filter/pinyin_filter.cpp:
##########
@@ -392,37 +402,58 @@ void PinyinFilter::addCandidate(const TermItem& item) {
candidate_.push_back(new_item);
}
-void PinyinFilter::processAsciiBuffer(const std::string& ascii_buffer, int
start_pos, int end_pos) {
+void PinyinFilter::processAsciiBuffer(const std::string& ascii_buffer,
+ const std::vector<int>&
source_rune_indices) {
if (ascii_buffer.empty() || !config_->keepNoneChinese) {
return;
}
+ DORIS_CHECK_EQ(ascii_buffer.size(), source_rune_indices.size());
if (config_->noneChinesePinyinTokenize) {
// Use PinyinAlphabetTokenizer to split ASCII buffer into meaningful
tokens
std::vector<std::string> tokens =
PinyinAlphabetTokenizer::walk(ascii_buffer);
- int current_offset = start_pos;
+ size_t compact_offset = 0;
+ int fixed_offset = source_rune_indices.front();
for (const auto& token : tokens) {
+ const size_t compact_end = compact_offset + token.size();
+ DORIS_CHECK_LE(compact_end, source_rune_indices.size());
position_++;
- int token_end = (config_->fixedPinyinOffset)
- ? (current_offset + 1)
- : (current_offset +
static_cast<int>(token.length()));
- addCandidate(TermItem(token, current_offset, token_end,
position_));
- current_offset = token_end;
+ if (config_->fixedPinyinOffset) {
+ addCandidate(TermItem(token, fixed_offset, fixed_offset + 1,
position_));
+ ++fixed_offset;
+ } else {
+ const int source_start = source_rune_indices[compact_offset];
+ const int source_end = source_rune_indices[compact_end - 1] +
1;
+ addCandidate(TermItem(token, source_start, source_end,
position_));
+ }
+ compact_offset = compact_end;
}
+ DORIS_CHECK_EQ(compact_offset, source_rune_indices.size());
} else {
// Treat the entire ASCII buffer as a single token
position_++;
- addCandidate(TermItem(ascii_buffer, start_pos, end_pos, position_));
+ addCandidate(TermItem(ascii_buffer, source_rune_indices.front(),
+ source_rune_indices.back() + 1, position_));
}
}
void PinyinFilter::setTokenAttributes(Token* token, const std::string& term,
int start_offset,
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_;
Review Comment:
[P1] Do not inherit unset offsets for whole-token alternatives. This new
whole-token branch assumes every upstream tokenizer publishes a document span,
but supported custom tokenizers such as `KeywordTokenizer` and
`StandardTokenizer` call `DorisTokenStream::set()`, which sets only text and
position. In a valid keyword -> Pinyin analyzer over `刘德华`,
original/first-letter/joined whole-token alternatives therefore inherit the
default `[0,0)` range instead of `[0,9)`; with a reused `Token` on a
multi-token stream the inherited range can even be stale from the preceding
Pinyin output. The existing keyword tests check terms only. Please define and
enforce upstream offset publication before trusting these fields, or derive the
whole-token range from reliable source boundaries, and add exact keyword plus
multi-token reset/reuse offset assertions.
--
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]
