[fix](inverted index) Add a norms index property and a BE config to skip norms on variant paths - #68039
Open
eldenmoon wants to merge 6 commits into
Open
[fix](inverted index) Add a norms index property and a BE config to skip norms on variant paths#68039eldenmoon wants to merge 6 commits into
eldenmoon wants to merge 6 commits into
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#53980, apache#60722 Problem Summary: A VARIANT column with many sparse paths indexed by analyzed field_pattern inverted indexes can grow its index size by about 10x after compaction. Root cause: since apache#53980, InvertedIndexColumnWriter::create_field enables CLucene norms. CLucene's DocumentsWriter::writeNorms stores one byte per document and fills documents without a value with defaultNorm, so every .nrm file is exactly as long as the segment has rows, however sparse the column is. Each extracted variant subcolumn gets its own index, so a segment pays rows * indexed paths * analyzed indexes bytes of norms. apache#60722 removed norms only for non-tokenized indexes. Reproduction: 1M rows with 1000 typed sparse paths and 11 field_pattern indexes (3 analyzed), loaded on 3.1 and then fully compacted. - Compacted on 3.1: .idx 99.6 MiB, no .nrm. - Compacted after an in-place upgrade to 4.1: .idx 934.1 MiB (9.4x). 875 .nrm files of 1,000,012 bytes each account for 834.5 MiB; the rest is 99.3 MiB. Fix: analyzed indexes on variant subcolumns (non-empty index suffix) no longer write norms. The mutable BE config inverted_index_write_norms_for_variant_subcolumn (default false) restores them. Indexes on ordinary columns are unchanged. Without norms, BM25 scores used to become NaN: CLucene keeps a field's token count in the .nrm header, so sumTotalTermFreq() is empty, avgdl is 0, and BM25Similarity::compute_tf_cache divides 0 by 0. This already hit any norm-less segment. compute_tf_cache now skips length normalization when avgdl is 0, so scores stay finite and depend on term frequency and idf only. With 1M rows compacted on this build, the norms switch off shrinks .idx from 457.3 MiB (357.6 MiB of norms) to 99.6 MiB, and MATCH queries return the same rows. ### Release note Analyzed inverted indexes on variant subcolumns no longer write BM25 norms, which removes index bloat for variant columns with many sparse indexed paths. BM25 score() on variant subcolumns no longer applies document-length normalization; set BE config inverted_index_write_norms_for_variant_subcolumn=true to keep norms. ### Check List (For Author) - Test: - Unit Test: BM25SimilarityTest.* and InvertedIndexWriterTest.* (34 tests, including the new ZeroAvgDlScoresWithoutLengthNorm and NormsFileSkippedForVariantSubcolumn) - Regression test: inverted_index_p0 test_variant_subcolumn_index_norms (new), test_bm25_score_variant, test_omit_norms - Manual test: 3.1 -> 4.1 upgrade reproduction, and a norms on/off comparison with BM25 score checks on this build - Behavior changed: Yes. BM25 scores on variant subcolumn indexes no longer use length normalization by default. - Does this need documentation: No Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
eldenmoon
requested review from
airborne12,
csun5285,
gavinchou and
yiguolei
as code owners
September 15, 2026 21:57
Member
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
### What problem does this PR solve? Issue Number: None Related PR: apache#53980, apache#60722 Problem Summary: The previous commit skipped norms for variant subcolumn indexes behind a BE config. Elasticsearch controls norms per field instead: `norms` is a mapping parameter, true by default for text and false for keyword, and a dynamic template can turn it off for a whole family of dynamically mapped fields. A Doris variant path is the equivalent of a dynamically mapped field, so the switch belongs on the index rather than on the BE. This commit replaces the config with an inverted index property: - PROPERTIES("norms" = "true" | "false") on the index. - The default is false when the index is on a variant path: a field_pattern index, or the copy inherited by one extracted subcolumn, which carries the path as its index suffix. - The default is true otherwise, so analyzed indexes on ordinary columns keep their norms. FE accepts the key and rejects any value other than true or false. BE reads it in InvertedIndexColumnWriter::create_field through get_index_norms_from_properties, so the decision is made per index instead of per process, and one table can keep norms on the paths that need scoring while dropping them elsewhere. ### Release note Inverted indexes accept a new "norms" property. Analyzed indexes on variant paths no longer write BM25 norms by default; PROPERTIES("norms" = "true") keeps them for one index, and "norms" = "false" drops them for an ordinary column. ### Check List (For Author) - Test: - Unit Test: BE BM25SimilarityTest.* and InvertedIndexWriterTest.*, 34 tests passed, including NormsFollowIndexNormsProperty which covers a subcolumn index by default, a subcolumn index with norms = true, a field_pattern index by default, and an ordinary column with norms = false; FE InvertedIndexPropertiesTest, 51 tests passed, including the accepted and rejected property values - Regression test: inverted_index_p0 test_variant_subcolumn_index_norms now asserts norms per index suffix (ordinary column keeps them, s_* paths drop them, a t_* pattern index with norms = true keeps them); test_bm25_score_variant and test_omit_norms pass - Behavior changed: Yes, the BE config from the previous commit is gone and the default now lives on the index. - Does this need documentation: Yes, the inverted index property list needs the new key (doc PR to follow) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
eldenmoon
changed the title
Member
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
…s test
### What problem does this PR solve?
Problem Summary: The norms regression test only covered indexes declared with a
field_pattern. An inverted index built on a VARIANT column without a field_pattern
is copied to every extracted subcolumn, and that copy carries the properties of the
index it was copied from, so the `norms` property on the parent index decides the
behaviour of all of its subcolumn copies. Add two such indexes to the test, one
taking the default and one asking for norms back, so the inherited path is covered
as well:
norms by index suffix: [:true, v.s_host:false, v.s_note:false, v.t_note:true,
vd.a_host:false, vd.other:false, vn.b_host:true, vn.other:true]
### Release note
None
### Check List (For Author)
- Test: Regression test
- regression-test/suites/inverted_index_p0/test_variant_subcolumn_index_norms.groovy
- Behavior changed: No
- Does this need documentation: No
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
run buildall |
### What problem does this PR solve?
Problem Summary: The "norms" property did not mean the same thing on every index. An analyzed index
on a variant path defaulted to writing no norms while an analyzed index on any other column defaulted
to writing them, so the default came from the shape of the index and the property only corrected it.
That also changed how variant path indexes rank, for every user, as soon as the fix landed.
Give the property one meaning everywhere -- an analyzed index writes norms unless the property says
otherwise, on a variant path as on an ordinary column -- and move the variant skip into the new
mutable BE config inverted_index_skip_norms_for_variant, which is off by default. Turning it on drops
norms for every index on a variant path (an index declared with a field_pattern, or the copy that
each extracted subcolumn inherits from it) whatever that index's property says, so a cluster that
cares about the space can reclaim it without rewriting its index definitions, and a cluster that
cares about ranking keeps what it has.
Default behaviour therefore matches the state before this PR: norms are written everywhere.
### Release note
Added the BE config inverted_index_skip_norms_for_variant (off by default, mutable). Turning it on
drops BM25 norms for every inverted index on a VARIANT path, whatever the index's "norms" property
says.
### Check List (For Author)
- Test: Unit Test / Regression test
- be/test/storage/segment/inverted_index_writer_test.cpp: NormsFollowIndexNormsProperty covers
the default and the property in both directions on an ordinary column, a variant subcolumn and
a field_pattern index, and then the config dropping norms for variant path indexes (including
one that sets "norms" = "true") while leaving an ordinary column index alone
(InvertedIndexWriterTest.*: 18 passed)
- regression-test/suites/inverted_index_p0/test_variant_subcolumn_index_norms.groovy: the
default and "norms" = "false" on field_pattern and whole-column VARIANT indexes, plus a
setBeConfigTemporary case proving the config takes effect without a restart; the .out was
regenerated because variant paths now keep norms, so BM25 length normalization changes those
scores
- test_bm25_score_variant and test_omit_norms rerun on the same build
- Behavior changed: No (defaults unchanged; the variant skip is opt-in)
- Does this need documentation: Yes (apache/doris-website#4146)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
eldenmoon
changed the title
Member
Author
|
run buildall |
Member
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review summary
Request changes. The fresh V2/V3 writer path and the all-normless BM25 fallback are useful, but the patch still mis-ranks mixed normed/normless generations and silently accepts a norms policy that SNII ignores. The config-changing regression suite also needs isolation, and the config's nearest documentation states the opposite property precedence.
Critical checkpoint conclusions
- Goal and proof: Partially achieved for fresh CLucene V2/V3 writes and an entirely normless collection. Mixed generations and SNII prevent the feature from being correct end to end.
- Scope and clarity: The code is otherwise focused and reuses the existing index-property map and config machinery.
- Concurrency: No new production lock or shared-object lifecycle was introduced. The regression suite itself races other normal-pool suites because it mutates every BE without the required
nonConcurrentgroup (inline comment). - Lifecycle: Forward/reverse config transitions and rolling writers can mix norm-bearing and normless segments; collection-wide statistics then apply false length normalization (inline comment). The suspected direct-compaction gap was traced and dismissed because inherited VARIANT indexes retain the root VARIANT UID and fail the slice-type fast-compaction gate.
- Configuration: The mutable config is observed by subsequent CLucene field creation, but its comment contradicts the implemented and tested precedence over
norms=true(inline comment). - Compatibility and persistence: The default preserves existing behavior and the property persists through the existing generic metadata map, with no new FE-BE protocol or EditLog field. Rolling/config-transition compatibility is not correct because of mixed-generation scoring.
- Parallel paths: SNII fresh writes, raw rebuilds, and direct compaction ignore both controls even though FE accepts the property; SNII scoring also requires norms today (inline comment).
- Conditions and error handling: FE's lowercase boolean validation and BE's default-true parsing agree. The new
avgdl <= 0condition is too narrow for a mixed collection; no separate unchecked-Status or exception-boundary regression was found. - Tests and results: BE, FE, and ordered regression coverage was added, but it covers fresh/all-normless behavior only; mixed-generation and SNII cases are absent, and the global-config suite is not isolated. This was a static-only review: no local build or test was run under the review contract.
- Observability: No new metric or log is required for this local policy; the remaining semantic failures should be covered deterministically by tests.
- Transactions, data writes, and failure handling: No transaction/failover protocol changes were introduced, and index files retain the existing writer lifecycle. No distinct crash/leak/atomicity defect was found.
- FE-BE variable propagation: No new RPC field is needed because properties use the existing map; CLucene consumes it, while the missing SNII consumption is called out.
- Memory, nullability, and performance: No new ownership or nullable-column path was introduced. The cache fill is bounded by the existing table. The intended one-byte-per-row-per-path saving is valid; no separate hot-path regression was found.
- Other issues: The duplicated stale config comment and test preamble should be corrected together with the inline documentation finding.
No additional user focus was supplied. The review converged after round 2; the final sweep covered all 13 changed files and left no unresolved candidate.
| // CLucene keeps a field's token count in its .nrm header, so avgdl is 0 when no segment of the | ||
| // field stores norms (e.g. variant subcolumn indexes). Every document then has an unknown length: | ||
| // score without length normalization instead of computing 0 / 0. | ||
| if (_avgdl <= 0.0F) { |
Contributor
There was a problem hiding this comment.
[P1] Handle mixed normed and normless segments
This fallback runs only when the collection-wide avgdl is zero. During the supported transition, old segments still have norms while new ones omit them: collection statistics count documents from both but add zero tokens for the normless segment, so any old segment keeps avgdl > 0. Both scoring paths then feed a default 0/1 norm for the normless rows into this positive-avgdl cache, treating them as extremely short documents and boosting them over otherwise identical old rows. Please track missing norms and apply one consistent policy to the whole mixed collection (or per segment), and add a mixed-generation scoring test.
Contributor
FE UT Coverage ReportIncrement line coverage |
…s without norms
### What problem does this PR solve?
Problem Summary: The "norms" index property and inverted_index_skip_norms_for_variant only reached
the CLucene writer. SNII decided on its own: its writer set _writes_norms from the analyzer and the
positions flag, and its direct compaction from the analyzer alone, so on a SNII table both knobs
were silently ignored. SNII scoring also rejected any segment without norms, so it could not have
honoured them anyway.
- Move the rule into one function, should_write_index_norms(const TabletIndex&), and call it from
the CLucene writer, the SNII writer and SNII direct compaction. SNII direct compaction rebuilds
norms from the merged postings, so sources with and without norms merge correctly; the other SNII
compaction path goes through the SNII writer.
- Let SNII score a segment without norms. SNII keeps the token count in its stats block, so avgdl
stays correct. SniiStatsProvider::encoded_norm now returns std::nullopt for such a segment, and
ScorerContext::score scores that document as if its length were avgdl, the same length-neutral
form the CLucene BM25 fix uses. A document with a norm goes through the same arithmetic as
before. resolve_snii_scoring_segment no longer takes has_norms.
- Mark both norms regression suites nonConcurrent, since they flip a BE config.
### Release note
The "norms" inverted index property and the BE config inverted_index_skip_norms_for_variant now
also apply to the SNII index storage format, and SNII scoring no longer fails on a segment written
without norms.
### Check List (For Author)
- Test: Unit Test / Regression test
- SniiWriterNorms.WritesNormsFollowSharedNormsPolicy,
SniiCompactionEligibilityTest.DestinationWritesNormsFollowSharedNormsPolicy,
SniiScoringQuery.IndexWithoutNormsScoresWithoutLengthNormalization,
CollectionStatisticsTest.SniiScoringAdmitsSegmentWithoutNorms
- regression inverted_index_p0/storage_format/test_storage_format_snii_norms (new), plus
test_variant_subcolumn_index_norms, test_bm25_score_variant, test_omit_norms and
test_storage_format_snii
- Behavior changed: Yes. A SNII segment without norms is now scored without length normalization
instead of failing the query.
- Does this need documentation: Yes (apache/doris-website#4146)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
run buildall |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve?
Problem Summary: The previous commits of this PR let an analyzed index be written without norms
("norms" = "false", or inverted_index_skip_norms_for_variant on a variant path), and scored such
an index without length normalization. That left two problems:
- CLucene keeps a field's token count in its .nrm header. A segment without norms adds no tokens
but still adds documents, and its rows read the fake norm encodeNorm(0), i.e. length 0. When some
segments of a field have norms and others do not, as while the config is being turned on, avgdl
stays positive, the zero-avgdl fallback never runs, and the rows without norms rank as the
shortest possible documents, above otherwise identical rows.
- SNII was changed to score such segments too, which made the two formats and the transition
harder to reason about.
Follow what SNII already did: BM25 scoring needs norms from every segment it reads.
- CollectionStatistics now refuses an analyzed CLucene segment whose field has no norms with
INVERTED_INDEX_NOT_SUPPORTED, before any score is computed, so a collection with some or all
segments lacking norms fails the score() query instead of ranking inconsistently. An index that
is not analyzed never writes norms and is collected as before, so SEARCH scoring over keyword
fields is unchanged.
- The zero-avgdl fallback in BM25Similarity is dropped: with the refusal it can no longer be reached
by an analyzed index.
- SNII scoring is back to rejecting segments without norms (resolve_snii_scoring_segment,
SniiStatsProvider::encoded_norm, ScorerContext::score are as on master). Its message now names
the property and the config instead of suggesting that compaction will add the norms back.
- The writer side is unchanged: should_write_index_norms still decides for the CLucene writer, the
SNII writer and SNII direct compaction.
- Fix the config comment, which still said the property could override the config.
MATCH filtering is not affected; only queries that compute score() need norms.
### Release note
A query that computes score() on an analyzed inverted index now fails when a segment it reads was
written without norms ("norms" = "false", inverted_index_skip_norms_for_variant, or a segment
written before norms were supported), instead of returning NaN or inconsistent scores.
### Check List (For Author)
- Test: Unit Test / Regression test
- CollectionStatisticsTest.LegacyV3RejectsSegmentWrittenWithoutNorms (alone and mixed with a
segment that has norms), CollectionStatisticsTest.LegacyV3KeywordIndexWithoutNormsIsStillCollected
- regression test_variant_subcolumn_index_norms and test_storage_format_snii_norms: MATCH still
filters, score() fails on indexes without norms and on a mixed-generation table; plus
test_bm25_score_variant, test_omit_norms, test_search_score_topn_predicates,
test_search_score_cache, test_search_score_topn_delete_predicate and test_storage_format_snii
- Behavior changed: Yes. score() on an analyzed index with a segment that has no norms fails
instead of returning NaN or skewed scores.
- Does this need documentation: Yes (apache/doris-website#4146)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
run buildall |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 16884 ms |
Contributor
TPC-DS: Total hot run time: 82089 ms |
Contributor
ClickBench: Total hot run time: 14.74 s |
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?
Problem Summary:
An inverted index on a VARIANT column is copied to every extracted subcolumn, so a segment holds one
copy of it per path. Since #53980 every analyzed index calls
setOmitNorms(false), and CLucene writesnorms densely: 12 header bytes plus one byte per segment row for every indexed field, including
the rows that hold no value for that field. For a VARIANT column that cost is multiplied by the
number of paths.
On a real table (~1400 paths, 7 non-BKD indexes per path) a single base-compacted segment held 9891
.nrmfiles of 4,055,884 bytes each: 37.36 GiB of norms out of a 38.47 GiB.idx, 97.1%.Segments written before the upgrade had no
.nrmat all, and the non-norms bytes had not grown.Reproduced end to end on 1M rows / 1000 paths / 11
field_patternindexes, by loading with 3.1 andthen upgrading the same data in place to 4.1:
.nrm9.4x, and the norms are the whole difference. A/B on the same data: norms on 457.3 MiB vs norms
off 99.6 MiB (4.6x), with identical
MATCHrow counts and sums.What this PR changes
This PR gives operators a way to reclaim that space without changing what anyone gets by default,
and applies it the same way to the CLucene formats (V2/V3) and to SNII.
normsindex property. An analyzed index writes norms unless it sets"norms" = "false",on a VARIANT path exactly as on an ordinary column. A subcolumn copy inherits the properties of
the index it came from, so the property on a whole-column VARIANT index also settles every one of
its subcolumn copies.
inverted_index_skip_norms_for_variant, off by default. Turning it ondrops norms for every index on a VARIANT path (an index declared with a
field_pattern, or thecopy an extracted subcolumn inherits, which carries the path as its index suffix), whatever that
index's property says, so a cluster hit by the size regression can recover the space without
rewriting its index definitions.
should_write_index_norms(const TabletIndex&)holds the rule, and thethree places that decide whether norms get written all call it: the CLucene writer, the SNII
writer, and SNII direct compaction. SNII direct compaction rebuilds norms from the merged postings
rather than copying them; the other SNII compaction path rebuilds the index through the SNII
writer.
MATCHfiltering isunaffected; a query that computes
score()on an analyzed index fails withINVERTED_INDEX_NOT_SUPPORTEDwhen any segment of the field has no norms.resolve_snii_scoring_segment); its message now names theproperty and the config instead of suggesting that compaction will bring the norms back.
.nrmheader, and a row without normsreads the fake norm
encodeNorm(0), i.e. length 0. With no segment carrying norms,avgdlwas0 and
score()came out NaN. With only some segments carrying norms, as while the config isbeing turned on,
avgdlstayed positive and the rows without norms ranked as the shortestpossible documents, above otherwise identical rows.
CollectionStatisticsnow refuses such asegment before any score is computed. An index that is not analyzed never writes norms and is
collected as before, so
SEARCHscoring over keyword fields is unchanged.Compatibility
that has norms scores exactly as before.
"norms" = "false", is for indexes that are never ranked withscore(). For such an index,score()fails as soon as one segment without norms exists.score()on itsanalyzed indexes until compaction has rewritten the old segments, where it used to return NaN or
skewed scores.
Release note
Added the
normsinverted index property and the BE configinverted_index_skip_norms_for_variant(off by default, mutable), which drops BM25 norms for inverted indexes on VARIANT paths and can cut
the index size of a VARIANT column with many indexed paths by several times. Both apply to the V2,
V3 and SNII index storage formats. A query that computes
score()on an analyzed index now failswhen a segment it reads has no norms, instead of returning NaN or inconsistent scores.
Check List (For Author)
be/test/storage/segment/inverted_index_writer_test.cpp:NormsFollowIndexNormsPropertybe/test/storage/index/snii_writer_test.cpp:SniiWriterNorms.WritesNormsFollowSharedNormsPolicybe/test/storage/index/snii/compaction/snii_compaction_eligibility_test.cpp:DestinationWritesNormsFollowSharedNormsPolicybe/test/storage/index/inverted/similarity/collection_statistics_test.cpp:LegacyV3RejectsSegmentWrittenWithoutNorms(alone and next to a segment with norms),LegacyV3KeywordIndexWithoutNormsIsStillCollected, and the existing SNII rejection testsfe/fe-core/src/test/java/org/apache/doris/analysis/InvertedIndexPropertiesTest.javainverted_index_p0/test_variant_subcolumn_index_norms(checks the.nrmfiles)and
inverted_index_p0/storage_format/test_storage_format_snii_norms(reads norms off thescores):
MATCHstill filters,score()fails without norms, including on a mixed-generationtable. Both are nonConcurrent, since they flip the BE config. Also rerun:
test_bm25_score_variant,test_omit_norms,test_search_score_topn_predicates,test_search_score_cache,test_search_score_topn_delete_predicate,test_storage_format_sniiscore()on an analyzed index with a segment that has no normsnow fails instead of returning NaN or skewed scores.
🤖 Generated with Claude Code