Repository navigation
[fix](be) Support BM25 scoring on keyword SEARCH indexes - #69051
Merged
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
airborne12
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.
Request changes for one P1: the shared SNII scoring gate now enables scoring for ordinary indexed keyword equality/MATCH filters, although their field statistics are not collected. A score() query with matching keyword filter rows can therefore fail with Not such column. The inline comment gives a concrete supported query and the complete caller/collector mismatch. No existing inline findings need to be carried forward.
Reviewed all 16 changed files at f22f161. The review converged in 2 rounds: both complete reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS in the latest round, and every candidate/risk has an explicit disposition. No additional user focus was supplied.
The relevant execution flow is:
score runtime -> shared index context -> every index iterator
SEARCH -> collected stats -> private native score sink -> scored bitmap -> outer collector
keyword equality/MATCH -> shared SNII scorer -> missing keyword stats -> P1 failure
Checkpoint conclusions:
- Goal and evidence: intended keyword SEARCH scoring is implemented across CLucene V2/V3 and SNII, with unit/regression fixtures, but the ordinary-predicate regression requires correction before merge.
- Scope/reuse: changes are focused on existing BM25, postings, statistics and native decoding. Reusing the eligibility predicate needs a narrower caller boundary, as described inline.
- Concurrency/locking: no new threads, locks or changed lock order. Scored native calls bypass shared bitmap caching/single-flight; no additional concurrency defect found.
- Lifecycle/static initialization: existing cache handles and local ownership retain readers through decoding; private score maps move into query-owned immutable storage. No new owner cycle or cross-translation-unit initialization dependency found.
- Configuration: no new configuration; the existing WAND option has true/false regression pairs.
- Compatibility: no storage writer or FE/BE protocol change. Legacy keyword indexes use unit defaults for omitted metadata; analyzed indexes still reject missing scoring metadata. No new libc API is introduced; final binary ABI was not audited in this static review.
- Parallel paths: traced V2/V3/SNII, docs-only/positioned, term/phrase, inline/slim/windowed, candidate/exhaustive, cache, scalar/array/null and compound/TopN/ascending paths. Ordinary equality/MATCH eligibility is the one verified mismatch.
- Special conditions/error handling: position/norm guards match persisted capabilities and preserve decode-error propagation. Pinned CLucene source confirms absent frequency pointers and negative missing-block-stat sentinels, making the unit-frequency and conservative unit-norm WAND fallback sound.
- Test coverage: added fixtures cover scalar keyword case/whitespace, null/empty/missing values, multiple segments, dual indexes and rankings. Missing SNII score()+ordinary keyword equality/MATCH coverage explains the P1. The new docs-only unit uses df 1/10/501, below the 512 windowed threshold; docs-only windowed, positional keyword arrays, large real CLucene skip blocks and mixed historical segments have static path evidence without new execution coverage.
- Golden outputs: a temporary static audit checked all 54 golden hit sets and BM25 values, plus all 24 WAND on/off pairs. Native physical-token statistics explain its different scores from legacy average-length defaults. Golden generation and author-reported executions were not independently verified.
- Observability: existing field/segment errors, collection-statistics VLOGs and query timers cover these branches; no additional metric requirement identified.
- Transactions/persistence: no edit-log, commit/publish, failover, version-map or persistent metadata mutation; existing visibility and delete-bitmap filtering are retained.
- Data writes/atomicity: no production writer changes. Writer inspection established the reader's keyword, array, null and frequency invariants; no new atomicity/crash path found.
- FE/BE variables: none added. Existing regression query shapes establish that SEARCH/score with ordinary indexed filters is supported.
- Performance/memory: docs-only decoding skips absent PRX/norm reads and preserves candidate-window selection. Existing buffers/cache ownership are reused; no additional verified allocation, complexity or redundant-scan defect found.
- Other checks: no new hub-header dependency, hygiene-table/PCH change, explicit-instantiation family or src-.cpp test inclusion. No second independent finding; equality/MATCH and related uncollected-clause observations are one root cause, deduplicated into the single inline.
Validation was static only, as required: no builds, Doris tests, source edits or submodule initialization. All changed checkout blobs match the reviewed head. The listed runtime test gaps are validation limits; no unresolved correctness candidate or unreviewed changed-file scope remains.
Review converged after 2 round(s).
| const bool actual_similarity = | ||
| context->collection_similarity && | ||
| IndexReaderHelper::is_need_similarity_score(query_type, &_index_meta); | ||
| context->collection_similarity && IndexReaderHelper::is_need_snii_similarity_score( |
Contributor
There was a problem hiding this comment.
[P1] Keep uncollected keyword filters out of the scoring path
_query() is also used by ordinary SQL filters, and SegmentIterator::_init_index_iterators() attaches the same non-null collection_similarity to every iterator in a score query. On an SNII table with an analyzed body index and a keyword tag index, SELECT id, score() AS s FROM t WHERE SEARCH('body:apple') AND tag = 'x' ORDER BY s DESC LIMIT 10 now sends the tag's EQUAL_QUERY through score_plain_term_candidates(). However, CollectionStatistics::extract_collect_info() only collects MATCH/SEARCH leaves, so no statistics exist for the equality filter's tag field: get_or_calculate_avg_dl() throws Index statistics collection failed: Not such column ..., failing a query that previously worked. Keyword MATCH predicates have the same mismatch because MatchPredicateCollector explicitly skips non-analyzed indexes. Please distinguish SEARCH leaf scoring from ordinary filters, and keep reader eligibility consistent with the statistics collected for each caller; add an SNII regression combining SEARCH/score() with an indexed keyword equality or MATCH filter.
Contributor
TPC-H: Total hot run time: 29865 ms |
Contributor
TPC-DS: Total hot run time: 153423 ms |
Contributor
ClickBench: Total hot run time: 24.77 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
4 of 5 tasks
Problem Summary: Resolve conflicts with the unified MATCH and SEARCH index execution refactor. Preserve this PR's scoring behavior through the new query and reader interfaces. Related PR: apache#68758 Release note: Preserve search scoring fixes after index execution unification. Tests: ASAN unit validation in progress; header hygiene and clang-format 16 passed. Behavior changed: No additional behavior beyond the existing scoring fix. Documentation: Not required; no new syntax or settings.
Problem Summary: The general postings mock inherits unsupported multi-reader block-bound methods. Use a leaf mock to validate keyword norm and WAND defaults across partitions. Tests: affected ASAN suites; production code unchanged. Release note: none. Behavior changed: no. Documentation: not required.
Member
Author
|
run buildall |
Member
Author
|
/review |
Contributor
There was a problem hiding this comment.
Reviewed all nine changed files at 46e15ab, including the SEARCH collector/lowering, both index formats, postings, BM25 and WAND consumers. No new distinct inline findings were confirmed. Changes are requested because an existing P1 remains applicable.
Existing finding disposition: 4278499777 is partially addressed. Ordinary keyword equality now stays out of scoring. However, a keyword index configured with support_phrase=true still lets an ordinary MATCH predicate score, while MatchPredicateCollector skips that non-analyzed index. For example, with an analyzed body index and keyword tag index carrying that property, SELECT id, score() AS s FROM t WHERE SEARCH('body:apple') AND tag MATCH_ANY 'x' ORDER BY s DESC LIMIT 10 collects only body statistics, then the tag reader calls BM25Similarity::for_one_term and fails in get_or_calculate_avg_dl(tag) with a missing-column error. FE property validation permits this configuration. This is the keyword MATCH concern already stated in the existing thread, so it is retained without another inline comment. Default keyword indexes without support_phrase=true do not take this path.
Critical checkpoint conclusions:
- Goal and evidence: keyword SEARCH now admits optional metadata and exposes unit norms consistently; analyzed missing-metadata restrictions remain. Unit and regression cases support the intended behavior, but were not executed during this review. The existing mixed SEARCH/MATCH failure remains above.
- Scope and reuse: focused changes reuse current format adapters, statistics admission and scoring machinery; no unrelated implementation change found.
- Concurrency and locks: no threads, locks or lock-order changes. Statistics are collected during scanner preparation before reader open; new state is a cursor-local scalar or classification parameter.
- Lifecycle, memory and initialization: iterator/cache-handle ownership is preserved, the partition flag follows the owned leaf iterator, and no new allocating container, owner cycle or cross-TU static is introduced.
- Configuration: no configuration item added.
- Compatibility: no persisted format, public protocol or rolling-upgrade representation changes. Existing keyword metadata omissions are interpreted at read time.
- Thrift/protobuf: untouched.
- Parallel paths: CLucene and SNII, scored/unscored access, single/partitioned readers, term/phrase lowering and scalar/WAND consumers were traced. Keyword PHRASE lowers to a raw term. Norm one is used for rows and block bounds; unknown frequency keeps the saturation upper bound.
- Conditional checks and comments: analyzed-only rejection and keyword defaults have concrete metadata-omission explanations. No speculative error recovery was added; existing error propagation is preserved.
- Test coverage: new cases cover TERM/PHRASE/EXACT, multiple inserts, NULL/empty input, case sensitivity, missing terms, OR TopN, ascending order, WAND on/off and dual indexes. Existing analyzed rejection tests remain. The untested extensions are listed below.
- Changed results: all 54 golden labels and result IDs were checked statically. The distinct BM25 values match the implementation: CLucene uses avgdl=1; SNII keeps ten physical tokens across eleven rows, including one NULL, giving avgdl=10/11. Dual-index exact scoring also matches. Output ordering is deterministic for these inputs; generation provenance was not independently established.
- Observability: existing collection logs and failure diagnostics remain suitable; no new distributed operation or metric requirement identified.
- Transactions and persistence: query-only change; version visibility, delete bitmaps, edit-log replay and failover state are unaffected by the modified code.
- Writes and atomicity: no production write path changed. Existing SNII segment staging/commit and failure cleanup remain intact.
- FE/BE variable passing: no new transmitted variable or send path.
- Performance: constant-size per-leaf norm metadata and per-field analyzer classification add no extra posting scan or large allocation; no material new performance defect identified.
- Additional risks: collector/index selection, metadata defaults, partition/shallow-seek invalidation, conservative pruning and writer token conventions were checked. Every investigated candidate has a verified duplicate or dismissal disposition; no unresolved concrete candidate remains.
User focus: no additional focus was supplied; the complete PR scope was reviewed.
Coverage limits: this was static inspection only, with no builds, tests or live SQL replay. Author-reported test runs were not treated as independently executed evidence. The external CLucene decoder implementation is absent from this checkout, so its unchanged internals could not be re-audited. The added tests do not directly force large multi-block WAND, heterogeneous norm partitions with shallow seeks, keyword arrays, ignore_above, or positional keyword MATCH. Repository adapter/writer contracts were inspected for these cases; these limits did not establish a distinct new bug.
All three assigned reviewers returned NO_NEW_VALUABLE_FINDINGS in round 1; the full changed-file scope and all initial risk questions have explicit dispositions. Review convergence does not clear the independently confirmed existing P1.
Review converged after 1 round(s).
Existing P0/P1 findings confirmed for this head: #69051 (comment)
Contributor
TPC-H: Total hot run time: 29766 ms |
Contributor
TPC-DS: Total hot run time: 152347 ms |
Contributor
ClickBench: Total hot run time: 24.75 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#69051 Problem Summary: Physical CLucene fixtures omit norms and now correctly read unit document length, but their Boolean and multi-segment expectations still assumed zero. Update those expectations, including benchmark checksums. Add V2/V3/SNII SQL coverage for SEARCH scores combined with keyword equality and MATCH filters, with WAND enabled and disabled. The current unified reader and predicate collector already share scoring eligibility; the new regression verifies that ordinary keyword filters neither request uncollected statistics nor add scores. Pin the SEARCH execution prerequisites under CI session fuzzing. ### Release note None. This updates test expectations and regression coverage. ### Check List (For Author) - Test: 324 ASAN unit tests, changed-line clang-tidy and header hygiene passed. Full ASAN FE/BE build passed. Generated and replayed the 24-query keyword filter suite; all filters match baseline IDs and scores. Replayed test_search_score_raw_index successfully. - Regression test / Unit Test - Behavior changed: No production behavior change - Does this need documentation: No
Contributor
There was a problem hiding this comment.
Changes requested at reviewed head 34616deab2616a225ffa110bd7547ffda473325f: one new P1 for scored SNII SEARCH conjunctions, and existing P1 4278499777 remains applicable. All 13 changed files were reviewed against the supplied aggregate diff, including upstream/downstream execution, unit tests and both regression goldens. This was static review only; no builds or tests were run and no repository source was edited.
The new finding is an internal SEARCH AND requesting positions from a docs-only keyword source after its statistics are admitted. The existing finding is different: ordinary keyword MATCH can request statistics the collector skipped. Equality and default keyword MATCH are fixed, but INDEX idx_tag(tag) USING INVERTED PROPERTIES("parser"="none") gets support_phrase=true from FE catalog defaults. WHERE SEARCH('body:apple') AND tag MATCH_ANY 'x' with score()/TopN then passes the reader's phrase-support scoring gate while MatchPredicateCollector skips the non-analyzed tag. BM25 requests the missing tag statistics. Keep the existing P1 as blocking; it is not reposted.
Concrete execution reviewed:
Scanner preparation: SEARCH predicates -> collection statistics
Scan execution: SEARCH lowering/bindings -> weights -> source -> postings -> scorer -> TopN
SNII scored AND -> positions (new P1)
Critical checkpoint conclusions:
- Goal and proof: scalar keyword TERM/PHRASE/EXACT/OR scoring is supported by the inspected implementation and supplied tests; the scored SNII AND path remains incomplete.
- Scope and reuse: focused admission/default-norm changes reuse PostingsBlock and the existing BM25 formula.
- Concurrency and locks: statistics are created per scanner during preparation; new norm state belongs to each cursor. No new thread, shared mutation, lock or lock-order change.
- Lifecycle and static initialization: iterator ownership, borrowed positions and next-block span validity remain intact. No new static/global initializer or ownership cycle.
- Configuration: no new configuration item; tests use existing execution and WAND variables.
- Compatibility: V2/V3 share CLucene handling; SNII retains physical statistics. No persisted format change, new libc API or FE/BE protocol change. Historical analyzed segments missing required metadata remain explicitly rejected. No release ELF was built or audited here.
- Thrift/protobuf: no changed definitions or field IDs.
- Parallel paths: ordinary/seek partition transitions, WAND, term/phrase/Boolean and format-specific paths were traced. New P1 identifies the missed positional conjunction assumption; existing P1 covers the residual filter eligibility mismatch.
- Conditional checks: analyzer classification matches writers and the norm fallback is documented; analyzed missing metadata still produces an error.
- Test coverage: inspected no-norm and partition unit cases, physical fixture updates, collection admission/negative cases, and regressions across V2/V3/SNII with WAND on/off, multiple inserts, NULL/empty/case values, absent terms, dual-index EXACT and TopN. Missing internal SEARCH AND and parser=none keyword MATCH variants correspond to the two blockers. ARRAY and mixed-norm/shallow-seek behavior were inspected statically without new dedicated regression coverage.
- Changed results: all distinct golden scores and row memberships were independently checked against BM25 math, including legacy avgdl=1 versus SNII physical avgdl=10/11 and the analyzed filter fixture. Ordering is deterministic for the supplied limits. Golden generation provenance and author-reported runtime results were not independently verified.
- Observability: existing segment/field errors, timing and collection VLOG remain adequate for these local paths; no new mandatory metric identified.
- Transactions and persistence: no changed transaction, EditLog, failover or persistence behavior.
- Data mutation and crashes: no production write path changed; visible-rowset selection, physical segment identity, delete bitmap handling and failure clearing remain unchanged.
- FE/BE variables: none added; no serialization/send sites require changes.
- Performance and memory: one norm-capability check per scoring term opening and one boolean per partition; no new per-row allocation, scan or large buffer. Existing ownership/accounting is unchanged.
- Other concerns and user focus: no additional user focus was supplied. Remaining code concerns are the new inline P1 and the independently confirmed existing P1 above; no unresolved candidate or unreviewed changed-file scope remains. Execution and artifact validation remain outside this static review's evidence.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #69051 (comment)
| bool has_positions, bool has_norms, | ||
| bool is_analyzed) { | ||
| // A keyword posting has implicit frequency and length one, without either section. | ||
| if (is_analyzed && (!has_positions || !has_norms)) { |
Contributor
There was a problem hiding this comment.
[P1] Support docs-only keywords in scored SNII conjunctions
This now admits keyword SNII indexes without positions, but the scored AND path still requires them. On the SNII schema from the new keyword-filter regression, SELECT id, score() AS s FROM t WHERE SEARCH('body:apple AND tag:x', '{"mode":"standard"}') ORDER BY s DESC LIMIT 10 reaches OperatorBooleanWeight::listed_terms. SNII sources always batch reads, including a single tag term, so ListedTerms::open(true, true) calls open_terms(..., positions=true, scoring=false); SniiIndexSource rejects the keyword source with snii: the index holds no positions. Support implicit unit frequencies in this conjunction path or use the individual term scorers, and add an internal SEARCH AND regression with WAND on/off. The new SQL AND filter cases do not exercise this path.
### What problem does this PR solve? Issue Number: None Related PR: apache#69051 Problem Summary: A docs-only SNII keyword term in a scored SEARCH conjunction entered a batched path that requires positions. Keep it on the existing term scorer with implicit unit frequency. Separately, parser=none may inherit support_phrase=true; ordinary keyword MATCH filters then requested statistics that collection skipped. Share the analyzer-and-phrase eligibility rule between both query-type overloads, keeping heavy analyzer dependencies out of the header. ### Release note Scored SEARCH conjunctions support docs-only keyword indexes. Keyword MATCH filters do not request uncollected statistics even with inherited phrase support. ### Check List (For Author) - Test: 365 targeted ASAN BE unit tests, complete ASAN BE build, 60 keyword SQL output checks and 54 raw-index SQL checks passed. Both reported scenarios reproduce on the previous binary. Output is generated with forceGenOut and replayed; the original 24 output blocks remain unchanged. - Behavior changed: Yes, as described above. - Does this need documentation: No.
yiguolei
pushed a commit
that referenced
this pull request
Oct 16, 2026
…69051 (#69159) ### What problem does this PR solve? Related PR: #69051 Problem Summary: Backport #69051 to `branch-4.1`. Legacy keyword SEARCH indexes lack norms and total-term-frequency metadata. Scoring could use a zero average length or zero norm and return invalid scores. Preserve whole keyword values, use unit document length when metadata is absent, and keep WAND upper bounds conservative. Numeric BKD scoring remains unsupported. #### Branch adaptations Backport the final net change from f22f161. Keep the branch-4.1 collection-statistics and predicate-collector paths and TermInfo representation. Select the same reader for plain-field statistics and execution when keyword and analyzed indexes coexist; retain the branch-specific variant binding path. The branch has no SNII, so this PR includes V2/V3 coverage and omits SNII-only changes. ### Release note SEARCH on legacy keyword indexes can return valid BM25 scores. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [ ] Manual test - Behavior changed: - [x] Yes. SEARCH on legacy keyword indexes can return valid BM25 scores. - Does this need documentation? - [x] No. ### Validation - PASS: Clang 16 across the complete candidate merged with `origin/branch-4.1` (`0993d5e5512b3d27649e6c44f38b13836e03ee97`). - PASS: English code check for the complete PR diff. - PASS: 69 targeted ASAN BE unit tests from 5 suites on this backport. - PASS: ASAN BE/FE build with `./build.sh --be --fe -j48`. - PASS: `test_search_score_raw_index` and `test_search_score_cache`, each replayed on this backport with one suite executed and zero failures, fatal scripts, or skips. - PASS: the BE binary requires GLIBC no newer than 2.10 (baseline 2.17). - PASS: the two backport commits merge together without conflicts. Validation completed on the committed backport head. The temporary test instances are shut down after the run. The unit run selected relevant test sources and existing repository helpers in a temporary local CMake manifest. The original manifest has been restored and is not included in this PR.
Contributor
There was a problem hiding this comment.
Reviewed all 19 changed files at 1c146c172aaf4c308563b9aa0abe2943b6d086e0, including upstream collectors/readers/writers and downstream scoring/TopN paths. No new actionable findings.
Both previously reported P1 mechanisms are addressed at this head:
- 4278499777: the shared reader eligibility gate keeps equality and non-analyzed keyword MATCH filters out of scoring, consistent with collected statistics, including explicit
parser=nonewith inherited phrase support. - 4295241438: scored AND leaves docs-only sources on individual term scorers instead of requesting positions through ListedTerms. Their postings supply unit frequency/norm.
Neither remains an existing blocker; neither is reposted. No additional user review focus was supplied.
The reviewed execution chain is:
SEARCH/MATCH predicates -> physical-index/term statistics -> BM25 similarity
-> CLucene/SNII source -> streamed postings or ListedTerms -> score collection/TopN/WAND
Critical checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and proof | Keyword SEARCH receives usable scoring defaults; unit/regression cases cover intended behavior. This invocation provides static evidence only. |
| Scope and clarity | Changes are focused on eligibility, norms, statistics and conjunction selection; existing analyzer predicates and term scorers are reused. |
| Concurrency and locks | No new thread, shared mutable state or lock-order edge. The new norm flag is local to its cursor/partition. |
| Lifecycle and static initialization | Reader/iterator ownership and cache-handle lifetimes remain intact; no new global initializer or cross-TU dependency. |
| Configuration | No configuration item added. |
| Compatibility and rolling upgrades | No storage/wire format change. Legacy keyword metadata gets defaults; analyzed missing-metadata rejection remains explicit. Release ELF/glibc compatibility was not executed/audited here. |
| Thrift/protobuf IDs | No definition changes; branch comparison is inapplicable. |
| Parallel paths | Checked V2/V3/SNII, SQL filters versus SEARCH leaves, streamed/batched execution, AND/OR/NOT and occurrence booleans, phrase/exact lowering, partitions and WAND. |
| Conditional branches | Comments explain the concrete omitted-norm and missing-position scenarios; canonical analyzer classification matches writers and collectors. |
| Functional coverage | New suites cover three formats, WAND on/off, internal AND, external equality/MATCH filters, TERM/PHRASE/EXACT/OR, case/whitespace, NULL/empty scalar values, absent terms, multiple rowsets, TopN directions and dual indexes. |
| Negative/unit coverage | Inspected analyzed missing-position/norm errors, numeric BKD rejection, index-binding conflicts, cleanup, unit defaults, partition switches, and docs-only conjunction tests. |
| Changed results | All 60 keyword-filter and 54 raw-index golden blocks match static row-set/BM25 arithmetic. WAND pairs agree; SQL keyword filters retain baseline scores. Generation provenance was not independently verified. |
| Observability | Existing query timers, statistics VLOG and error propagation cover these read-side branches; no new telemetry requirement identified. |
| Transactions and persistence | No transaction, EditLog, failover or persisted-metadata path changes. Read versions and delete-bitmap mutation are unaffected. |
| Data writes and crash behavior | No new write path or ownership edge requiring recovery handling. |
| FE/BE variable passing | No new transmitted variable or send path. |
| Performance and memory | New production state is a cursor/partition boolean and stack capability data; SNII capability probes are constant time. No significant new allocation or asymptotic regression identified. |
| Other risks | The exact pinned CLucene source confirms docs-only max frequency stays unknown (-1), preserving the conservative WAND saturation bound. No additional verified issue remains. |
Review convergence: converged after 1 round. Both complete-scope reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS; all initial risks have evidence-backed dispositions and the final sweep found no unresolved candidate or unreviewed changed file.
Validation limits: static-only as required; no builds, unit/regression execution, sanitizers, runtime pruning checks, format/hygiene executables or final release-artifact audit. The added SQL suites do not exercise arrays/repeated keyword elements, ignore_above, wholly NULL raw fields or mixed-age analyzed metadata collections; relevant source paths and existing negative tests were inspected. Author-reported runs were not treated as executed evidence from this review.
Review converged after 1 round(s).
Member
Author
|
run buildall |
Member
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29549 ms |
Contributor
There was a problem hiding this comment.
Reviewed all 21 changed paths at head 9b356cb594f60b802de117ff9b5915b60500d424. No new actionable findings. Three independent reviewers returned NO_NEW_VALUABLE_FINDINGS; the static review converged after one round.
Both existing P1 concerns are addressed at this head:
- 4278499777: shared MATCH eligibility excludes external keyword equality/MATCH filters from scoring while SEARCH retains its separate keyword statistics and scorer path.
- 4295241438: docs-only SNII AND terms retain individual scorers with
positions=false; positional sources retain batching. Both original triggers have direct regressions. Neither remains in the blocking-comment set.
Critical checkpoint conclusions:
- Goal and evidence: keyword SEARCH uses compatible scoring defaults while preserving raw values, case and whitespace; source tracing and targeted tests support the intended behavior.
- Scope and reuse: production changes are focused and reuse existing eligibility, statistics and term scorers. Three variant fixture edits remove an option absent from the current encoder API.
- Concurrency and locks: no new threads, shared state, locks or lock-order changes.
- Lifecycle and initialization: cursor/source ownership and block validity remain coherent; no new static initialization dependencies or cleanup hazards.
- Configuration: none added; dynamic reload is not applicable.
- Compatibility: reader-side defaults support existing keyword indexes without changing disk/wire formats or dependencies; analyzed missing-metadata and numeric BKD restrictions remain. The new translation unit is included by existing source discovery.
- Thrift/protobuf: unchanged; field-ID comparison is not applicable.
- Parallel paths: checked both helper overloads, MATCH/SEARCH, CLucene V2/V3 and SNII, normalizers, positional/docs-only postings, AND/OR/NOT, partition/shallow-seek transitions and WAND. Unit norm bounds agree with scored postings; unavailable frequency bounds remain conservative.
- Conditions and errors: comments explain inherited phrase metadata and missing positions; failures propagate through the existing Status/exception boundary.
- Test coverage: added unit and regression cases cover explicit/implicit keyword metadata, internal conjunctions, external filters, exact/phrase/term matching, TopN, ascending scores, case, missing values and WAND on/off. Existing analyzed-metadata negative cases remain.
- Expected results: all 114 output markers are accounted for; every distinct BM25 result was independently recomputed. Ordering and bounded ties are deterministic.
- Observability: existing errors, collection logging and SEARCH timers remain sufficient for this local change.
- Transactions and persistence: no EditLog, version, recovery or failover changes.
- Data writes and atomicity: no production write-path changes.
- FE/BE variables: none added or transmitted.
- Performance and memory: the capability probe reads SNII metadata; existing streaming fallback adds no new data-sized allocation or lock work.
- Other core invariants: no new visibility, delete-bitmap, nullable-column or ignored-status issue identified. No additional user focus was supplied.
Validation limits: this was static review only, as required; no builds or tests were run. Runtime success, output-generation provenance and final ELF compatibility were not independently verified. Larger postings, array/deletion combinations and additional nesting were not executed. No unresolved static candidate or assigned coverage gap remains.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 29326 ms |
Contributor
TPC-DS: Total hot run time: 152370 ms |
Contributor
ClickBench: Total hot run time: 24.61 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
csun5285
approved these changes
Oct 20, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
4 of 12 tasks
yiguolei
pushed a commit
that referenced
this pull request
Oct 21, 2026
…69051 (#69318) ### What problem does this PR solve? Issue Number: None Related PR: #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 4016f20^..4016f20 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 was referenced Oct 29, 2026
airborne12
added a commit
to airborne12/apache-doris
that referenced
this pull request
Oct 29, 2026
…oks up ### What problem does this PR solve? Issue Number: None Related PR: apache#69051 Problem Summary: the previous commit collects BM25 statistics for MATCH on a keyword index. Two inputs made that collection disagree with the reader, and the score() query failed where it used to run with a score of 0: 1. Phrase slop. A keyword MATCH_PHRASE reads a trailing " ~N" or " ~N+" as its slop: lower_match strips it, so tag MATCH_PHRASE 'BP ~1' looks up the term BP. The collector stored "BP ~1", and the scorer then failed with "Index statistics collection failed: Not such term BP". The collector now strips the slop with the same take_slop, as the no-index MATCH_PHRASE path in match.cpp already does. 2. A number cast to text. With enable_match_without_inverted_index (on by default), body MATCH 'apple' AND CAST(n AS STRING) MATCH '1' runs the second predicate as a scalar filter. The collector looks through the cast to n, selects its BKD index, and, because a BKD index has no analyzer, the keyword rule admitted it; collecting statistics then opened the BKD index as a text index and the query failed with "segment: logical index is not a text inverted index". Only a text index is read with a score, so a MATCH on a non-string leaf now collects nothing. ### Release note None ### Check List (For Author) - Test: Regression test / Unit Test - Unit Test: CollectionStatisticsTest (new KeywordPhraseCollectsTermWithoutSlop and MatchOnNumericColumnCollectsNothing), IndexReaderHelperTest - Regression test: test_match_keyword_score gains phrase slop and CAST(n AS STRING) MATCH cases on V2/V3/SNII (.out generated, only new blocks added); test_omit_norms, test_match_phrase_prefix_score, test_bm25_score, test_search_score_keyword_filters and test_search_score_raw_index pass - Mutation check: with the collector change reverted, both new unit tests fail, the suite fails with "Not such term BP", and the CAST(n AS STRING) MATCH query fails with the error above - Behavior changed: No (fixes the previous commit before release) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
airborne12
added a commit
that referenced
this pull request
Oct 30, 2026
…sticsearch (#69815) ### What problem does this PR solve? Issue Number: None Related PR: #69052 Problem Summary: score() of pattern queries disagrees with Elasticsearch and with itself: -- 'Inverted indexes are suitable for full text search', -- 'The parser splits text into multiple terms', ... SELECT score() FROM document WHERE SEARCH('content:/.*ext/'); -- BM25 sum SELECT score() FROM document WHERE SEARCH('content:*ext'); -- BM25 sum SELECT score() FROM document WHERE SEARCH('content:te*'); -- 1 SELECT score() FROM document WHERE content MATCH_REGEXP 'ext'; -- 0 Elasticsearch scores a query_string /regexp/, wildcard or prefix, and a regexp, wildcard or prefix query, with a constant 1 per matching row (default rewrite constant_score_blended); measured on 9.5.5 with the rows above: 1.0 for both matching rows. Only an explicit "rewrite": "scoring_boolean" sums the BM25 of the expanded terms. Next to a scored clause in a bool must, a regexp adds exactly 1. #69052 made SEARCH regexp and wildcard sum the BM25 of every expanded term, i.e. scoring_boolean always on, while prefix stayed constant. That sum also takes idf and avgdl from the current segment, not from the tablet statistics MATCH uses: two inserts of the same text scored 0.2876821 and 1.3862942 for identical 'apple' rows, while MATCH 'apple' scored 0.6931471 for both. MATCH_REGEXP never requested a scoring context, so it scored 0 like a filter. Fix: 1. Revert the scoring part of #69052: ExpandWeight scores every pattern with the constant scorer again. The test fixture extensions that #69051 built on are kept. 2. MATCH_REGEXP requests scoring on every index; the regexp expansion then publishes the constant 1. The MATCH statistics collector skips it, since a constant reads no statistics and the pattern is not text to analyze.
github-actions Bot
pushed a commit
that referenced
this pull request
Nov 1, 2026
…sticsearch (#69815) ### What problem does this PR solve? Issue Number: None Related PR: #69052 Problem Summary: score() of pattern queries disagrees with Elasticsearch and with itself: -- 'Inverted indexes are suitable for full text search', -- 'The parser splits text into multiple terms', ... SELECT score() FROM document WHERE SEARCH('content:/.*ext/'); -- BM25 sum SELECT score() FROM document WHERE SEARCH('content:*ext'); -- BM25 sum SELECT score() FROM document WHERE SEARCH('content:te*'); -- 1 SELECT score() FROM document WHERE content MATCH_REGEXP 'ext'; -- 0 Elasticsearch scores a query_string /regexp/, wildcard or prefix, and a regexp, wildcard or prefix query, with a constant 1 per matching row (default rewrite constant_score_blended); measured on 9.5.5 with the rows above: 1.0 for both matching rows. Only an explicit "rewrite": "scoring_boolean" sums the BM25 of the expanded terms. Next to a scored clause in a bool must, a regexp adds exactly 1. #69052 made SEARCH regexp and wildcard sum the BM25 of every expanded term, i.e. scoring_boolean always on, while prefix stayed constant. That sum also takes idf and avgdl from the current segment, not from the tablet statistics MATCH uses: two inserts of the same text scored 0.2876821 and 1.3862942 for identical 'apple' rows, while MATCH 'apple' scored 0.6931471 for both. MATCH_REGEXP never requested a scoring context, so it scored 0 like a filter. Fix: 1. Revert the scoring part of #69052: ExpandWeight scores every pattern with the constant scorer again. The test fixture extensions that #69051 built on are kept. 2. MATCH_REGEXP requests scoring on every index; the regexp expansion then publishes the constant 1. The MATCH statistics collector skips it, since a constant reads no statistics and the pattern is not text to analyze.
airborne12
added a commit
to airborne12/apache-doris
that referenced
this pull request
Nov 2, 2026
…tory ### What problem does this PR solve? Issue Number: None Related PR: apache#69051 Problem Summary: while collecting BM25 statistics, snii_field_has_no_terms checks whether a field is a BKD index or a VARIANT path the segment does not hold. It called IndexFileReader::snii_logical_indexes(), which returns the directory by value, so every scored field of every segment deep-copied all directory entries, with their suffix strings and BKD file tables. The check runs for every SNII score query, including analyzed MATCH and SEARCH on a table where a VARIANT index adds thousands of unrelated paths. A standalone estimate with the same entry layout gave about 260 us per copy at 2048 entries, against about 4 us for the scan alone. Add SniiSegmentReader::find_logical_index, which exposes the MetadataDirectory::find lookup open_index already uses, and IndexFileReader::snii_logical_index_kind on top of it. The check now reads the kind of one entry and copies nothing. Results do not change. ### Release note None ### Check List (For Author) - Test: Regression test / Unit Test - Unit Test: new SniiSegmentReaderLogicalIndexesTest .FindsDirectoryEntryWithoutCopyingIt; 1620 tests in IndexReaderHelperTest, CollectionStatistics* and *Snii* pass, among them SniiScoringSkipsVariantPathMissingFromSegment and SniiScoringRejectsMissingLogicalIndexForWholeCollection - Regression test: test_match_keyword_score (its BKD and sparse VARIANT cases on V2/V3/SNII), test_omit_norms, test_match_phrase_prefix_score, test_bm25_score, test_search_score_keyword_filters, test_search_score_raw_index, test_search_score_pattern_constant pass - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
airborne12
added a commit
that referenced
this pull request
Nov 3, 2026
### What problem does this PR solve? Issue Number: None Related PR: #69051 Problem Summary: score() of a MATCH on an untokenized (keyword) inverted index is always 0, while SEARCH on the same field scores BM25: INDEX idx_redirect (redirect) USING INVERTED -- one row ('BP') SELECT score() FROM t WHERE redirect MATCH 'BP'; -- 0 SELECT score() FROM t WHERE SEARCH('redirect:BP'); -- 0.2876821 Elasticsearch scores a match, term or match_phrase query on a keyword field with BM25 in query context: no norms and no frequencies, so freq = dl = avgdl = 1 and the score is the idf of the whole value (0.2876821 above, 0.6931471 for N=4, n=2). Only a bool filter scores 0. SEARCH already gives exactly these values. MATCH on keyword behaved like a filter, so `body MATCH 'apple' AND tag MATCH 'x'` ranked as if the tag clause were not there. Root cause: #69051 made the engine score keyword postings (unit frequency and norm) but kept MATCH out on purpose: can_score_match required an analyzed index, and MatchPredicateCollector returned early for indexes without an analyzer, so no statistics were collected for them. Fix: a keyword (non-analyzed) index is eligible for MATCH scoring regardless of support_phrase, and MatchPredicateCollector collects its whole value as one term (case and spaces kept), the same term the reader looks up and the same as SearchPredicateCollector collects. Analyzed indexes keep the existing support_phrase / lowercase-normalizer rule. Equality and IN stay pure filters. A keyword MATCH_PHRASE_PREFIX scores the constant 1 of a single-term prefix; MATCH_REGEXP is unchanged.
yiguolei
pushed a commit
that referenced
this pull request
Nov 5, 2026
…ARCH #69790 (#70008) Backport of #69790 to branch-4.2. ### What problem does this PR solve? Issue Number: None Related PR: #69790, #69051 Problem Summary: score() of a MATCH on an untokenized (keyword) inverted index is always 0, while SEARCH on the same field scores BM25: INDEX idx_redirect (redirect) USING INVERTED -- one row ('BP') SELECT score() FROM t WHERE redirect MATCH 'BP'; -- 0 SELECT score() FROM t WHERE SEARCH('redirect:BP'); -- 0.2876821 Elasticsearch scores match, term and match_phrase on a keyword field with BM25 in query context, so the score is the idf of the whole value. #69051 (already on 4.2) made the engine score keyword postings but kept MATCH out. This makes a keyword index eligible for MATCH scoring and collects its whole value as one term, the one the reader looks up. It also carries the follow-up fixes from #69790's review: - A keyword MATCH_PHRASE drops its ` ~N` slop before collecting. - A MATCH on a non-string leaf such as `CAST(n AS STRING)` collects nothing. - A VARIANT path that a segment indexed as BKD, or does not hold, adds no statistics. This covers the BKD searcher coming from the searcher cache, and SNII. - A SNII index kind is looked up without copying the directory. ### Branch adaptations - `can_score_match` keeps 4.2's analyzed rule (support_phrase only); branch-4.2 has no lowercase-normalizer clause. - `test_omit_norms` does not exist on 4.2, so its `.out` change is dropped. The branch was cut before #69922 (the 4.2 backport of #69815) and then merged with branch-4.2 to resolve its conflict in `index_reader_helper_test.cpp`: both tests are kept, as on master. - `test_match_keyword_score` inserts its VARIANT rows as JSON strings: `parse_to_variant` in `VALUES` fails on 4.2 with a max-subcolumns conversion error. The expected output is master's file, unchanged. - **#69901 (BM25 statistics per index) is not on 4.2.** A MATCH on a keyword index and a MATCH on an analyzed index of the same column in one `score()` query (`name MATCH 'apple' USING ANALYZER english AND name MATCH 'apple' USING ANALYZER none`) now fails with "Scoring predicates for one field select different inverted indexes". Master behaved the same before #69901. Before this change, that query ran with the keyword clause scoring 0. Each clause alone scores fine. Two analyzed indexes on one column already fail this way on 4.2. Backporting #69901 lifts this. ### Validation On this branch merged with branch-4.2 c2aaed9, ASAN build: - BE UT: 1611 tests in `IndexReaderHelperTest`, `CollectionStatistics*` and `*Snii*` pass. - Regression: `test_match_keyword_score` passes against master's `.out` on V2/V3/SNII. The following also pass: `test_match_phrase_prefix_score`, `test_bm25_score`, `test_score_filter_matrix`, `test_score_filter_usage`, `test_search_score_topn_predicates`, `test_multi_analyzer_query`, `test_match_using_analyzer`, `test_single_column_multi_index` (with debug points on), `test_search_score_keyword_filters`, `test_search_score_raw_index`, `test_search_score_pattern_constant`. - Local note: the local thirdparty predates the lance-c upgrade, so `lance_reader.cpp`'s `lance_scanner_set_scalar_index_auto_selection` call was stubbed for the local build only. It is not part of this PR. ### Release note score() of MATCH / MATCH_ALL / MATCH_PHRASE on a keyword (untokenized) inverted index now returns the BM25 score of the value, the same as SEARCH, instead of 0. ### Check List (For Author) - Test: Regression test / Unit Test (see Validation) - Behavior changed: Yes. Keyword MATCH contributes its BM25 score to score() instead of 0. - Does this need documentation: No 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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: #68758
Problem Summary: SEARCH can match a keyword inverted index, but projecting score() can drop matched rows or report an unsupported index. Keyword indexes omit norms and may omit frequencies; they still need usable BM25 defaults.
Preserve whole keyword values, including case and whitespace. Use frequency and encoded norm 1 for docs-only/no-norm postings. Legacy indexes without total-term-frequency metadata use average length 1; SNII keeps its physical statistics. Analyzed segments missing required scoring metadata and numeric BKD scoring retain their existing restrictions.
This branch incorporates master's unified MATCH/SEARCH engine from #68758. Keyword norm handling lives in the CLucene postings cursor, including partition switches and WAND bounds; SNII cursors provide docs-only frequencies and norms. The obsolete query_v2/SNII scoring implementations are removed with master.
Keyword matching and missing-metadata behavior follow Elasticsearch while retaining Doris's existing BM25 formula and collection-statistics conventions.
P1 follow-up: keyword filters and internal conjunctions
parser=nonecan inheritsupport_phrase=truefrom FE defaults. Ordinary MATCH filters then entered scoring although the statistics collector skipped that field. Both IndexReaderHelper query-type overloads now share the canonical analyzer-and-phrase eligibility check used by statistics collection. The implementation is out of line to keep analyzer dependencies out of the header. Named-analyzer and normalizer behavior is preserved. Equality and keyword MATCH filters keep the SEARCH-only baseline score.SEARCH('body:apple AND tag:x')continues to score both fields.Release note
SEARCH with score() supports keyword inverted indexes with valid BM25 scores, including conjunctions, TopN ordering and WAND execution. Keyword MATCH filters do not request uncollected statistics when phrase support is inherited.
Check List (For Author)
-forceGenOutand replayed. The original 24 output blocks retain exactly the same values. All 54 raw-index SQL checks also pass../build.sh --be -j16and live SQL replay on an isolated instance passed. Both reported P1 scenarios reproduced on the prior binaries. The final BE ELF requires no GLIBC version newer than 2.10 (baseline 2.17).Validation
8f43208c770fab8a697d4ec71cf615154e4c0878under clang-format 16, and the complete PR English code gate.1c146c172aaf4c308563b9aa0abe2943b6d086e0, TeamCity compilation 1072159, BE UT 1072160 (15,254 passed, 114 ignored), P0 1072181 (6,204 passed, 1 muted), Cloud P0 1072183 (5,016 passed, 3 muted), External 1072182 (761 passed), and Vault P0 1072184 (21 passed) finished successfully. Automatic review 38923289710 reports zero P0/P1 findings and confirms both prior P1 issues are addressed. NonConcurrent retry 1072283 finished successfully on the same head and configuration (548 passed, 1 muted). The original first-failing index-compaction fault-injection test passed without muting. The source of the earlier memory pressure remains unconfirmed. Compilation, BE UT and the core regression configurations now have current-head finished successes. The latest coverage aggregation 1072314 and FE coverage check 1072315 also finished successfully. Compilation, BE UT, P0, Cloud P0, External, Vault, NonConcurrent and both coverage checks have current-head finished SUCCESS results, with no running or queued builds at verification. Performance 1071833 ran the older head 46e15ab and is excluded from this validation. GitHub checks were complete with success or workflow-defined skips, and the PR had no merge conflict at verification. Maintainer approval is still required.Check List (For Reviewer who merge this PR)