airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4078869277
##########
be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp:
##########
@@ -91,6 +96,9 @@ void NGramTokenizer::reset() {
_char_buffer = nullptr;
_char_offset = 0;
_char_length = _in->read((const void**)&_char_buffer, 0,
static_cast<int32_t>(_in->size()));
+ if (_char_length > 0 && !validate_utf8(_char_buffer, _char_length)) {
Review Comment:
Confirmed, and it was a regression this PR introduced in 31450230c09. Fixed
in 2ecd08d4c16.
`NGramTokenizer::reset()` and `ICUTokenizer::reset()` rejected the whole
input whenever `validate_utf8()` failed, regardless of whether offsets were
being tracked, while `to_code_points()` had always skipped a malformed code
point (`if (c < 0) continue;`) and kept indexing the valid neighbours. A column
that legitimately holds such bytes therefore turned every affected row into a
failed index write.
The rejection is now gated on `_source_byte_offsets_enabled`, which is the
one path that cannot map a source span through a malformed byte. That flag is
only turned on by `PinyinFilterFactory` when `ignore_pinyin_offset` is false,
so ordinary indexing is back to the previous behaviour while the earlier offset
repair still holds.
Two existing tests pinned the old rejection on the default path; both now
enable offset tracking instead, because the assertion encoded the regression
rather than the intended behaviour. Each tokenizer also gains a default-path
case: `NGramTokenizerTest.IndexesMalformedUtf8UnlessOffsetsAreTracked` and
`ICUTokenizerFactoryTest.IndexesMalformedUtf8UnlessOffsetsAreTracked`.
End-to-end writer coverage is in the new
`test_analyzer_malformed_utf8_write` suite: rows built with `UNHEX('C0AF')` and
`UNHEX('FF')` are inserted into ngram- and icu-analyzed columns, and it asserts
every row lands and that the text on both sides of the malformed bytes still
matches.
##########
be/src/runtime/index_policy/index_policy_mgr.cpp:
##########
@@ -291,29 +377,33 @@ void IndexPolicyMgr::process_filter_configs(
continue;
}
- // Use normalized name for case-insensitive lookup
std::string normalized_filter_name = normalize_name(filter_name);
- if (_name_to_id.contains(normalized_filter_name)) {
+ if (const auto* filter_policy =
find_policy_by_name_locked(filter_name);
+ filter_policy != nullptr) {
// Nested filter policy
- const int64_t filter_policy_id =
_name_to_id.at(normalized_filter_name);
- const auto& filter_policy = _policys.at(filter_policy_id);
- auto type_it = filter_policy.properties.find(PROP_TYPE);
- if (type_it == filter_policy.properties.end()) {
+ if (filter_policy->type != expected_type) {
Review Comment:
Confirmed and fixed in 2ecd08d4c16. Thanks - this is a genuinely separate
path that the scoring fix did not cover.
`FieldReaderResolver::resolve_with_analyzer_context()` returns `Status` but
called the file-local `build_analyzer_context()` in
`variant_inverted_index_search.cpp`, which reaches `create_analyzer_provider()`
with no catch, so a wrong-family replayed component threw out of it.
The helper is now `build_search_analyzer_context()`, returns
`Result<InvertedIndexAnalyzerCtxSPtr>`, maps both `CLuceneError` and
`Exception` to `INVERTED_INDEX_ANALYZER_ERROR`, and is declared in
`variant_inverted_index_search.h` so the direct SEARCH path can be exercised
without standing up index readers. The `requires_analysis()` gate and the
`analyzer_key` it records are unchanged.
Test: `SearchAnalyzerContextTest.ReportsWrongFamilyComponentAsStatus`
installs an `IndexPolicyMgr` that replays a CHAR_FILTER policy referenced as a
`token_filter` and asserts the call returns that status instead of throwing,
with a valid analyzer as the negative.
--
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]
