airborne12 opened a new pull request, #69318: URL: https://github.com/apache/doris/pull/69318
### What problem does this PR solve? Issue Number: None Related PR: https://github.com/apache/doris/pull/69051 Problem Summary: Backport the merged keyword SEARCH BM25 fix to branch-4.2. Projecting `score()` on keyword indexes can drop matching rows or reject missing scoring metadata. Keyword MATCH filters can also request statistics that were not collected, and scored SNII conjunctions can incorrectly require positions from docs-only terms. Use unit frequency and encoded norm 1 for keyword postings, preserve whole keyword values, and score docs-only terms through the existing individual term scorer. Legacy indexes without norms use average length 1; SNII retains its physical collection statistics. The BM25 formula and the restrictions for analyzed indexes remain unchanged. Source commit: `4016f207bfe7eff77f56f4d98d23629888539462`. #### Target branch adaptations - branch-4.2 already contains the unified MATCH/SEARCH engine. It does not contain master's normalizer-only scoring exception, so this pick retains the target analyzer-and-phrase eligibility policy and analyzed-index metadata requirements. - The older CLucene interface lacks norm-presence propagation. Pass `reader->hasNorms()` through posting partitions and restore it on partition switches, including WAND bounds. - Retain the target compile-check wrappers and adapt the statistics helper signatures without weakening assertions. Both source regression suites and their expected outputs are preserved verbatim. - GitHub's historical PR-head diff also shows three Variant parser-option compatibility files. They are absent from the final merged squash, and branch-4.2 still has that parser option. They are excluded. Merging the PR head `9b356cb594f60b802de117ff9b5915b60500d424` into the landing parent reproduces the exact landed tree `57f5e5d63fafe808a83bc393e36ea59a6e367ca0`. ### Release note SEARCH with `score()` supports keyword inverted indexes, including conjunctions, TopN ordering, WAND execution, and keyword MATCH filters. ### Check List (For Author) - Test - [x] Regression test: both source suites passed; all 114 expected tags ran exactly once (54 raw-index and 60 keyword-filter checks). - [x] Unit Test: 369 affected ASAN tests passed; 3 pre-existing benchmarks are disabled. All 7 source-added or renamed cases were discovered and executed. - [ ] Manual test - [ ] No need to test or manual test. - Behavior changed: - [ ] No. - [x] Yes, as described above. - Does this need documentation? - [x] No. No syntax or configuration changes. - [ ] Yes. ### Backport validation - PASS: `source ./custom_env.sh && bash build.sh --be --fe -j 24` (ASAN BE, FE and packaged output; 45m 19s). - PASS: full ASAN UT binary build with `bash run-be-ut.sh -j 48`, default `GLIBC_COMPATIBILITY=ON`; no local CMake test-source selection. - PASS: `bash run-be-ut.sh --run --filter="BooleanPostingsBench.*:CluceneIndexSourceTest.*:CollectionStatisticsDetailedTest.*:CollectionStatisticsSniiScoringTest.*:CollectionStatisticsTest.*:CursorChainedPostings.*:DocSetCollectorTest.*:ExpansionLeafTest.*:FunctionSearchNullTest.*:FunctionSearchTest.*:IndexReaderHelperTest.*:ListedTermsTest.*:ListedTermsWalkTest.*:MultiSegmentCollectorTest.*:PhraseQueryV2Test.*:PhraseScorerStreamTest.*:PrefixQueryV2Test.*:SegmentPostingsTest.*" -j 48`; 369 passed, 3 existing disabled benchmarks, zero failures or sanitizer reports. Coverage includes postings/norms, collection statistics, MATCH eligibility, conjunctions, prefix/phrase queries, cursor chaining, and SEARCH execution/NULL handling. - PASS: `bash run-regression-test.sh --run -d search -s test_search_score_raw_index,test_search_score_keyword_filters -parallel 1` on an isolated ASAN FE/BE instance. Both suites passed with zero failed/fatal/skipped scripts. All 114 tags ran exactly once; source expected files are byte-identical. Coverage includes V2/V3/SNII, WAND on/off, whole keyword values, TopN/ascending order, internal AND, external equality/MATCH filters, and implicit/explicit `parser=none`. - PASS: complete merged-candidate format preflight for head `809eb7498a0fdfce905bdde7a350f3ba207947bd` against latest branch-4.2 `bcaaa5f293674f1adba0dfa52c8f8f57fb5e9549`; complete committed PR English check (17 source files, zero exceptions). - Builds and execution used target base `62cb1df3a7f8124ad338e8788e4faaf71f030b07`. The subsequent commit-message amendment preserved the exact tested code tree. The newer target commit changes JSON_SEARCH path escaping and has no changed-file overlap with this pick. - Modified-line clang-tidy 20.1.8 processed five production and six test translation units with the branch configuration and current compilation database. It emitted no changed-line diagnostics. The command exits with the pre-existing unmatched `NOLINTEND` in `be/src/core/types.h:578`. The same diagnostic was reproduced with all modified existing production files restored to target-base contents through a VFS overlay; the offending header is byte-identical to base `62cb1df3a7f8124ad338e8788e4faaf71f030b07`. No unrelated baseline fix is included. <details> <summary>Source hunk audit</summary> | File | Source hunk | Disposition | | --- | --- | --- | | `be/src/storage/index/index_reader_helper.cpp` | `@@ -0,0 +1,54 @@` | Adapted: use the target canonical analyzer-and-phrase policy; no master-only normalizer exception. | | `be/src/storage/index/index_reader_helper.h` | `@@ -103,36 +103,12 @@ public:` | Adapted: retain target compile-check wrappers and phrase-only MATCH eligibility. | | `be/src/storage/index/inverted/clucene/spi/clucene_postings_cursor.h` | `@@ -155,7 +155,7 @@ public:` | Adapted: preserve source unit norm and WAND bound behavior, adding missing norm-presence partition fields from the older target interface. | | `be/src/storage/index/inverted/clucene/spi/clucene_postings_cursor.h` | `@@ -305,6 +305,8 @@ public:` | Adapted: preserve source unit norm and WAND bound behavior, adding missing norm-presence partition fields from the older target interface. | | `be/src/storage/index/inverted/clucene/spi/clucene_postings_cursor.h` | `@@ -314,8 +316,8 @@ private:` | Adapted: preserve source unit norm and WAND bound behavior, adding missing norm-presence partition fields from the older target interface. | | `be/src/storage/index/inverted/query/engine/boolean_query/operator_boolean_weight.h` | `complete postimage` | Ported (verbatim) | | `be/src/storage/index/inverted/query/scoring/bm25_similarity.h` | `@@ -24,14 +24,8 @@` | Ported: source change retained with target-only surrounding context. | | `be/src/storage/index/inverted/reader/collection_statistics.cpp` | `@@ -44,9 +44,11 @@ namespace collection_statistics_detail {` | Adapted: add keyword/analyzed distinction; keep target analyzed-index metadata restrictions and omit unavailable normalizer-only exception. | | `be/src/storage/index/inverted/reader/collection_statistics.cpp` | `@@ -264,10 +266,11 @@ Status CollectionStatistics::process_segment(const RowsetSharedPtr& rowset,` | Adapted: add keyword/analyzed distinction; keep target analyzed-index metadata restrictions and omit unavailable normalizer-only exception. | | `be/src/storage/index/inverted/reader/collection_statistics.cpp` | `@@ -337,12 +340,11 @@ Status CollectionStatistics::process_segment(const RowsetSharedPtr& rowset,` | Adapted: add keyword/analyzed distinction; keep target analyzed-index metadata restrictions and omit unavailable normalizer-only exception. | | `be/src/storage/index/inverted/reader/collection_statistics.cpp` | `@@ -351,10 +353,8 @@ Status CollectionStatistics::process_segment(const RowsetSharedPtr& rowset,` | Adapted: add keyword/analyzed distinction; keep target analyzed-index metadata restrictions and omit unavailable normalizer-only exception. | | `be/src/storage/index/inverted/reader/collection_statistics.cpp` | `@@ -375,11 +375,12 @@ Status CollectionStatistics::process_segment(const RowsetSharedPtr& rowset,` | Adapted: add keyword/analyzed distinction; keep target analyzed-index metadata restrictions and omit unavailable normalizer-only exception. | | `be/src/storage/index/inverted/reader/collection_statistics.h` | `@@ -86,8 +86,9 @@ private:` | Adapted: target helper gains only is_analyzed; retain compile-check wrappers. | | `be/src/storage/index/inverted/reader/collection_statistics.h` | `@@ -121,11 +122,12 @@ struct SniiScoringSegmentStats {` | Adapted: target helper gains only is_analyzed; retain compile-check wrappers. | | `be/test/storage/index/inverted/query/engine/boolean_postings_bench_test.cpp` | `complete postimage` | Ported (verbatim) | | `be/test/storage/index/inverted/query/engine/boolean_query/listed_terms_test.cpp` | `complete postimage` | Ported (verbatim) | | `be/test/storage/index/inverted/query/engine/multi_segment_collector_test.cpp` | `complete postimage` | Ported (verbatim) | | `be/test/storage/index/inverted/query/engine/segment_postings_test.cpp` | `complete postimage` | Ported (verbatim) | | `be/test/storage/index/inverted/query/spi/fake_index_source.h` | `complete postimage` | Ported (verbatim) | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -667,6 +667,7 @@ protected:` | Adapted: keep keyword tests and target fixtures; omit signature update for the normalizer-only test that does not exist on 4.2. | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -676,8 +677,8 @@ protected:` | Adapted: keep keyword tests and target fixtures; omit signature update for the normalizer-only test that does not exist on 4.2. | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -975,26 +976,45 @@ TEST_F(CollectionStatisticsTest, LegacyV3RejectsSegmentWrittenWithoutNorms) {` | Adapted: keep keyword tests and target fixtures; omit signature update for the normalizer-only test that does not exist on 4.2. | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -1230,9 +1250,10 @@ TEST(CollectionStatisticsSniiScoringTest, ResolveRejectsSegmentWithoutPositions)` | N-A: this hunk only updates the pre-existing normalizer-only exception test; that path/test does not exist on branch-4.2. Keyword coverage is retained. | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -1939,7 +1960,9 @@ TEST_F(CollectionStatisticsTest, CollectSkipsIndexWithoutAnalyzer) {` | Adapted: keep keyword tests and target fixtures; omit signature update for the normalizer-only test that does not exist on 4.2. | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -2377,7 +2400,7 @@ TEST_F(CollectionStatisticsTest, SearchArrayStringSelectsFulltextLeafIndex) {` | Adapted: keep keyword tests and target fixtures; omit signature update for the normalizer-only test that does not exist on 4.2. | | `be/test/storage/index/inverted/reader/collection_statistics_test.cpp` | `@@ -2397,8 +2420,8 @@ TEST_F(CollectionStatisticsTest, SearchExactIgnoresAnalyzedBindingHint) {` | Adapted: keep keyword tests and target fixtures; omit signature update for the normalizer-only test that does not exist on 4.2. | | `be/test/storage/segment/index_reader_helper_test.cpp` | `@@ -22,6 +22,7 @@` | Ported: source change retained with target-only surrounding context. | | `be/test/storage/segment/index_reader_helper_test.cpp` | `@@ -380,6 +381,7 @@ TEST_F(IndexReaderHelperTest, IsNeedSimilarityScoreWithInvertedIndexQueryTypeTes` | Ported: source change retained with target-only surrounding context. | | `be/test/storage/segment/index_reader_helper_test.cpp` | `@@ -448,6 +450,7 @@ TEST_F(IndexReaderHelperTest, IsNeedSimilarityScoreWithTExprOpcodeTest) {` | Ported: source change retained with target-only surrounding context. | | `be/test/storage/segment/index_reader_helper_test.cpp` | `@@ -460,3 +463,51 @@ TEST_F(IndexReaderHelperTest, IsNeedSimilarityScoreWithTExprOpcodeTest) {` | Ported: source change retained with target-only surrounding context. | | `regression-test/data/search/test_search_score_keyword_filters.out` | `complete postimage` | Ported (verbatim) | | `regression-test/data/search/test_search_score_raw_index.out` | `complete postimage` | Ported (verbatim) | | `regression-test/suites/search/test_search_score_keyword_filters.groovy` | `complete postimage` | Ported (verbatim) | | `regression-test/suites/search/test_search_score_raw_index.groovy` | `complete postimage` | Ported (verbatim) | | `be/src/storage/index/inverted/clucene/spi/clucene_index_source.cpp` | `additional target adaptation` | Adapted: pass reader.hasNorms into the older postings partition so keyword unit norms work. Source parent already contains this plumbing. | | `be/test/core/data_type_serde/data_type_serde_arrow_test.cpp` | `@@ -957,9 +957,8 @@ TEST(DataTypeSerDeArrowTest, PlainArrowConverterDoesNotInferIcebergUuid) {` | N-A: historical PR-head compatibility edit, absent from the final landed squash. The target API still has check_duplicate_json_path. | | `be/test/core/data_type_serde/data_type_serde_arrow_test.cpp` | `@@ -1054,9 +1053,8 @@ TEST(DataTypeSerDeArrowTest, IcebergVariantExtensionAndParquetSchema) {` | N-A: historical PR-head compatibility edit, absent from the final landed squash. The target API still has check_duplicate_json_path. | | `be/test/core/data_type_serde/data_type_variant_v2_serde_jsonb_test.cpp` | `@@ -112,9 +112,8 @@ std::string json_at(const DataTypeVariantV2SerDe& serde, const IColumn& column,` | N-A: historical PR-head compatibility edit, absent from the final landed squash. The target API still has check_duplicate_json_path. | | `be/test/exprs/variant_v2_execution_test.cpp` | `@@ -49,9 +49,8 @@ TEST(VariantV2ExecutionTest, TypePropertiesDoNotAffectEquality) {` | N-A: historical PR-head compatibility edit, absent from the final landed squash. The target API still has check_duplicate_json_path. | </details> #### Range-diff review Compared the landed source commit against the target pick with `git range-diff 4016f207bfe^..4016f207bfe HEAD^..HEAD`. Representative differences: ```diff ## be/src/storage/index/index_reader_helper.cpp (new) ## @@ @@ be/src/storage/index/index_reader_helper.cpp (new) + // parser=none can inherit support_phrase=true from FE defaults. MATCH statistics skip + // these keyword indexes, so their filters must not request a scoring context either. + return inverted_index::InvertedIndexAnalyzer::should_analyzer(properties) && -+ (get_parser_phrase_support_string_from_properties(properties) == -+ INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES || -+ IndexReaderHelper::is_normalizer_only(properties)); ++ get_parser_phrase_support_string_from_properties(properties) == ++ INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES; +} + +} // namespace @@ be/src/storage/index/index_reader_helper.h: public: return iter->get_reader(InvertedIndexReaderType::STRING_TYPE) != nullptr; } -+ // MATCH filters score analyzed phrase indexes and single-term normalizers. ++ // MATCH filters score analyzed indexes with phrase support. static bool is_need_similarity_score(InvertedIndexQueryType query_type, - const TabletIndex* index_meta) { - if (query_type == InvertedIndexQueryType::MATCH_ANY_QUERY || + ## be/src/storage/index/inverted/clucene/spi/clucene_index_source.cpp ## +@@ be/src/storage/index/inverted/clucene/spi/clucene_index_source.cpp: ClucenePostingsCursor::Partition open_postings(lucene::index::IndexReader* reade + lucene::index::Term* term, bool positions, + bool scoring, const io::IOContext* io_ctx) { + ClucenePostingsCursor::Partition result; ++ result.has_norms = reader->hasNorms(term->field()); + if (positions) { + auto iter = make_term_positions_ptr(reader, scoring, io_ctx); + result.positions = iter.get(); ## be/src/storage/index/inverted/reader/collection_statistics.cpp ## @@ be/src/storage/index/inverted/reader/collection_statistics.cpp: namespace collection_statistics_detail { + Result<SniiScoringSegmentStats> resolve_snii_scoring_segment(uint64_t index_doc_count, uint64_t sum_total_term_freq, - bool has_positions, bool has_norms, -+ bool is_analyzed, - bool single_term_normalizer) { -- // Lowercase-normalized docs-only postings use unit frequency and length for scoring. -- if ((!has_positions || !has_norms) && !single_term_normalizer) { -+ // Keywords and lowercase-only normalizers can omit both sections: a matched value -+ // contributes one term with unit frequency and length. Other analyzed sources need both. -+ if (is_analyzed && !single_term_normalizer && (!has_positions || !has_norms)) { +- bool has_positions, bool has_norms) { +- if (!has_positions || !has_norms) { ++ bool has_positions, bool has_norms, ++ bool is_analyzed) { ++ // Keyword terms use unit frequency and length without positions or norms. ++ // Analyzed sources still require both sections. ++ if (is_analyzed && (!has_positions || !has_norms)) { return ResultError(Status::Error<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED, false>( "SNII scoring requires positions and norms; this segment was written without " "positions or without norms. Norms are left out when the index sets \"norms\" = " ``` The first and third groups omit the master-only normalizer exception and preserve branch-4.2's existing analyzed-index restrictions. The second group supplies the older target's missing norm-presence plumbing; the landed source parent already has it. Other differences are the corresponding helper signatures and tests, retained compile-check context, and backport commit metadata. The source hunk that updates the normalizer-only test is inapplicable because that feature and test do not exist on branch-4.2. The complete audit above accounts for all source hunks and the extra target plumbing file. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label -- 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]
