Repository navigation
branch-4.1: [fix](be) Score MATCH on keyword inverted indexes like SEARCH - #69791
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: (master PR of the same title, pending)
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
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
|
run buildall |
![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 all seven changed files at head 15b2d09 found two P2 regressions: mixed keyword/analyzed MATCH clauses on one column share incompatible BM25 statistics, and numeric BKD indexes become eligible for text-index statistics through supported casted MATCH predicates. Both are reported inline; no existing inline findings were supplied.
Critical checkpoints:
- Goal and proof: basic keyword MATCH/MATCH_ALL/MATCH_PHRASE scoring follows SEARCH, but the two inline cases prevent complete correctness. Added unit/regression tests cover the scalar path; neither failing case is covered.
- Scope and clarity: three focused production changes reuse the existing scoring machinery; the remaining changes are two unit tests and a regression input/output pair.
- Concurrency and locks: no new thread, lock, or shared-state protocol. TabletCollectionStatistics serializes collection under its existing mutex and copies counters into private scanner statistics; segment scores remain local. No new lock-order problem found.
- Lifecycle and static initialization: query/scorer/iterator ownership and RAII cache handles remain valid through search; variant metadata clones retain their owners. No added globals, initialization-order dependencies, or ownership cycles.
- Configuration: no configuration added; existing score runtime and query options gate scoring/cache behavior.
- Compatibility: no changed storage/wire format or public signature; existing keyword postings are reused. The BKD/text reader mismatch is the separate inline type-boundary bug.
- Thrift/protobuf: no definitions changed, so release-branch ID/enum comparisons are inapplicable.
- Parallel paths: both scoring-helper overloads are aligned; keyword reader cache bypass matches full-text behavior. Equality/ranges remain filters, one-term phrases delegate to term scoring, and prefix/regexp retain existing scores. V2/V3 fixtures are present. Mixed indexes and numeric casts are the two exceptions reported inline.
- Special conditions: comments explain the keyword whole-value term and score-only bitmap-cache bypass. The no-analyzer condition needs the type restriction described inline.
- Test coverage: inspected unit eligibility/collection tests and regression V2/V3 cases for one row, case/spaces, nulls, MATCH variants, SEARCH parity, mixed columns, and equality. Same-column different analyzers and casted numeric indexes need regressions. Array/variant, empty, ignore_above, and cache paths were traced statically.
- Expected results: all 22 new result blocks and 62 row-score pairs agree analytically with the checked-out BM25 formula, including ln(4/3), ln(2), and the analyzed-plus-keyword sums. Result-generation provenance cannot be established from static inspection.
- Observability: existing index-query/search/cache timers, error paths, and optional per-field statistics logging cover this change; no additional observability defect found.
- Transactions/persistence/failover: query-only changes introduce no EditLog, commit/replay, or persistent-state modifications.
- Data writes/atomicity/crashes: production writes, visible-version selection, delete bitmaps, and recovery remain unchanged; regression inserts only set up test data.
- FE/BE variables: none added; existing analyzer context and statistics propagation were traced through planner, scanner, and reader.
- Performance/memory: score-producing reads must enumerate postings instead of returning bitmap-only cache hits; unscored caching is preserved. Query-local score ownership is retained. No additional substantiated performance or allocation issue found.
- Other issues: the full-diff and upstream/downstream sweeps found only the two verified inline issues after deduplication.
User focus: no additional focus points were supplied. All three initial risk areas have explicit dispositions: same-field index identity is a finding; prefix expansion does not read expanded-term scores; keyword cache/norm/null/empty/array/ignore_above paths yielded no additional finding. The numeric-index admission issue was also independently confirmed.
Validation limits: static inspection and arithmetic only. No builds, Doris tests, benchmarks, or runtime validation were performed; no source edits or submodule initialization. CLucene implementation sources are absent from the uninitialized submodule, so format/iterator conclusions use the checked-in writer/reader contracts; the numeric cached-reader variant mismatch is explicit in local code. Review converged in two rounds with no unresolved candidates or unreviewed changed files.
Review converged after 2 round(s).
| // A keyword index stores the whole value as one term, case and spaces included, and | ||
| // its reader looks it up unanalyzed: tag MATCH 'Hello World' scores the single term | ||
| // "Hello World", as SEARCH('tag:"Hello World"') does. | ||
| term_infos.emplace_back(value); |
There was a problem hiding this comment.
[P2] Keep BM25 statistics separate for each selected index
On a STRING column with both a parser=standard, support_phrase=true index and a keyword index, insert apple banana, apple, and pear, then score (body MATCH 'apple' USING ANALYZER standard) OR (body MATCH 'apple' USING ANALYZER none). The clauses need different df(apple) values (2 versus 1) and average lengths (4/3 versus 1), but this newly admitted keyword term merges into the same field-only CollectInfoMap entry. Only the first index_meta survives, and CollectionStatistics/BM25Similarity use field/term keys without index identity, so at least one clause receives incorrect statistics regardless of collection order. Before this change the keyword clause was skipped and did not contaminate analyzed scoring. Carry selected-index identity through collection and scorer lookups, and cover this same-column mixed-index query.
There was a problem hiding this comment.
Known limitation on this branch, not fixed here. Statistics on branch-4.1 are keyed per field, so a keyword index and an analyzed index of one column share one entry when both are scored in the same query, and at least one clause can get the wrong idf. It takes two indexes on one column, both selected in one query with USING ANALYZER; single-index queries are not affected.
Master fixed this separately in #69901 by keying MATCH statistics per index. That does not carry over to this branch directly, because its reader path differs, so it would be its own backport rather than part of this bug fix.
| // postings score with unit frequency and length whatever support_phrase says, as an | ||
| // Elasticsearch keyword field does: over tags (x, x, y, z), tag MATCH 'x' scores | ||
| // idf(x) = ln(2) for both matching rows, the same as SEARCH('tag:x'). | ||
| if (!inverted_index::InvertedIndexAnalyzer::should_analyzer(properties)) { |
There was a problem hiding this comment.
[P2] Restrict no-analyzer scoring eligibility to string indexes
Numeric BKD indexes also have no analyzer, so this condition admits them into the text scoring collector. For a nonempty duplicate-key table with n INT and an inverted index on n, SELECT n, score() FROM t WHERE CAST(n AS STRING) MATCH '1' is accepted and enters common-expression pushdown when enable_common_expr_pushdown=true. The collector strips the cast and selects the original numeric index, then CollectionStatistics tries to open its BKD directory as a CLucene text index (or extracts FulltextIndexSearcherPtr from a cached BKD reader). This fails before the normal incompatible-cast fallback; previously the no-analyzer metadata was skipped. Check the actual column/reader type before admitting these indexes and add this casted-numeric regression.
There was a problem hiding this comment.
Confirmed and fixed in 79bc425. On this branch it was worse than a failed query: IndexReader::open has no try/catch here, so the CLuceneError aborted the BE, and a cached BKD searcher aborted it through std::get.
Fixed where the BKD index is opened rather than in this gate, because a VARIANT path reaches the same code with a schema type that says text: the path is typed per segment. A BKD index now adds no statistics, as a segment without the path does: the cache miss checks for the BKD data file before opening, the cache hit uses get_if. Those rows match without the index and score 0.
test_match_keyword_score covers the numeric cast, VARIANT numeric, text and sparse paths, and the score query after a numeric predicate, on V2/V3; with the old std::get the suite fails with the same abort.
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: None Related PR: apache#69790 Problem Summary: keyword MATCH scoring made statistics collection reach indexes that are BKD, and on branch-4.1 that aborts the BE, because IndexReader::open throws on a scanner thread with nothing to catch it: -- n INT with an inverted index SELECT score() FROM t WHERE body MATCH 'apple' AND CAST(n AS STRING) MATCH '1' ORDER BY score() DESC LIMIT 10; -- terminate called after throwing an instance of 'CLuceneError' -- what(): No segments* file found in DorisCompoundReader@ A VARIANT path reaches it too: it is typed per segment, so a numeric path is BKD where it holds numbers while the schema calls it text. And when a numeric predicate has cached the BKD searcher, the cache hit throws std::bad_variant_access from std::get<FulltextIndexSearcherPtr> instead: SELECT COUNT(*) FROM t WHERE CAST(v['n'] AS BIGINT) = 1; SELECT score() FROM t WHERE CAST(v['n'] AS STRING) MATCH '1' ORDER BY score() DESC LIMIT 10; Fix: a BKD index has no terms, so it adds no statistics, as a segment without the path already does; the cache miss detects it by the BKD data file in the compound directory and the cache hit by the searcher type. Its rows match without the index and score 0, and text paths of the same column keep their BM25 score. The other follow-ups of apache#69790 need nothing here: branch-4.1 has no SNII, so a VARIANT path absent from a segment was already skipped, and its keyword reader takes the whole MATCH_PHRASE value as the term, so the collector already agrees with it on a trailing " ~N". ### Release note None ### Check List (For Author) - Test: Regression test / Manual test - Regression test: test_match_keyword_score adds the numeric cast, a VARIANT numeric, text and sparse path, and the score query after a numeric predicate, on V2/V3. All 32 blocks equal the master PR's; with the old std::get the suite fails with the same abort; 15 score-related suites in inverted_index_p0 and search pass - Unit Test: 65 tests in IndexReaderHelperTest, CollectionStatistics* - Manual test: keyword and analyzed VARIANT indexes on a branch-4.1 ASAN build; the numeric cast aborted the BE before this change - Behavior changed: Yes. These queries return rows instead of aborting the BE. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
run buildall |
…ant like Elasticsearch (#69825) ### What problem does this PR solve? Issue Number: None Related PR: #69815, #69269 Problem Summary: score() of pattern queries disagrees with Elasticsearch and with itself on branch-4.1: -- '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 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. #69269 (the branch-4.1 pick of #69052) made SEARCH regexp and wildcard sum the BM25 of every expanded term, i.e. scoring_boolean always on, with idf and avgdl taken from the current segment rather than the tablet statistics MATCH uses, so identical rows in two segments scored differently. MATCH_REGEXP never requested a scoring context, so it scored 0 like a filter. Fix, the same as #69815 on master: 1. Revert #69269: RegexpWeight scores every pattern with the constant scorer again; its suite that pinned the BM25 sums is removed. 2. MATCH_REGEXP requests scoring on every index, and the legacy RegexpQuery records 1 for every matching row when score() is requested. The MATCH statistics collector skips it, since a constant reads no statistics and the pattern is not text to analyze. A MATCH_REGEXP on a keyword (untokenized) index also scores 1 once the keyword reader requests scoring (#69791); until then it keeps 0. ### Release note score() of SEARCH regexp and wildcard clauses is a constant 1 per matching row again, like prefix clauses and Elasticsearch's default. score() of MATCH_REGEXP on a tokenized index is 1 instead of 0. ### Check List (For Author) - Test: Regression test / Unit Test / Manual test - Regression test: new search/test_search_score_pattern_constant (V2/V3, two segments); all 18 blocks equal the master PR's V2/V3 blocks; 16 existing score suites in inverted_index_p0 and search pass unchanged - Unit Test: 130 tests (IndexReaderHelperTest, CollectionStatistics*, RegexpQueryTest and the query_v2 regexp/wildcard tests restored by the revert) - Manual test: Elasticsearch 9.5.5 on the same rows; a two-segment table now scores 1 for every pattern hit on a branch-4.1 ASAN build - Behavior changed: Yes. SEARCH regexp/wildcard scores and MATCH_REGEXP scores change as described in the release note. - Does this need documentation: No 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ch-score-branch-4.1 Resolve the conflicts with apache#69825 and the merge-on-write statistics change: - index_reader_helper.h: keep the regexp shortcut in front of can_score_match. - index_reader_helper_test.cpp: keep both tests. - collection_statistics.cpp: keep the BKD check in front of the CLucene open, which is now in a try/catch, so a BKD index no longer needs to reach it. With the keyword reader of this PR, a MATCH_REGEXP on a keyword index now scores 1, so test_search_score_pattern_constant gets the case it had to leave out. Tested on the merged tree: test_match_keyword_score, the VARIANT and numeric cases by hand, 17 score-related suites, 91 unit tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run nonConcurrent |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run nonConcurrent |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
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:
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 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)
explicit parser=none); its output equals the master PR's V2/V3
blocks
equals 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