[fix](be) Decouple SNII BM25 scoring from CommonGrams - #67134
Open
airborne12 wants to merge 1 commit into
Open
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#66052 Problem Summary: `score()` over an ordinary analyzed SNII index failed with "SNII semantic scoring metadata is missing". V1/V2/V3 rank the same index (regression test_bm25_score.groovy), so SNII was the outlier. CommonGrams stores gram tokens in its physical postings, so its SniiStatsPB.sum_total_term_freq and per-document length are not the numbers BM25 wants; it needs a SEMANTIC view of the collection. That view was introduced inside the CommonGrams segment metadata, and every downstream capability check was then written as "does this segment carry CommonGrams metadata" rather than "does this segment carry scoring data". A phrase-query performance optimization therefore became a prerequisite for a core feature, in five separate places: the writer's tier decision, the writer's norms accumulation, the query-side statistics gate, the per-segment stats provider, and compaction eligibility. For an index that does NOT use CommonGrams there is no such divergence -- its physical statistics ARE the semantic ones -- so the fix is to ask the right question instead of adding a second mechanism. No metadata field and no proto message is added. Writer: an index reaches the scoring tier and persists per-document norms when it is ANALYZED and keeps POSITIONS (`_writes_norms()`), not when it uses CommonGrams. ARRAY columns are covered too; CommonGrams rejects ARRAY outright, so an array text column could previously never be ranked on SNII. The plain analyzer lane also never counted its tokens -- `*semantic_length` was only incremented in the CommonGrams branch -- which is now done in `consume_token`. Reader: `SniiStatsProvider::open()` and `resolve_snii_scoring_segment()` share one predicate, "the index persists the BM25 inputs" = scoring tier + positions + norms. A CommonGrams segment still has its semantic view validated before use. Two DORIS_CHECKs that would have aborted the BE on the plain shape are removed, and the term-df bound now uses the physical document count (proved equal to scoring_doc_count for CommonGrams by validate_snii_scoring_metadata). The analyzer fingerprint is a CommonGrams identity, so it is only compared when the segment records one -- V1/V2/V3 perform no such check. `avgdl` now divides by doc_count on both shapes. Norms are written for every row (a null row contributes encode_norm(0)), so the denominator must span the same rows the lengths do; the CommonGrams branch already used every row. Compaction: a third streamed-merge kind, kPlainT3 -- scoring tier with norms and no CommonGrams metadata. The existing norms remap is reused; only the metadata seed and the semantic token count stay CommonGrams-only. Segments written before this change keep working for filtering and are refused for scoring rather than ranked at a guessed document length: they carry neither norms nor freq regions, and a silent unit-length fallback would rank them on a different scale from their siblings in the same table. Ranking them requires rebuilding the index. SIZE IMPACT, measured by section on a harmonic-df corpus (1k/20k/200k docs): an analyzed index grows 30-38% against current master, roughly two thirds freq region and one third norms. freq is a required BM25 input -- V1/V2/V3 and the shipped SelectDB branch have always written it, and master dropped it for plain positions indexes (G16-c) on the premise that such an index never scores, which is the very defect fixed here. Measured against those two baselines the net addition is norms alone, about +7% at 200k documents. The DICT region -- read on every term lookup -- grows 13.2% at 1k docs, 8.0% at 20k and 0.0% at 200k, as frequent postings move from inline to windowed and take their freq bytes out of the dictionary block. The two SniiWriterGoldenBytes digests for analyzed lanes were RE-HARVESTED. The image changed for four reasons, not one: the freq region, the tier-dependent dict entry layout, the norms region, and index_config. kGoldenKeywordDocsOnly is unchanged, pinning that the analyzed lane is the only one affected. ### Release note Fix score() on SNII inverted indexes built with an ordinary analyzer: BM25 ranking no longer requires a CommonGrams analyzer. Indexes written before this change must be rebuilt to be ranked. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - SniiPlainIndexScoring.* (4 cases: scalar, ARRAY, null runs, no-positions guard) - CollectionStatisticsTest.SniiPlainAnalyzedIndexCollectsScoringStatistics enters through CollectionStatistics::collect and was verified RED against unmodified production code with the reported error text. - Full inverted-index suites: 3212 tests, 0 failures - ./run-be-ut.sh -j 160 --run - Behavior changed: - [ ] No. - [x] Yes. An analyzed SNII index with phrase positions is now written at the scoring tier with norms and freq, and can be ranked. Segments written before this change are refused for scoring instead of erroring later. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
airborne12
requested review from
csun5285,
eldenmoon,
gavinchou,
luwei16 and
yiguolei
as code owners
August 25, 2026 15:03
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
16 tasks
Contributor
TPC-H: Total hot run time: 16987 ms |
Contributor
TPC-DS: Total hot run time: 82379 ms |
Contributor
ClickBench: Total hot run time: 14.68 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #66052
Problem Summary:
score()over an ordinary analyzed SNII index failed with"SNII semantic scoring metadata is missing". V1/V2/V3 rank the same index
(regression test_bm25_score.groovy), so SNII was the outlier.
CommonGrams stores gram tokens in its physical postings, so its
SniiStatsPB.sum_total_term_freq and per-document length are not the numbers
BM25 wants; it needs a SEMANTIC view of the collection. That view was introduced
inside the CommonGrams segment metadata, and every downstream capability check
was then written as "does this segment carry CommonGrams metadata" rather than
"does this segment carry scoring data". A phrase-query performance optimization
therefore became a prerequisite for a core feature, in five separate places:
the writer's tier decision, the writer's norms accumulation, the query-side
statistics gate, the per-segment stats provider, and compaction eligibility.
For an index that does NOT use CommonGrams there is no such divergence -- its
physical statistics ARE the semantic ones -- so the fix is to ask the right
question instead of adding a second mechanism. No metadata field and no proto
message is added.
Writer: an index reaches the scoring tier and persists per-document norms when
it is ANALYZED and keeps POSITIONS (
_writes_norms()), not when it usesCommonGrams. ARRAY columns are covered too; CommonGrams rejects ARRAY outright,
so an array text column could previously never be ranked on SNII. The plain
analyzer lane also never counted its tokens --
*semantic_lengthwas onlyincremented in the CommonGrams branch -- which is now done in
consume_token.Reader:
SniiStatsProvider::open()andresolve_snii_scoring_segment()shareone predicate, "the index persists the BM25 inputs" = scoring tier + positions +
norms. A CommonGrams segment still has its semantic view validated before use.
Two DORIS_CHECKs that would have aborted the BE on the plain shape are removed,
and the term-df bound now uses the physical document count (proved equal to
scoring_doc_count for CommonGrams by validate_snii_scoring_metadata). The
analyzer fingerprint is a CommonGrams identity, so it is only compared when the
segment records one -- V1/V2/V3 perform no such check.
avgdlnow divides by doc_count on both shapes. Norms are written for everyrow (a null row contributes encode_norm(0)), so the denominator must span the
same rows the lengths do; the CommonGrams branch already used every row.
Compaction: a third streamed-merge kind, kPlainT3 -- scoring tier with norms and
no CommonGrams metadata. The existing norms remap is reused; only the metadata
seed and the semantic token count stay CommonGrams-only.
Segments written before this change keep working for filtering and are refused
for scoring rather than ranked at a guessed document length: they carry neither
norms nor freq regions, and a silent unit-length fallback would rank them on a
different scale from their siblings in the same table. Ranking them requires
rebuilding the index.
SIZE IMPACT, measured by section on a harmonic-df corpus (1k/20k/200k docs): an
analyzed index grows 30-38% against current master, roughly two thirds freq
region and one third norms. freq is a required BM25 input -- V1/V2/V3 and the
shipped SelectDB branch have always written it, and master dropped it for plain
positions indexes (G16-c) on the premise that such an index never scores, which
is the very defect fixed here. Measured against those two baselines the net
addition is norms alone, about +7% at 200k documents. The DICT region -- read on
every term lookup -- grows 13.2% at 1k docs, 8.0% at 20k and 0.0% at 200k, as
frequent postings move from inline to windowed and take their freq bytes out of
the dictionary block.
The two SniiWriterGoldenBytes digests for analyzed lanes were RE-HARVESTED. The
image changed for four reasons, not one: the freq region, the tier-dependent
dict entry layout, the norms region, and index_config. kGoldenKeywordDocsOnly is
unchanged, pinning that the analyzed lane is the only one affected.
Release note
Fix score() on SNII inverted indexes built with an ordinary analyzer: BM25
ranking no longer requires a CommonGrams analyzer. Indexes written before this
change must be rebuilt to be ranked.
Check List (For Author)
Test
enters through CollectionStatistics::collect and was verified RED against
unmodified production code with the reported error text.
Behavior changed:
scoring tier with norms and freq, and can be ranked. Segments written
before this change are refused for scoring instead of erroring later.
Does this need documentation?
Check List (For Reviewer who merge this PR)