Repository navigation
[fix](inverted index) Align phrase position defaults with analysis - #69199
Merged
airborne12 merged 5 commits intoOct 18, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
This was referenced Oct 16, 2026
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.
Static review of all four changed files and their FE DDL/persistence, CLucene/SNII writer, query/scoring, compaction, and SQL TOKENIZE consumers at ee2af95. Four P2 findings are reported inline: normalizer phrase predicates fail, normalizer scores silently become zero and affect score filters, TOKENIZE output changes, and existing BE unit fixtures retain incompatible expectations. No preexisting inline comments or additional user focus were supplied.
Critical checkpoint conclusions:
- Goal and proof: the local defaults align with tokenization and explicit overrides survive; added helper/constructor assertions match those local rules, but the inline findings expose missing downstream handling.
- Scope and reuse: the four-file edit is focused and reuses parser/alias getters; TOKENIZE needs a separate output-policy decision.
- Concurrency and locks: no new threads, shared mutations, lock acquisition, lock-order change, or expanded critical section; constructor properties are copied before mutation.
- Lifecycle and initialization: no new ownership, globals, cross-TU initialization dependencies, or changed analyzer/writer release order.
- Configuration: no new configuration or reload behavior.
- Compatibility: explicit stored phrase flags remain unchanged through Gson, Thrift, and PB conversion. Ordinary base-to-head creation/rolling-BE paths do not establish a separate legacy-omission bug. SQL output, normalizer phrase predicates, and normalizer scoring/result compatibility are the verified issues above.
- Thrift/protobuf field IDs: no definitions changed; cross-branch ID comparison is not applicable.
- Parallel paths: CLucene/SNII, scalar/array, cloud/shared-nothing, query/scoring, and compaction paths were checked; numeric BKD does not use phrase positions.
- Conditions: parser validation/canonicalization, alias priority, empty analyzer, explicit flags, and gram-family forced-false defaults were checked. Normalizer FULLTEXT phrase and scoring capability gates require the inline corrections.
- Test coverage: new FE/BE unit cases cover local property maps; end-to-end normalizer phrase/scoring and TOKENIZE compatibility cases are missing. Existing default-sensitive BE fixtures need correction.
- Test results: no goldens were changed; existing TOKENIZE goldens contradict the new behavior. No build, unit test, regression test, or formatter was executed, as required by the review contract; author test claims are not independent execution evidence.
- Observability: existing phrase errors and compaction eligibility diagnostics cover these local changes; no additional metrics or high-volume logs are required.
- Persistence and failover: existing property serialization/journal paths remain intact; no added transaction, visible-version, delete-bitmap, or failover transition.
- Writes and atomicity: existing writer/publication lifecycles are preserved; direct SNII compaction still validates physical source tiers/positions, with rebuild fallback.
- FE-BE variable passing: no new field; existing property-map converters preserve explicit values and omissions.
- Performance and memory: bounded property lookups, no new row-scale scan, large allocation, ownership risk, or toolchain/ABI dependency. Docs-only savings are intended, subject to the normalizer query finding.
- Other concerns: full changed scope reviewed; no additional user focus was supplied.
Review converged after three rounds: every reviewer returned NO_NEW_VALUABLE_FINDINGS in round 3 on this complete four-comment set, all changed scope was covered, and every candidate/risk has an evidence-based disposition. No unresolved static coverage remains. Remaining validation limit: static analysis only, with no executed runtime evidence.
Review converged after 3 round(s).
| || this.properties.containsKey(InvertedIndexProperties.INVERTED_INDEX_NORMALIZER_NAME_KEY)) { | ||
| if (!InvertedIndexProperties.INVERTED_INDEX_PARSER_NONE.equalsIgnoreCase(getInvertedIndexParser()) | ||
| || StringUtils.isNotEmpty(this.properties.get( | ||
| InvertedIndexProperties.INVERTED_INDEX_ANALYZER_NAME_KEY))) { |
Contributor
There was a problem hiding this comment.
[P2] Preserve phrase predicates on normalizer-only indexes
For PROPERTIES("normalizer"="lowercase") with omitted support_phrase, this removes the previously injected true. The BE still classifies that index as FULLTEXT because should_analyzer() includes normalizers, so a query such as v MATCH_PHRASE 'foo' over a stored FOO now fails with "phrase queries require setting support_phrase = true" in match.cpp / inverted_index_iterator.cpp; prefix/edge predicates are gated too. INDEX_INVALID_PARAMETERS is terminal rather than a scan fallback. A normalizer emits one keyword, and singleton phrase lowering already produces a term/expansion query that needs no positions. Please adapt those capability gates for docs-only normalizers and cover these predicates alongside the new default.
| || this.properties.containsKey(InvertedIndexProperties.INVERTED_INDEX_PARSER_KEY_ALIAS) | ||
| || this.properties.containsKey(InvertedIndexProperties.INVERTED_INDEX_ANALYZER_NAME_KEY) | ||
| || this.properties.containsKey(InvertedIndexProperties.INVERTED_INDEX_NORMALIZER_NAME_KEY)) { | ||
| if (!InvertedIndexProperties.INVERTED_INDEX_PARSER_NONE.equalsIgnoreCase(getInvertedIndexParser()) |
Contributor
There was a problem hiding this comment.
[P2] Preserve normalizer scoring when changing position defaults
Removing the default true also makes IndexReaderHelper::is_need_similarity_score() false for normalizer-only indexes. For a new lowercase-normalizer index containing FOO, SELECT id, score() AS s FROM t WHERE v MATCH 'foo' AND score()>0 ORDER BY s DESC LIMIT 10 previously returned a positive-scoring hit. Now MatchPredicateCollector skips the field and TextIndexReader disables scoring; CollectionSimilarity substitutes zero for missing entries, so the score filter silently removes that matching row. This query has no phrase predicate, so fixing the phrase gates does not address it. Preserve scoring independently from phrase-position storage, or keep the previous effective setting until this case is supported, and cover normalizer scoring/min-score queries.
| } | ||
| return INVERTED_INDEX_PARSER_PHRASE_SUPPORT_NO; | ||
| const auto analyzer = properties.find(INVERTED_INDEX_ANALYZER_NAME_KEY); | ||
| return get_parser_string_from_properties(properties) != INVERTED_INDEX_PARSER_NONE || |
Contributor
There was a problem hiding this comment.
[P2] Keep TOKENIZE's omitted-position default unchanged
FunctionTokenize::execute_impl also calls this helper (be/src/exprs/function/function_tokenize.cpp:247), and _do_tokenize uses the result to include "position" in every JSON object. An existing call such as TOKENIZE('alpha beta', '"parser"="english"') therefore changes from token-only output to output with positions, although this PR is scoped to index defaults. The unchanged test_tokenize and custom-analyzer regression goldens still expect token-only objects. Give TOKENIZE its own omitted=false behavior, preserving explicit true, so the physical-index default does not silently change this SQL output contract.
|
|
||
| properties[INVERTED_INDEX_PARSER_KEY] = INVERTED_INDEX_PARSER_ENGLISH; | ||
| EXPECT_EQ(get_parser_phrase_support_string_from_properties(properties), | ||
| INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES); |
Contributor
There was a problem hiding this comment.
[P2] Update existing BE fixtures for the new phrase default
The new helper expectation is correct, but SniiWriterTest.BuiltinAnalyzerNameStillInitialises (be/test/storage/index/inverted/snii/snii_writer_test.cpp:696-712) still passes only parser=english and expects kDocsOnly because support_phrase is absent. SniiIndexColumnWriter::init now selects kDocsPositions for that exact input, so the existing assertion fails. CollectionStatisticsTest.CollectSkipsIndexWithoutSimilarityScore likewise omits the flag and expects the old false scoring gate. Update the default-sensitive expectations, and set explicit support_phrase=false in fixtures intended to exercise disabled phrase/scoring behavior.
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
### What problem does this PR solve? Issue Number: CIR-22263 Related PR: apache#69192 Problem Summary: Inferring phrase positions from an omitted support_phrase property changes how older docs-only indexes are read and adds positions to TOKENIZE output. Newly created tokenized indexes already persist support_phrase=true in FE. Keep an absent property false when reading persisted metadata, while allowing normalizer-only single-term indexes to use phrase predicates and similarity scoring without stored positions. Update SHOW INDEX expectations for parser=none. ### Release note Existing indexes with an omitted support_phrase property retain their stored docs-only interpretation. ### Check List (For Author) - Test: Unit Test: focused FE IndexTest and two BE tests passed. - Behavior changed: Yes. Legacy omitted phrase metadata remains docs-only and normalizer-only queries remain available. - Does this need documentation: No
airborne12
added a commit
to airborne12/apache-doris
that referenced
this pull request
Oct 16, 2026
### What problem does this PR solve? Issue Number: CIR-22263 Related PR: apache#69199 Problem Summary: Inferring phrase positions from a parser when support_phrase is absent changes how older docs-only indexes are read and adds positions to TOKENIZE output. Newly created tokenized indexes already persist support_phrase=true in FE. Keep an absent property false when reading persisted metadata, while allowing normalizer-only single-term indexes to use phrase predicates and similarity scoring without stored positions. Update SHOW INDEX expectations for parser=none. ### Release note Existing indexes with an omitted support_phrase property retain their stored docs-only interpretation. ### Check List (For Author) - Test: Unit Test: focused FE IndexTest and two BE tests passed. - Behavior changed: Yes. Legacy omitted phrase metadata remains docs-only and normalizer-only queries remain available. - Does this need documentation: No
Contributor
TPC-H: Total hot run time: 29516 ms |
Contributor
TPC-DS: Total hot run time: 151788 ms |
Member
Author
|
run buildall |
Member
Author
|
/review |
Contributor
ClickBench: Total hot run time: 24.44 s |
Contributor
There was a problem hiding this comment.
Reviewed all six changed files at e145fed, including FE creation/persistence/transport, both BE posting writers and query paths, and affected tests. One new P2 performance regression is reported inline: permitted token-expanding custom normalizers lose SNII phrase-index acceleration when positions are omitted.
The existing normalizer-scoring concern still applies to SNII: the helper now enables scoring, but docs-only writers omit positions/norms and collection-statistics admission rejects them. This is already covered and is not reposted. The prior singleton-normalizer phrase gate is fixed on this head; mixed upgraded-FE/old-BE normalizer capability concerns remain covered by the existing phrase/scoring threads. The prior TOKENIZE and BE-fixture concerns no longer apply because the physical omitted-property default remains false. All existing findings are P2; there are no existing P0/P1 IDs to carry forward.
Critical checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and evidence | Exact-index defaults and legacy omission are handled locally; custom-normalizer position requirements and known SNII scoring remain problematic. Added tests establish metadata/helper choices only. |
| Scope and clarity | Changes are focused on defaults, capability selection and related tests. |
| Concurrency and locks | No new threads, shared mutations, locks or lock-order changes. |
| Lifecycle and initialization | No new resource ownership, release ordering or static initialization. |
| Configuration | No new process configuration; explicit existing index properties remain authoritative. |
| Compatibility | Legacy omission and persisted explicit values retain physical interpretation; mixed-version normalizer caveats are described above. |
| Thrift/protobuf IDs | No protocol definitions, IDs, enum values or required fields changed. |
| Parallel paths | CREATE TABLE/ADD/CREATE INDEX, local/cloud property transport, both scoring overloads and both posting writers were traced. |
| Conditions and comments | The new universal single-term normalizer assumption is false for allowed expanding filters; see inline finding. Explicit true/false handling is preserved. |
| Test coverage | FE/BE unit additions cover defaults and helper dispatch; end-to-end split-normalizer phrases/scoring and mixed-version queries are missing. No tests were executed. |
| Expected results | The two changed SHOW INDEX strings match sorted parser=none properties. No .out changes or additional stale metadata expectations were identified. |
| Observability | Existing status/fallback paths remain; no new distributed operation requiring new metrics. The inline regression can silently lose index acceleration. |
| Persistence and transactions | Existing Gson serialization and full property-map transport remain; no journal/transaction-ordering change. |
| Writes and crash behavior | Posting detail changes for newly created indexes; no new rowset, visibility, delete-bitmap, segment-ID, atomicity or crash-recovery mechanism. |
| FE-BE variables | No new field; generic map conversion preserves omitted versus explicit values. |
| Performance and memory | Bounded metadata lookups add no material allocation/hot-loop concern; the inline finding proves a selective phrase-to-row-scan regression. |
| Other module checks | No new heavy project-header edge, PCH/template/unity/gate change, ownership or error-propagation issue identified. No build-artifact ABI validation was performed. |
No additional user focus was supplied. Review was static only, as required: no builds, tests or source modifications. The inline finding is established by SNII control flow; no particular external CLucene runtime outcome is claimed. All changed files, risk focuses and candidates have explicit dispositions; no unresolved candidate remains.
Review converged after 2 round(s).
| static bool is_normalizer_only(const std::map<std::string, std::string>& properties) { | ||
| const auto normalizer = properties.find(INVERTED_INDEX_NORMALIZER_NAME_KEY); | ||
| const auto analyzer = properties.find(INVERTED_INDEX_ANALYZER_NAME_KEY); | ||
| // A normalizer keeps the whole value as one term, so phrase queries need no positions. |
Contributor
There was a problem hiding this comment.
[P2] Retain positions for token-expanding custom normalizers
A normalizer property does not guarantee one output term: CREATE INVERTED INDEX NORMALIZER split_norm PROPERTIES("token_filter"="word_delimiter") passes FE validation and turns foo-bar into foo and bar. With the new FE default, an SNII index using this normalizer stores no positions, but this predicate admits v MATCH_PHRASE 'foo-bar'. Lowering retains a two-slot phrase, SniiIndexSource::open_terms(..., positions=true, ...) returns NotSupported, and SegmentIterator falls back to row evaluation. Previously this query used the positional index; now selective phrase queries scan the data. Preserve positions for normalizers that can expand tokens, or enforce a single-token invariant before choosing docs-only storage, and cover this with a real postings test. This differs from the existing lowercase-normalizer thread: its singleton gate is fixed, while this permitted multi-token case needs physical positions.
Contributor
TPC-H: Total hot run time: 29240 ms |
Contributor
TPC-DS: Total hot run time: 152009 ms |
Contributor
ClickBench: Total hot run time: 24.84 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: None Related PR: apache#69199 Problem Summary: A custom normalizer can split one value into multiple terms, so omitting phrase positions by default can turn indexed phrase queries into scans. A built-in lowercase normalizer emits one term, but its docs-only postings have no frequencies or norms, which caused scoring statistics collection to reject the index. Keep positions for custom normalizers and use unit frequency and document length when scoring built-in lowercase postings without positions or norms. ### Release note Custom normalizers retain phrase positions by default. Built-in lowercase normalizer indexes keep phrase matching and positive relevance scores with docs-only postings. ### Check List (For Author) - Test: FE unit tests for normalizer defaults and policy binding; BE unit tests for SNII postings, collection statistics, reader capabilities, and missing norms; C++ format and build hygiene checks. - Behavior changed: Yes. Custom normalizers retain positions by default, and docs-only lowercase postings remain eligible for scoring. - Does this need documentation: No
Member
Author
|
run buildall |
Member
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review of all 14 changed paths at 4a10ba3 found no new substantiated defect. The main reviewer, FE reviewer, BE reviewer, and separate scoring-risk reviewer completed the full scope; all three subagents returned NO_NEW_VALUABLE_FINDINGS. The review converged after one round, with every candidate and initial risk given an evidence-backed disposition.
Reviewed flow: DDL validation → persisted Index properties → create/alter task transfer → physical SNII/CLucene writers → MATCH/SEARCH lowering → collection statistics/postings cursors → BM25 scoring and pruning.
Critical checkpoint conclusions:
- Goal and evidence: exact and builtin lowercase indexes can omit positions; parsers/analyzers and custom normalizers retain their appropriate defaults. The changed tests cover metadata defaults, policy shadows, physical SNII statistics, missing norms, and actual split-normalizer positions. This is static evidence; no runtime correctness claim is made.
- Scope: the changes are focused on defaults, capability admission, corresponding scoring support, and tests. No unrelated functional change was found.
- Concurrency and locking: statement-local maps and query-local statistics retain existing ownership; policy lookups use existing read locks. No new thread, shared state, lock ordering, or heavy operation under a new lock was introduced.
- Lifecycle and memory: no new significant allocation, resource owner, cache-handle lifetime, or cross-translation-unit initializer was introduced. The test policy-manager fixture restores the prior pointer using the adjacent pattern.
- Configuration: no new configuration item. Explicit support_phrase=true/false remains authoritative; existing norms and variant skip-norm settings reach the changed admission logic.
- Compatibility: stored explicit properties survive replay and cloning, and the physical omitted=false default remains unchanged. The supported BE-before-FE upgrade order preserves the old-FE/new-BE window. No persisted format definition changes.
- Thrift/protobuf identifiers: no definitions changed; cross-branch identifier comparison is not applicable.
- Parallel paths: CREATE TABLE, ADD/CREATE INDEX late-default write-back, MATCH/SEARCH singleton phrase lowering, both scoring-enum overloads, SNII/CLucene, variant inheritance, and TOKENIZE were traced. Direct and typed ARRAY normalizers are rejected; dynamic array inheritance removes the normalizer.
- Conditional checks and errors: policy-aware validation keeps custom lowercase shadows out of the absent-property shortcut; singleton lowering avoids physical position requests. Status/Result failures still propagate. No new silent invariant failure was found.
- Test coverage: all changed FE/BE tests and the metadata regression were inspected. Full scalar ranking and real CLucene end-to-end execution were not performed; the added tests do not independently prove those complete runtime flows. No builds, tests, source changes, or submodule initialization were performed in this review.
- Expected results: the SHOW INDEX expectation correctly removes only the unnecessary support_phrase default for parser=none. No .out/golden file changed.
- Observability: existing property display, unsupported errors, and collection-statistics logs cover the changed local paths; no missing critical new metric was identified.
- Transactions and persistence: existing Gson/journal/property-transfer paths preserve the metadata map. Transaction visibility, visible versions, delete bitmaps, and failover state machines are unchanged.
- Data writes and atomicity: physical position selection still follows stored metadata; no new transactional or crash-recovery branch was introduced.
- FE–BE variables: no new protocol variable. The shared converter transfers the same stored properties on create and alter.
- Performance: singleton queries avoid positional I/O; docs-only unknown block bounds use conservative BM25 saturation bounds. Pinned CLucene source confirms absent frequency buffers correctly select unit frequency. No new superlinear work or substantial memory-accounting issue was found.
- Other concerns and user focus: no additional user focus was supplied. No remaining unresolved candidate or uncovered changed path remains.
Existing review context was independently revalidated without reposting: 4294093294 (normalizer phrase admission), 4294093301 (normalizer scoring), 4294093310 (TOKENIZE output), 4294093318 (default-sensitive BE fixtures), and 4295209408 (token-expanding custom-normalizer positions) are addressed for their reported triggers at this head. All existing findings are P2; there are no existing P0/P1 findings to carry forward.
Remaining limits: this review is static. Physically normless CLucene uses the documented doc_count token-count estimate, so NULL/empty-heavy collections can score differently from SNII's physical token totals; numerical cross-format equality is not claimed for that newly admitted approximate path. Ordinary CLucene retains norms. This explicit approximation was not treated as a separate verified regression.
Review converged after 1 round(s).
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29315 ms |
Contributor
TPC-DS: Total hot run time: 151917 ms |
Contributor
ClickBench: Total hot run time: 24.78 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: apache#69199 Problem Summary: The missing-norm fallback treated a stored zero norm as an absent norm, changing existing CLucene query scores and failing BE unit tests. An empty norms span already identifies postings without stored norms. Preserve stored norm bytes while retaining the unit fallback for absent norms. ### Release note None ### Check List (For Author) - Test: Header hygiene and clang-format checks passed. Local BE UT compilation was blocked by a missing libpaimon_c.a in the third-party installation. - Behavior changed: Yes. Restore the prior interpretation of stored zero norms. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#69199 Problem Summary: CLucene supplies zero-filled norm arrays for fields that omit norms. The shared postings API must distinguish those placeholders from stored norm bytes. Pass the physical norm capability from each reader partition and leave the norm span empty when norms are absent, so scoring uses the unit fallback. Preserve stored norm bytes and update query expectations for no-norm fields. ### Release note Queries over CLucene indexes without norms use the unit norm for scoring. ### Check List (For Author) - Test: Header hygiene and clang-format checks passed. Local BE UT compilation was blocked by a missing libpaimon_c.a in the third-party installation. - Behavior changed: Yes. Scoring over CLucene indexes without norms uses the unit norm. - Does this need documentation: No
Member
Author
|
run buildall |
Member
Author
|
/review |
airborne12
added a commit
that referenced
this pull request
Oct 18, 2026
### What problem does this PR solve? Issue Number: CIR-22263 Problem Summary: SNII streamed compaction rejects compatible docs-only (T1) postings when the destination does not request phrase positions, then rebuilds those indexes from raw columns. This PR merges compatible T1 postings directly and retains raw rebuild for mixed or incompatible sources. Gram sources also retain raw rebuild because they can omit common postings and need their gram scheme preserved. Cloud compaction can assign nonzero physical destination segment IDs. The streamed SNII path now looks up its destination writer by physical ID, matching the existing CLucene path; looking it up by ordinal caused a BE crash in Cloud P0. This removes one repeated fallback seen in the CIR logs. The main T2 merge hotspot for tokenized indexes remains a separate performance question. Phrase-position defaults are handled in #69199. ### Release note Compatible docs-only SNII indexes use streamed compaction. ### Check List (For Author) - Test - [x] Unit Test: 17 SNII eligibility tests and the nonzero-segment-ID compaction test passed on the updated commit; 18 compaction-focused BE tests passed before the follow-up fix. - [x] Regression test: `test_storage_format_snii` passed on the combined source tree before this PR was split. - [x] Manual test: complete FE and BE builds and local health checks passed before the split. - Behavior changed: - [x] Yes. Compatible T1 postings are streamed; gram sources rebuild from raw columns. - Does this need documentation? - [x] No. The complete PR diff passes the English description check and clang-format 16 on the merged target tree. The updated commit is awaiting remote CI.
Contributor
There was a problem hiding this comment.
No new actionable findings for head 47b7eac4b28bc1e9cce14ea2fbd46da0fbb587d7. The review converged in 1 round: the FE reviewer, BE reviewer, and separate risk reviewer all returned NO_NEW_VALUABLE_FINDINGS against the same empty inline set. Main-agent verification covered all 17 changed paths (12 BE, 4 FE, 1 regression) and resolved every initial risk with evidence.
The reviewed flow was:
DDL validation / policy resolution -> persisted index properties
-> CLucene or SNII writer capabilities -> MATCH/SEARCH index selection
-> phrase lowering / collection statistics -> postings cursor -> BM25
Required checkpoint conclusions:
- Goal and evidence: FE retains positions for tokenized/custom-normalized indexes and defaults exact/builtin-lowercase indexes to docs-only. Singleton lowercase phrase/scoring capability follows the writer and reader contracts. Added FE defaults/shadow-policy tests, physical SNII writer/statistics tests, and CLucene cursor expectations support these paths; runtime behavior was not executed in this review.
- Scope and reuse: production changes are confined to default selection, shared capability checks, scoring admission and missing-norm signaling; existing policy-resolution and property-transport mechanisms are reused.
- Concurrency and locks: no new threads or lock order. FE mutates statement-local property maps using established policy-manager locking; the added BE flag is cursor-local. No new heavy operation under a shared lock was found.
- Lifecycle and initialization: iterator ownership and borrowed position-pointer lifetimes are preserved through partition moves. The test policy-manager fixture restores its previous pointer after writer destruction. No new static-initialization dependency or memory-owning production object is added.
- Configuration: no new runtime configuration. Explicit support_phrase values remain authoritative; existing norms and variant skip-norm policy still determine physical norms.
- Compatibility: existing explicit property values survive Gson read/write, replay, clone and wire conversion; BE still treats an omitted phrase flag as false for legacy postings and TOKENIZE. No new storage-format or symbol contract was found. The supported upgrade sequence installs BEs before FEs. Official upgrade order.
- Thrift/protobuf: no definitions, field IDs or enum values change.
- Parallel paths: CREATE TABLE, ADD INDEX and standalone CREATE INDEX propagation were checked; both phrase gates, both scoring overloads, CLucene and SNII paths, and MATCH/SEARCH collection consumers were traced.
- Conditions and errors: custom policies that shadow lowercase receive explicit positional capability. Singleton phrase/prefix/edge lowering requires no positions. ARRAY validation, VARIANT array property stripping and analyzer-key mismatch prevent the suspected multivalue bypass. No newly ignored error/status was found.
- Test coverage: positive/negative defaults, explicit overrides, mixed builtin spelling, legacy custom shadows, physical split-normalizer positions, docs-only scoring statistics and absent/stored-zero norms are covered by inspected tests. Full end-to-end normalized phrase/min-score execution remains unvalidated; no builds or tests were permitted.
- Test results: the changed parser=none metadata strings follow the constructor defaults. No-norm fixture score expectations correctly use encoded length 1; a real stored zero norm remains zero. No .out file changes are included.
- Observability: existing statistics logging and failure context remain available; no new distributed operation or critical metrics requirement was identified.
- Transactions and persistence: selected defaults are persisted through existing index metadata paths. Transaction visibility, visible versions and delete bitmaps are unaffected; no new journal/failover boundary is introduced.
- Data writes and crash atomicity: no production writer transaction/publication or atomic-finish path changes. No new crash leak or partially published state was identified.
- FE-BE propagation: no new transmitted variable; existing property maps carry the selected normalizer and phrase flags through both wire converters.
- Performance: policy work is limited to DDL; missing-norm capability is read per term/physical partition, not per document. No new large allocation or redundant row scan was found. Missing-norm block bounds remain conservative for unit document length.
- Additional-risk recheck: pinned CLucene dependency behavior was inspected read-only without initializing submodules. NULL/empty values, legacy names, VARIANT arrays, partition switches, absent frequency buffers and pruning were rechecked. Normless CLucene deliberately uses a document-count estimate for token totals; exact numerical parity with norm-bearing or SNII indexes is not established by this review. No unresolved actionable candidate or uncovered changed path remains.
All five existing P2 threads were independently rechecked and are addressed on this head; none is reposted:
- 4294093294: singleton lowercase phrase gates now admit the position-free lowering.
- 4294093301: lowercase MATCH scoring now passes both capability checks and collection admission.
- 4294093310: omitted phrase support remains false in the shared parser helper, preserving TOKENIZE output.
- 4294093318: that omitted=false behavior preserves the named existing BE fixture expectations.
- 4295209408: custom normalizers retain positions, including exact builtin-name shadows; the writer test verifies split terms and positions.
No existing P0/P1 blocker was found. The user focus file contained no additional focus points. This was a static-only review: no source modifications, builds or test runs. Convergence describes completed review coverage and candidate disposition; it does not claim runtime validation.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 29432 ms |
Contributor
TPC-DS: Total hot run time: 152778 ms |
Contributor
ClickBench: Total hot run time: 24.74 s |
yiguolei
pushed a commit
that referenced
this pull request
Oct 18, 2026
### What problem does this PR solve? Issue Number: CIR-22263 Related PR: #69199 Problem Summary: FE persists `support_phrase=true` for new tokenized indexes and leaves exact indexes docs-only. FE and BE retain the docs-only interpretation of an omitted property on older persisted analyzer indexes, preserving their postings and `TOKENIZE` output. Built-in `lowercase` normalization emits one term, so phrase predicates can use docs-only postings and BM25 can use unit frequency and document length. Custom normalizers can emit multiple terms, so new indexes retain positions by default, including when a legacy custom policy shadows the built-in name. This is the `branch-4.1` adaptation of #69199. It uses the branch's CLucene scoring path and the keyword-index scoring support already merged into `branch-4.1`. This branch has no SNII streamed compaction path from #69192, so this PR does not eliminate docs-only rebuilds or the tokenized T2 merge hotspot in CIR-22263. ### Release note New exact and built-in lowercase-normalized indexes default to docs-only postings. Custom normalizers retain phrase positions by default. Lowercase-normalized indexes retain phrase matching and relevance scoring without stored positions. ### Check List (For Author) - Test - [x] Unit Test: two targeted FE default and policy-binding tests and eight BE collection-statistics, query-scoring, postings, and reader tests on the current target branch. - [x] Complete PR diff passed the English description check, clang-format 16, and Checkstyle on the merged target tree. - Behavior changed: - [x] Yes. Exact indexes omit positions by default; custom normalizers retain positions; built-in lowercase normalization keeps phrase and scoring behavior with docs-only postings. - Does this need documentation? - [x] No. Remote checks on the updated head are pending.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
airborne12
added a commit
to airborne12/apache-doris
that referenced
this pull request
Oct 19, 2026
…69192) ### What problem does this PR solve? Issue Number: CIR-22263 Problem Summary: SNII streamed compaction rejects compatible docs-only (T1) postings when the destination does not request phrase positions, then rebuilds those indexes from raw columns. This PR merges compatible T1 postings directly and retains raw rebuild for mixed or incompatible sources. Gram sources also retain raw rebuild because they can omit common postings and need their gram scheme preserved. Cloud compaction can assign nonzero physical destination segment IDs. The streamed SNII path now looks up its destination writer by physical ID, matching the existing CLucene path; looking it up by ordinal caused a BE crash in Cloud P0. This removes one repeated fallback seen in the CIR logs. The main T2 merge hotspot for tokenized indexes remains a separate performance question. Phrase-position defaults are handled in apache#69199. ### Release note Compatible docs-only SNII indexes use streamed compaction. ### Check List (For Author) - Test - [x] Unit Test: 17 SNII eligibility tests and the nonzero-segment-ID compaction test passed on the updated commit; 18 compaction-focused BE tests passed before the follow-up fix. - [x] Regression test: `test_storage_format_snii` passed on the combined source tree before this PR was split. - [x] Manual test: complete FE and BE builds and local health checks passed before the split. - Behavior changed: - [x] Yes. Compatible T1 postings are streamed; gram sources rebuild from raw columns. - Does this need documentation? - [x] No. The complete PR diff passes the English description check and clang-format 16 on the merged target tree. The updated commit is awaiting remote CI. (cherry picked from commit 8db0540)
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
3 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: CIR-22263
Related PR: #69192
Problem Summary: FE persists
support_phrase=truefor new tokenized indexes and leaves exact indexes docs-only. BE must treat an omitted property as false because older persisted analyzer indexes can omit it despite having docs-only postings; this also preservesTOKENIZEoutput. Built-inlowercasenormalization emits one term, so phrase predicates can use docs-only postings and BM25 can use unit frequency and document length. Custom normalizers can emit multiple terms, so new indexes retain positions by default, including when a legacy custom policy shadows the built-in name.This PR changes phrase-position defaults and normalizer compatibility. PR #69192 adds direct SNII compaction of compatible docs-only postings. Neither change addresses the tokenized T2 merge hotspot identified in CIR-22263.
Release note
New exact and built-in lowercase-normalized indexes default to docs-only postings. Custom normalizers retain phrase positions by default. Lowercase-normalized indexes retain phrase matching and relevance scoring without stored positions.
Check List (For Author)
Remote Clang Formatter and CheckStyle passed on the updated head. Automated code review is pending.