This is an automated email from the ASF dual-hosted git repository. airborne12 pushed a commit to branch feature/ik-custom-tokenizers in repository https://gitbox.apache.org/repos/asf/doris.git
commit 5843206b21cee1efa15663443a1e0cfee0550cc2 Author: airborne12 <[email protected]> AuthorDate: Wed Sep 23 03:13:52 2026 +0800 [fix](inverted-index) Clip Pinyin provenance to the emitted prefix and keep scoring analyzer failures as Status The Pinyin tokenizer capped an over-cap candidate at LUCENE_MAX_WORD_LEN bytes but still published the full candidate range, so the emitted prefix claimed source it did not represent and the byte cap could split a rune. Clip on a rune boundary through a shared utf8_prefix_at_most() helper and narrow the end offset and provenance to the emitted prefix. Building the scoring analyzer context happened outside the guarded region of SearchPredicateCollector, so the wrong-family exception from process_filter_configs escaped a Status-returning path. Return a Result from analyzer_context_from_properties() and wrap provider construction. --- .../index/inverted/analyzer/ik/IKTokenizer.cpp | 17 --------- .../inverted/similarity/predicate_collector.cpp | 26 ++++++++++++- .../inverted/similarity/predicate_collector.h | 8 ++++ be/src/storage/index/inverted/token_stream.h | 21 +++++++++++ .../inverted/tokenizer/pinyin/pinyin_tokenizer.cpp | 14 ++++++- .../similarity/collection_statistics_test.cpp | 31 ++++++++++++++++ .../inverted/token_filter/pinyin_filter_test.cpp | 43 ++++++++++++++++++++++ 7 files changed, 140 insertions(+), 20 deletions(-) diff --git a/be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp b/be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp index 19378d21771..88c6b97d6c5 100644 --- a/be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp +++ b/be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp @@ -61,23 +61,6 @@ void regularize_with_source_byte_offsets(std::string& token, bool lowercase, token = std::move(normalized); } -std::pair<size_t, size_t> utf8_prefix_at_most(std::string_view text, size_t max_bytes) { - const auto length = static_cast<int32_t>(text.size()); - const auto limit = static_cast<int32_t>(std::min(text.size(), max_bytes)); - int32_t offset = 0; - size_t rune_count = 0; - while (offset < length) { - int32_t next = offset; - U8_FWD_1(text, next, length); - if (next > limit) { - break; - } - offset = next; - ++rune_count; - } - return {static_cast<size_t>(offset), rune_count}; -} - } // namespace IKTokenizer::IKTokenizer(std::shared_ptr<Configuration> config, bool lower_case, bool own_reader) { diff --git a/be/src/storage/index/inverted/similarity/predicate_collector.cpp b/be/src/storage/index/inverted/similarity/predicate_collector.cpp index 095ea6977cf..282cdc2c3ce 100644 --- a/be/src/storage/index/inverted/similarity/predicate_collector.cpp +++ b/be/src/storage/index/inverted/similarity/predicate_collector.cpp @@ -41,7 +41,7 @@ using namespace segment_v2; namespace { -InvertedIndexAnalyzerCtx analyzer_context_from_properties( +InvertedIndexAnalyzerCtx build_analyzer_context( const std::map<std::string, std::string>& properties) { InvertedIndexAnalyzerConfig config; config.analyzer_name = get_analyzer_name_from_properties(properties); @@ -61,6 +61,24 @@ InvertedIndexAnalyzerCtx analyzer_context_from_properties( return analyzer_ctx; } +} // namespace + +Result<InvertedIndexAnalyzerCtx> analyzer_context_from_properties( + const std::map<std::string, std::string>& properties) { + // Replayed components can collide across policy families, so building the provider throws. + try { + return build_analyzer_context(properties); + } catch (const CLuceneError& error) { + return ResultError(Status::Error<ErrorCode::INVERTED_INDEX_ANALYZER_ERROR>( + "Build scoring analyzer failed: {}", error.what())); + } catch (const Exception& error) { + return ResultError(Status::Error<ErrorCode::INVERTED_INDEX_ANALYZER_ERROR>( + "Build scoring analyzer failed: {}", error.what())); + } +} + +namespace { + Result<std::vector<TermInfo>> analyze_plain_query(const std::string& value, const InvertedIndexAnalyzerCtx& analyzer_ctx) { DORIS_CHECK(analyzer_ctx.analyzer_provider != nullptr); @@ -527,7 +545,11 @@ Status SearchPredicateCollector::collect_from_leaf(const TSearchClause& clause, std::vector<TermInfo> term_infos; std::optional<InvertedIndexAnalyzerCtx> analyzer_ctx; if (InvertedIndexAnalyzer::should_analyzer(analysis_properties)) { - analyzer_ctx.emplace(analyzer_context_from_properties(analysis_properties)); + auto built_ctx = analyzer_context_from_properties(analysis_properties); + if (!built_ctx.has_value()) { + return built_ctx.error(); + } + analyzer_ctx.emplace(std::move(built_ctx.value())); } if (clause_type == "MATCH") { diff --git a/be/src/storage/index/inverted/similarity/predicate_collector.h b/be/src/storage/index/inverted/similarity/predicate_collector.h index 7cbbd2d1cd2..de3860c40b9 100644 --- a/be/src/storage/index/inverted/similarity/predicate_collector.h +++ b/be/src/storage/index/inverted/similarity/predicate_collector.h @@ -18,6 +18,7 @@ #pragma once #include <cstdint> +#include <map> #include <memory> #include <string> #include <unordered_map> @@ -31,6 +32,13 @@ namespace doris { +struct InvertedIndexAnalyzerCtx; + +// Build the analyzer context of an index, converting a failure to build the analyzer provider +// into a Status instead of letting the exception escape a Status-returning caller. +Result<InvertedIndexAnalyzerCtx> analyzer_context_from_properties( + const std::map<std::string, std::string>& properties); + class VSlotRef; class TabletIndex; class TabletSchema; diff --git a/be/src/storage/index/inverted/token_stream.h b/be/src/storage/index/inverted/token_stream.h index 98b7d1bcba8..6b26ddf6ee8 100644 --- a/be/src/storage/index/inverted/token_stream.h +++ b/be/src/storage/index/inverted/token_stream.h @@ -18,11 +18,14 @@ #pragma once #include <unicode/utext.h> +#include <unicode/utf8.h> +#include <algorithm> #include <cstddef> #include <memory> #include <span> #include <string_view> +#include <utility> #include "CLucene.h" #include "CLucene/analysis/AnalysisHeader.h" @@ -42,6 +45,24 @@ using TokenStreamPtr = std::shared_ptr<TokenStream>; // capacity for the rest of the writer lifetime. constexpr size_t ANALYZER_SCRATCH_HIGH_WATER_BYTES = 64 * 1024; +// Longest prefix of text that fits in max_bytes without splitting a rune, and its rune count. +inline std::pair<size_t, size_t> utf8_prefix_at_most(std::string_view text, size_t max_bytes) { + const auto length = static_cast<int32_t>(text.size()); + const auto limit = static_cast<int32_t>(std::min(text.size(), max_bytes)); + int32_t offset = 0; + size_t rune_count = 0; + while (offset < length) { + int32_t next = offset; + U8_FWD_1(text, next, length); + if (next > limit) { + break; + } + offset = next; + ++rune_count; + } + return {static_cast<size_t>(offset), rune_count}; +} + template <typename Container> void release_oversized_scratch(Container& container) { if (container.capacity() * sizeof(typename Container::value_type) > diff --git a/be/src/storage/index/inverted/tokenizer/pinyin/pinyin_tokenizer.cpp b/be/src/storage/index/inverted/tokenizer/pinyin/pinyin_tokenizer.cpp index 91699118b12..4e9d477a98e 100644 --- a/be/src/storage/index/inverted/tokenizer/pinyin/pinyin_tokenizer.cpp +++ b/be/src/storage/index/inverted/tokenizer/pinyin/pinyin_tokenizer.cpp @@ -217,7 +217,9 @@ Token* PinyinTokenizer::next(Token* token) { candidate_offset_++; const std::string& text = item.term; - size_t size = std::min(text.size(), static_cast<size_t>(LUCENE_MAX_WORD_LEN)); + // Clip on a rune boundary so a split rune is never published. + const size_t size = + utf8_prefix_at_most(text, static_cast<size_t>(LUCENE_MAX_WORD_LEN)).first; token->setNoCopy(text.data(), 0, static_cast<int32_t>(size)); int32_t start = item.start_offset; @@ -225,6 +227,16 @@ Token* PinyinTokenizer::next(Token* token) { if (config_->ignorePinyinOffset) { start = 0; end = runes_.empty() ? 0 : runes_.back().byte_end; + } else if (size < text.size()) { + // A clipped candidate must not claim the source of what it did not emit; when the + // candidate is the source slice itself, that source is its own prefix. + const int32_t clipped_start = std::clamp(start, 0, _char_length); + const int32_t clipped_end = std::clamp(end, clipped_start, _char_length); + if (std::string_view(_char_buffer + clipped_start, clipped_end - clipped_start) == + text) { + start = clipped_start; + end = clipped_start + static_cast<int32_t>(size); + } } token->setStartOffset(correct_source_start_offset(start)); token->setEndOffset(correct_source_offset(end)); diff --git a/be/test/storage/index/inverted/similarity/collection_statistics_test.cpp b/be/test/storage/index/inverted/similarity/collection_statistics_test.cpp index ca4854e9420..aa7c8be7a3b 100644 --- a/be/test/storage/index/inverted/similarity/collection_statistics_test.cpp +++ b/be/test/storage/index/inverted/similarity/collection_statistics_test.cpp @@ -47,6 +47,7 @@ #include "storage/index/index_writer.h" #include "storage/index/inverted/analyzer/analyzer.h" #include "storage/index/inverted/inverted_index_desc.h" +#include "storage/index/inverted/similarity/predicate_collector.h" #include "storage/index/inverted/util/string_helper.h" #include "storage/index/snii/format/phrase_bigram.h" #include "storage/index/snii/query/bm25_scorer.h" @@ -2695,4 +2696,34 @@ TEST_F(CollectionStatisticsTest, BuildFieldNameWithoutSuffix) { EXPECT_EQ(collector.build_field_name(42, ""), "42"); } +TEST_F(CollectionStatisticsTest, ScoringAnalyzerContextReportsWrongFamilyComponentAsStatus) { + IndexPolicyMgr policy_mgr; + auto* exec_env = ExecEnv::GetInstance(); + auto* original_policy_mgr = exec_env->index_policy_mgr(); + exec_env->_index_policy_mgr = &policy_mgr; + Defer restore_policy_mgr([&] { exec_env->_index_policy_mgr = original_policy_mgr; }); + + // Replay can keep a component whose family no longer matches how the analyzer uses it. + TIndexPolicy char_filter; + char_filter.id = 200; + char_filter.name = "Wrong"; + char_filter.type = TIndexPolicyType::CHAR_FILTER; + char_filter.properties["type"] = "char_replace"; + TIndexPolicy analyzer; + analyzer.id = 201; + analyzer.name = "wrong_family_analyzer"; + analyzer.type = TIndexPolicyType::ANALYZER; + analyzer.properties["tokenizer"] = "keyword"; + analyzer.properties["token_filter"] = "Wrong"; + policy_mgr.apply_policy_changes({char_filter, analyzer}, {}); + + const std::map<std::string, std::string> properties = {{"analyzer", "wrong_family_analyzer"}}; + auto analyzer_ctx = analyzer_context_from_properties(properties); + ASSERT_FALSE(analyzer_ctx.has_value()); + EXPECT_EQ(analyzer_ctx.error().code(), ErrorCode::INVERTED_INDEX_ANALYZER_ERROR); + + const std::map<std::string, std::string> valid = {{"parser", "english"}}; + EXPECT_TRUE(analyzer_context_from_properties(valid).has_value()); +} + } // namespace doris diff --git a/be/test/storage/index/inverted/token_filter/pinyin_filter_test.cpp b/be/test/storage/index/inverted/token_filter/pinyin_filter_test.cpp index 0de2f5d4836..1a7b28198ff 100644 --- a/be/test/storage/index/inverted/token_filter/pinyin_filter_test.cpp +++ b/be/test/storage/index/inverted/token_filter/pinyin_filter_test.cpp @@ -1092,6 +1092,49 @@ TEST_F(PinyinFilterTest, TestCaseAndFoldingFiltersCountRunesOnlyForOffsets) { } } +TEST_F(PinyinFilterTest, TestPinyinTokenizerClipsOriginalCandidateProvenance) { + Settings settings; + settings.set("keep_first_letter", "false"); + settings.set("keep_full_pinyin", "false"); + settings.set("keep_none_chinese", "false"); + settings.set("keep_original", "true"); + settings.set("ignore_pinyin_offset", "false"); + PinyinTokenizerFactory factory; + factory.initialize(settings); + + // An over-cap original candidate publishes only its prefix, so it owns only that source. + const std::string ascii(300, 'a'); + auto ascii_reader = std::make_shared<lucene::util::SStringReader<char>>(); + ascii_reader->init(ascii.data(), static_cast<int32_t>(ascii.size()), false); + auto tokenizer = factory.create(); + tokenizer->set_reader(ascii_reader); + tokenizer->set_source_byte_offsets_enabled(true); + tokenizer->reset(); + + Token token; + ASSERT_NE(tokenizer->next(&token), nullptr); + EXPECT_EQ(token.termLength<char>(), 255); + EXPECT_EQ(token.startOffset(), 0); + EXPECT_EQ(token.endOffset(), 255); + EXPECT_EQ(tokenizer->get_source_byte_offsets().size(), 256); + EXPECT_EQ(tokenizer->get_source_byte_offsets().back(), 255); + + // A cap that falls inside a rune clips to the rune boundary before it (2-byte runes here, + // so the 255-byte cap would split the 128th one). + std::string latin; + for (int i = 0; i < 200; ++i) { + latin += "\xC3\xA9"; // U+00E9 + } + auto latin_reader = std::make_shared<lucene::util::SStringReader<char>>(); + latin_reader->init(latin.data(), static_cast<int32_t>(latin.size()), false); + tokenizer->set_reader(latin_reader); + tokenizer->reset(); + ASSERT_NE(tokenizer->next(&token), nullptr); + EXPECT_EQ(token.termLength<char>(), 254); + EXPECT_EQ(token.endOffset(), 254); + EXPECT_EQ(tokenizer->get_source_byte_offsets().size(), 128); +} + TEST_F(PinyinFilterTest, TestOffsetTrackingReusesTokenizerScratchAcrossTokens) { for (const std::string tokenizer_type : {"standard", "ik_max_word"}) { SCOPED_TRACE(tokenizer_type); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
