airborne12 opened a new pull request, #67537:
URL: https://github.com/apache/doris/pull/67537
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
`CommonGrams` (the token filter, its word set, the term-key escaping codec,
the per-segment metadata, the hybrid docs-only/positioned query planner and its
result-cache gate, the SPIMI pair-key fast path, 17 profile counters, 4 BE
switches, the FE DDL validation and the `score()` admission rule) was an
experiment that never reached a released build, yet it had grown roots through
the whole SNII stack and had diverged the v1 on-disk layout from what the
released 3.1-based writer produces. This PR removes it end-to-end and realigns
the physical format so that a node running this code can read segments written
by the released writer, and vice versa, without a format version bump.
What changes:
- **Removed**: everything CommonGrams-specific in BE
(`be/src/storage/index/inverted/common_grams/`, the `common_grams` token
filter, the SNII term-key escaping codec, `SniiCommonGramsMetadataPB`, the
hybrid query planner and its cost model, the SPIMI pair-key path, the
frequency-less spill-run shape) and in FE (the `common_grams` token filter
type, the CommonGrams policy validation, the `score()` admission rule). Loading
an existing policy of that type marks it invalid with an error log instead of
crashing FE.
- **Term keys** are now the raw analyzed bytes everywhere. User terms that
overlap the internal `\x1F` phrase-bigram namespace bypass the index (the same
rule the released writer already implied).
- **Scoring (BM25) is kept** and decoupled from CommonGrams: a segment is
scorable iff it has positions *and* norms. Tokenized indexes with positions now
always write norms (an optional region referenced from the core metadata; no
new `index_config`). `tf` is the number of positions the term has in the
document. Older segments without norms answer `NOT_SUPPORTED` for scoring only;
a normal compaction reconstructs norms from the postings (byte-identical to a
rebuild), so they become scorable after one compaction. The WAND / block-max
paths are removed together with the per-window max statistics they depended on.
- **On-disk layout (S1)**: `kFormatVersion` stays 1. The dict entry / dict
block / frq prelude / frq pod encoders and decoders are byte-for-byte the
released layout again: prelude flag bit0 = `has_prx`, no freq regions, no
per-window `max_freq` / `max_norm`, no per-entry term statistics, no
`frq_docs_len` / `inline_dd_disk_len` / `freq_meta`, no `kNoTermStats`
dict-block flag. `sum_total_term_freq` and the norms reference live in optional
proto fields the released reader ignores. Segments carrying the experimental
CommonGrams metadata fields (or the experimental scoring `index_config`) are
rejected as `NOT_SUPPORTED` with a rebuild hint.
- Two follow-up fixes that fell out of the layout change: the phrase-query
"position work" estimate now parses prx frame headers without verifying CRCs
(`read_prx_frame(..., verify_crc=false)`), so a phrase query no longer
checksums every candidate prx window twice; and the frequency-less spill-run
record shape is gone (the production SPIMI path always materializes
frequencies).
Compatibility evidence (real binaries, golden corpus of 8 sample indexes /
179 query expectations / 513 checks across cache-off, cache-cold and
cache-warm):
- this branch reading segments written by the released 3.1-based writer: 8/8
open, 513/0
- segments written by this branch, checked against the expectations recorded
before the removal: 513/0 (query semantics unchanged)
- the released 3.1-based binary reading segments written by this branch: 8/8
open, 513/0 (rollback-safe: the old reader ignores the norms field)
Size: 178 files, +2321 / −20981 (be/src −8184, be/test −10066, fe −946).
### Release note
The experimental `common_grams` token filter and its scoring tier are
removed. BM25 scoring on a tokenized SNII index now requires the norms region,
which the writer always emits for tokenized indexes with positions; segments
written before this change become scorable after one compaction.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
Full BE UT after the change: 13158 passed, 1 failed
(`TableReaderTest.CountStarFallbackKeepsLateRuntimeFilterCarrierValues`, fails
identically on the base commit: a shared `/tmp` permission issue). Compared
name-by-name with the base commit's run (13448 tests): 274 tests removed or
renamed by this PR, 61 environment-gated tests skipped in both runs, 48 tests
added or renamed. Targeted SNII suites: 247 suites / 1828 tests green. FE:
fe-core 10442 tests, 3 failures + 5 errors — all eight reproduce identically
(same tests, same per-class counts) when the same seven test classes run on the
base commit in a clean worktree, so they are pre-existing: four are the test
environment's mocked backend not coming up ("available backend num is 0"), one
is a JSON cast in an HTTP response test, two are nereids rewrite-rule plan
pattern mismatches, one is a cloud-mode assertion in `PropertyAnalyzerTest`.
None of those test files or the code they exercise is touched by this PR.
Manual: the cr
oss-read matrix above, run with the golden-corpus test (`SNII_GOLDEN_DIR` /
`SNII_GOLDEN_MODE`).
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change --> `common_grams` is no
longer a valid token filter type; scoring requires norms (see release note);
segments written with the experimental CommonGrams metadata or scoring
`index_config` are rejected as `NOT_SUPPORTED`; one analyzer instance now
serves indexing and every query type.
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 --> The `common_grams` token
filter documentation needs to be removed (doc PR to follow).
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01YbPyc5mrQswEfVZXWi7nyJ
--
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]
