Repository navigation
[fix](be) Score MATCH on keyword inverted indexes like SEARCH - #69790
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#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: apache#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. ### 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 and Elasticsearch, instead of 0. ### Check List (For Author) - Test: Regression test / Unit Test / Manual test - Regression test: new test_match_keyword_score (V2/V3/SNII, implicit and explicit parser=none); test_search_score_keyword_filters .out regenerated: only the keyword MATCH blocks change, now adding idf(x) and equal to the SEARCH AND blocks; test_omit_norms .out regenerated: its queries are MATCH_PHRASE_PREFIX on keyword indexes, now 1.0 instead of 0.0. 25 score-related suites in inverted_index_p0, search and variant_p0 pass - Unit Test: IndexReaderHelperTest, CollectionStatisticsTest - Manual test: the reported table on an ASAN build; MATCH equals SEARCH and Elasticsearch 9.5.5 to 7 digits, also for ARRAY<STRING>, a column with both a keyword and an analyzed index, and OR/AND mixes - Behavior changed: Yes. Keyword MATCH contributes its BM25 score to score() instead of 0. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
|
run buildall |
|
/review |
TPC-H: Total hot run time: 28396 ms |
TPC-DS: Total hot run time: 151421 ms |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Static review of PR 69790 at exact head ae2b737. I verified one new P1 correctness issue in keyword MATCH_PHRASE scoring: the reader strips supported trailing phrase slop (~N/~N+) before executing the keyword term, while the new collection path records the unstripped literal and therefore computes IDF for a different term. The remaining changed code was traced through index selection, CLucene/SNII no-norm handling, NULL/hidden-row accounting, and array/variant candidate resolution without another verified defect. Changed tests and expected-output labels are structurally aligned; tests were inspected but not executed under the static-only contract. There were no existing inline P0/P1 findings to carry forward, and no additional user focus was supplied. The review did not converge because all assigned subagent launches terminated with the recorded temporary model-capacity error before returning NO_NEW_VALUABLE_FINDINGS; the main-agent recovery and final sweeps completed, but delegated convergence evidence is missing.
Review did not converge after 3 rounds. This automated review cannot pass the review check.
Convergence failed at the three-round cap because every assigned subagent launch terminated with the recorded temporary model-capacity error before appending a result or returning NO_NEW_VALUABLE_FINDINGS. The main agent independently completed the full changed-file, mechanism, test-structure, anchor, and live-state sweeps, but delegated convergence evidence remains unavailable.
ClickBench: Total hot run time: 24.44 s |
|
Codex automated review failed and did not complete. Error: Review recovery stopped: No final review submission was declared Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
There was a problem hiding this comment.
Request changes at head ae2b737. Two new P1 regressions abort supported scored queries: mixing keyword and analyzed indexes on the same column, and using an indexed numeric-to-string MATCH as a scalar filter beside a scored text MATCH. Both are detailed inline.
Existing P1 4408148364 remains applicable and is retained without reposting. Keyword MATCH_PHRASE slop is stripped by query lowering but retained in the collected term; the scorer can fail with missing term statistics. This is distinct from the two new findings.
Critical checkpoint conclusions from static inspection:
- Goal and proof: ordinary keyword MATCH/MATCH_ALL/MATCH_PHRASE now collect a whole-value term and request BM25, agreeing with SEARCH on the covered cases. The new BE unit tests and regression cases exercise this, but the three reported boundary cases remain uncovered.
- Scope and reuse: the two production changes are small and focused; analyzed collection reuses analyze_plain_query. The new raw-term helper still needs query-lowering alignment, physical-index identity and reader-type constraints as described in the findings.
- Concurrency: no new threads, mutable globals, atomics or lock ordering. Existing tablet collection is serialized by TabletCollectionStatistics::_mutex, with statistics copied into each scanner; score accumulation stays per scanner. No additional concurrency defect found.
- Lifecycle: TermInfo owns its value, selected variant metadata retains ownership, and cache/searcher handles keep their existing lifetimes. No new static initialization dependency or ownership cycle.
- Configuration: no configuration added or changed; dynamic update behavior is unaffected.
- Compatibility: no function signatures, wire definitions or storage formats changed. Existing V2/V3/SNII adapters consume the new keyword score requests; no added rolling-upgrade protocol requirement found.
- Thrift/protobuf IDs: no gensrc changes.
- Parallel paths: reviewed MATCH versus SEARCH, analyzed and keyword indexes, string arrays/variant resolution, multiple-index selection, scalar fallback, boolean/NULL handling, no-score queries, prefixes and regexp. The two new failures are at those selection/fallback boundaries. Keyword phrase prefixes correctly keep constant score 1; regexp and phrase-edge eligibility stays unchanged.
- Conditions and comments: the new eligibility and term branches explain whole-value semantics with examples, but non-analyzed properties alone do not prove a compatible text reader. The missing distinctions are covered inline.
- Test coverage: cases cover implicit/explicit parser=none, inherited phrase support, case/spaces, NULL rows, mixed fields, equality/IN controls, V2/V3/SNII and WAND on/off. Add same-column mixed-analyzer, numeric-cast filter and phrase-slop regressions. New queries order results and preserve the final fixture.
- Changed results: all changed row sets and distinct numeric outputs agree with the traced formulas. SNII's lower nullable-keyword values reflect existing physical token-count/average-length behavior already present in SEARCH; the prefix outputs correctly change from 0 to 1.
- Observability: existing statistics logs, scan profile timers and propagated error messages cover this path; no new logging or metrics requirement identified.
- Transactions/persistence: no transaction, visibility-version, rowset lifecycle, delete-bitmap mutation, EditLog or replay change.
- Writes/atomicity/crashes: no data-write path or persistence mutation added; no new crash-atomicity obligation.
- FE/BE variables: none added. Existing explicit analyzer bindings were traced through FE translation into BE selection for the findings.
- Performance and memory: a raw keyword term avoids analyzer work and uses the existing deduplicated term/statistics path. Scored requests correctly bypass bitmap-result cache/single-flight so scores are populated; unscored requests retain existing behavior. No additional substantiated performance, memory-ownership or error-propagation issue found.
- BE module conventions: no new headers/includes, template instantiations, src .cpp test inclusions or build-gate table edits. Index lifetime and three-valued bitmap handling are unchanged.
- Other issues and user focus: no additional user focus was supplied. The full nine-file diff and related control flow were reviewed; no other verified new issue remains.
Review converged after 2 rounds. All candidates and initial risk focuses have evidence-backed dispositions; no unresolved static coverage gap remains. This was a static-only review: no builds, Doris tests, benchmarks or runtime validation were run, and generation provenance of the .out files was not independently verified.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #69790 (comment)
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…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 <[email protected]>
|
run buildall |
|
/review |
There was a problem hiding this comment.
Review converged after three bounded rounds. I reviewed all nine changed files and the related MATCH, SEARCH, collection-statistics, CLucene, SNII, variant, and regression paths. Two new findings are included inline: an untyped VARIANT cast can make statistics open a physical BKD index through the text-reader path, and malformed raw keyword bytes can collide after conversion to wide-string statistics keys. Existing P1 comment 4409110669 remains applicable because same-column mixed physical indexes are still rejected; it is carried by ID only. The earlier numeric-BKD and phrase-slop findings are fixed in this head. The added V2/V3/SNII tests and filter cases are coherent; the SNII keyword score difference matches unchanged SEARCH behavior and was treated as pre-existing. There was no user-provided focus beyond the complete review. This was a static-only review; no builds or tests were executed.
Review converged after 3 round(s).
Existing P0/P1 findings confirmed for this head: #69790 (comment)
| @@ -285,6 +286,32 @@ | |||
There was a problem hiding this comment.
[P1] Do not score untyped VARIANT paths as text
When an extracted VARIANT path has no materialized type, resolve_text_scoring_index_candidates() forces it to STRING and clones the parent index. A dynamic numeric path can inherit metadata with parser/analyzer removed and be written as BKD, while CAST(v['n'] AS STRING) MATCH '1' is excluded from index execution because the source type is VARIANT. This gate nevertheless admits the statistics collector, which can open that BKD file through the CLucene/SNII text-statistics path and fail before row-level filtering. Keep unproven VARIANT paths out of scoring or prove the physical reader type, and add a dynamic numeric VARIANT cast regression.
There was a problem hiding this comment.
Confirmed and fixed in 27cdbb5, at the point where the failure happens rather than in the collector.
Reproduced on 7881be5 with INDEX idx_v(v) USING INVERTED and SELECT score() ... WHERE CAST(v['n'] AS STRING) MATCH '1': V2/V3 fail with "No segments* file found", SNII with "logical index is not a text inverted index". The same query also failed before this PR when the VARIANT index is analyzed (parser=english), so the collector is not the only way in; this PR made the plain VARIANT index reach it too.
Keeping untyped VARIANT paths out of scoring in the collector would also drop text paths, which score correctly today (CAST(v['s'] AS STRING) MATCH 'abc' returns BM25), and the collector cannot tell the two apart: the path type is only known per segment. So statistics collection now treats a segment whose index for the field is BKD like a segment without the path: it adds no statistics (V2/V3: BKD data file in the compound directory, checked before opening a CLucene reader; SNII: the logical index kind). Those rows still match without the index and score 0; the text path keeps its score.
test_match_keyword_score adds a VARIANT case with a numeric path that is BKD in one segment and text in another, plus a text path, on V2/V3/SNII; it failed on all three before the change. 26 score-related suites pass.
TPC-H: Total hot run time: 28351 ms |
TPC-DS: Total hot run time: 150987 ms |
ClickBench: Total hot run time: 24.2 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
There was a problem hiding this comment.
Reviewed the complete 10-file diff at 0487c2c, including the collector, both text-reader formats, upstream expression/index selection, and all changed tests and results.
One new P2 performance finding: the SNII presence/kind precheck deep-copies the complete logical-index directory for every collected field in every segment. This also adds avoidable allocation/copying to existing analyzed MATCH and SEARCH scoring queries on wide VARIANT tables. See the inline finding.
Existing review context: independently confirmed the previous P1 issues are fixed at this head: phrase-slop term identity, same-column physical-index separation, ordinary numeric BKD exclusion, dynamic numeric VARIANT handling, and absent SNII paths. Cached BKD searchers are also type-checked. No existing P0/P1 comment is carried as blocking. The known malformed-direct-UTF8 concern was not substantiated for supported SQL literal inputs; it is not reposted.
Critical checkpoint conclusions:
- Goal: keyword MATCH now uses the existing SEARCH/BM25 machinery; ordinary scalar cases and mixed analyzed/keyword sums agree with the supplied unit/regression evidence.
- Scope and reuse: the change is focused on eligibility, raw keyword term extraction, shared phrase-slop parsing, and optional per-segment index handling.
- Concurrency and locks: no new threads or shared mutable query state. Existing tablet-statistics collection is mutex-serialized, scanner statistics are separate, and cache/reader locking follows existing order. The inline finding identifies unnecessary directory-copy work under the reader's shared lock.
- Lifecycle and static initialization: cache handles/shared ownership retain readers through statistics access; optional-index early exits release local ownership. No new static initialization dependency or ownership cycle found.
- Configuration: no settings added; regression setup explicitly enables the required index execution.
- Compatibility: no storage encoding or externally visible function-symbol change; V2/V3 and SNII reader paths were traced.
- Thrift/protobuf IDs: no definitions changed.
- Parallel paths: checked both eligibility overloads, analyzed/keyword MATCH, MATCH/SEARCH coexistence, numeric and array leaf types, dynamic VARIANT paths, cached/uncached BKD, and prefix/regexp scoring.
- Conditional branches and errors: new checks describe concrete failure cases; missing whole-column SNII indexes and ordinary corruption/I/O failures still propagate errors.
- Test coverage: inspected both unit-test files and all changed regression suites/results, including 54 new format-specific cases, NULLs, phrase slop, mixed numeric/text and sparse VARIANT segments, cache warmup, and WAND toggles.
- Expected results: all changed score families match the source/formula. SNII's nullable-keyword average-length difference and positional keyword-array frequencies are existing SEARCH behavior; new MATCH preserves that behavior.
- Observability: existing statistics logging and propagated statuses remain available; no additional observability requirement identified.
- Transactions/persistence: no transaction, EditLog, replay or failover logic changed; current read-version/hidden-row filtering is preserved.
- Data writes and atomicity: production write paths are unchanged; no new write/crash-atomicity mechanism.
- FE/BE variables: none added; existing analyzer/query metadata is reused.
- Performance and memory: the inline P2 is a verified repeated owning-container copy; its runtime magnitude is unmeasured. No other new substantial allocation or hot-path regression was substantiated.
- Other concerns/user focus: no additional user focus was supplied. All candidates and initial risk foci have explicit dispositions; no unresolved source-review coverage gap remains.
Static review only: builds, unit/regression tests, benchmarks, and binary artifact checks were not run. Generated-output provenance and runtime performance cannot be established by this review.
The review converged after 2 rounds: every assigned agent in the final round returned NO_NEW_VALUABLE_FINDINGS, and the final coverage/candidate/risk sweep is complete.
Review converged after 2 round(s).
…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 <[email protected]>
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
TPC-H: Total hot run time: 28825 ms |
TPC-DS: Total hot run time: 151172 ms |
ClickBench: Total hot run time: 24.22 s |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
/review |
|
Codex automated review failed and did not complete. Error: Error running remote compact task: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
There was a problem hiding this comment.
Static review of PR 69790 at exact head a66e73d found no new P0-P2 defect. The keyword MATCH collector now uses the same whole-value and phrase-slop lowering as the reader; numeric/BKD leaves are excluded before text statistics; V2/V3 and SNII skip only legitimate BKD or absent suffixed VARIANT paths; cached searcher variants are type-checked; and the new SNII kind lookup preserves lock and lifetime behavior without copying the directory. Existing prior findings for phrase slop, numeric BKD, untyped VARIANT, sparse paths, and the SNII directory copy were revalidated as fixed. The mixed-index finding is fixed in the base commit c4b11b5, so no existing blocking comment ID is carried.
Critical checkpoints: the scope is focused and reuses existing lowering, analyzer, cache, and statistics paths; no new concurrency, lifecycle, configuration, FE/BE protocol, thrift/protobuf, transaction, persistence, or data-write behavior was introduced; status and corruption/error propagation remain intact; hidden-row, NULL, prefix, WAND, array, and normalizer behavior follows existing consumers; and no additional performance or observability defect was substantiated. Unit and regression cases were inspected for deterministic ordering and V2/V3/SNII coverage, including keyword, analyzed, numeric-cast, NULL, sparse VARIANT, BKD-cache, phrase-slop, and WAND cases.
This was a static-only review: builds, tests, benchmarks, and runtime validation were not run. The review converged after 3 bounded rounds; the final gpt-6-sol source and test assignments returned no new valuable findings after the earlier temporary model-capacity failures. No inline comments are proposed.
Review converged after 3 round(s).
Brings in test_match_keyword_score (apache#69790), which the next commit pins to a positive subcolumn count. No conflicts; ColumnPB 36 and TColumn 31 stay unique after the merge. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…est_match_keyword_score ### What problem does this PR solve? Issue Number: None Related PR: apache#69812, apache#69790 Problem Summary: test_match_keyword_score (apache#69790) creates `v VARIANT NULL`, so its subcolumn count comes from the session default, which the fuzzy session variables can set to 0. With this PR a new column with count 0 keeps every dynamic path in the sparse column, so `s` has no subcolumn index, MATCH still finds the rows, and score() returns 0 instead of 0.693147. Cloud P0 build 1089519 failed this way on `V2_variant_text_path`. The column now states a count of 2048, like the other suites that need indexed subcolumns. The expected output is unchanged. ### Release note None ### Check List (For Author) - Test: Regression test. With `SET GLOBAL default_variant_max_subcolumns_count = 0` the suite fails on `V2_variant_text_path` without this change and passes with it; it also passes with the default count. FE UT on the merged code: 485 tests, 0 failures. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ARCH (#69791) ### What problem does this PR solve? Issue Number: None Related PR: #69790 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 on branch-4.1, three places keep keyword MATCH out of scoring: StringTypeInvertedIndexReader::query never sets is_similarity_score (and answers score() queries from the query cache), MatchPredicateCollector skips indexes without an analyzer, and is_need_similarity_score only accepts phrase-capable or lowercase-normalizer indexes. Fix: a keyword (non-analyzed) index is eligible for MATCH scoring regardless of support_phrase; the keyword reader requests scoring for the MATCH query types and bypasses the result cache like the full-text reader does; MatchPredicateCollector collects the whole value as one term (case and spaces kept), the term the reader looks up. The legacy scorer already uses norm 1 when a field has none. Equality runs as MATCH_ANY inside the reader but keeps EQUAL_QUERY for the decision, so it stays a pure filter. MATCH_REGEXP and MATCH_PHRASE_PREFIX keep their current scores. ### 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 and Elasticsearch, instead of 0. ### Check List (For Author) - Test: Regression test / Unit Test / Manual test - Regression test: new test_match_keyword_score (V2/V3, implicit and explicit parser=none); its output equals the master PR's V2/V3 blocks - Unit Test: IndexReaderHelperTest, CollectionStatisticsTest - Manual test: the reported table on a branch-4.1 ASAN build; MATCH equals SEARCH and Elasticsearch 9.5.5 to 7 digits, also for ARRAY<STRING>, a column with both a keyword and an analyzed index, and OR/AND mixes - 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 <[email protected]>
…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 <[email protected]>
### Background SEARCH fields are bound during analysis before score binding runs in rewrite. Bound SEARCH predicates use SearchExpression instead of the scalar Search function. ### What problem does this PR solve? Issue Number: None Related PR: apache#69344 (early SEARCH binding), apache#69292 (score binding) PR Type: Bugfix Problem Summary: Valid SEARCH queries that call score() are rejected because score binding only recognizes the unbound Search type. Recognize SearchExpression in the WHERE predicate check, scoring alias propagation and HAVING/QUALIFY validation. ### Details - **Frequency:** SEARCH scoring queries after early SEARCH binding. - **Root cause:** The downstream score binder checks the expression type from the previous rule ordering. - **Why existing tests missed it:** The targeted SEARCH suites did not cover score(); existing score regression suites expose the failure. - **Fix:** Replace all three Search type checks with SearchExpression and update the import. - **Other paths and branches:** MATCH recognition and score usage restrictions remain unchanged. - **Behavior change:** Valid SEARCH scoring queries can pass score binding. ### Testing FE Checkstyle and git diff --check passed. Built FE with DISABLE_BUILD_UI=ON ./build.sh --fe and deployed it to the development cluster. The five targeted FE test classes executed 147 tests with zero failures, errors or skips, including 82 BindScoreToOlapScanTest cases. Ran 21 targeted regression suites: 20 passed, one failed because the existing BE lacks the keyword MATCH scoring change from apache#69790. Its actual 0.290716 score matches the BE-version expected output, while the current expected output is 0.983863. Re-running that suite against the existing generated expected output from the running BE commit passed. Repository expected outputs were not changed. The previous SEARCH score binding error was not reproduced. The initial local FE unit-test attempt stopped during dependency resolution due to Maven-cache sandbox permissions; remote tests provide the validation above. ### Release note Fix planning failures when using score() with SEARCH predicates. ### Check List (For Author) - Behavior changed: Yes (see Details) - Does this need documentation: No - Pre-submission status: Partially verified: current keyword MATCH expected scores require a BE containing apache#69790; validation used the existing BE ### AI Disclosure - Tool: OpenAI Codex - Generated: Code and commit description - Human review: Pending
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:
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 tagclause 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.
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 and Elasticsearch, instead of 0.
Check List (For Author)
and explicit parser=none); test_search_score_keyword_filters .out
regenerated: only the keyword MATCH blocks change, now adding idf(x)
and equal to the SEARCH AND blocks; test_omit_norms .out regenerated:
its queries are MATCH_PHRASE_PREFIX on keyword indexes, now 1.0
instead of 0.0. 25 score-related suites in inverted_index_p0, search
and variant_p0 pass
SEARCH and Elasticsearch 9.5.5 to 7 digits, also for ARRAY,
a column with both a keyword and an analyzed index, and OR/AND mixes
score() instead of 0.
🤖 Generated with Claude Code