Skip to content

[fix](inverted index) Count delete-sign rows in BM25 statistics when the scan returns them - #69854

Open
guptashivam04 wants to merge 9 commits into
apache:masterfrom
guptashivam04:fix/bm25-statistics-followups
Open

guptashivam04 wants to merge 9 commits into
apache:masterfrom
guptashivam04:fix/bm25-statistics-followups

Conversation

@guptashivam04

@guptashivam04 guptashivam04 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #69438 (BM25 statistics of merge-on-write tables count only the rows the scan returns). This PR carries the review points left open when #69438 was merged.

Problem Summary:

#69438 leaves out of a merge-on-write table's BM25 statistics the rows the delete bitmap hides, and the rows written with the delete sign, so that every replica of a tablet scores alike. Review raised three points it did not address before merging:

  1. Rows with the delete sign are left out even when the query returns them. With skip_delete_sign = true or show_hidden_columns = true, the FE leaves out the __DORIS_DELETE_SIGN__ = 0 conjunct, so the scan returns and scores those rows, but the statistics still hide them. Their scores then differ from those of a table holding the rows the query returns. Reproduction: the new suite below fails on master at once (V2, skip_delete_sign, compared with the returned rows: 313 rows score differently).
  2. SNII statistics redo their work on every query, and a buffer goes untracked. On SNII the statistics opened each segment's logical index and summed its norms again on every query; about 39 ms of the 47 ms the statistics took on a 2 M-row table. On CLucene, each query filtered its hidden rows by the null bitmap only to build the key of the cached sum. Summing the norms of selected rows decoded them into one buffer as large as the selection, which the memory tracker did not see.
  3. CLucene norm reads ignore the query's I/O context. A CLucene reader reads a field's norms through a stream it keeps open for its lifetime and shares between every query of a cached searcher, so the stream carries no query's context. The statistics' norm reads therefore ignored disable_file_cache and the remote cache write limiter, and counted to no query's file cache statistics. Setting the query's context on that shared stream would leave a pointer to the query on a cached reader.

Fix:

  • Delete sign. Neither setting is sent to the BE, so OlapScanLocalState notes, before the conjuncts are normalized into column predicates and storage expressions, whether a scan conjunct is __DORIS_DELETE_SIGN__ = 0, the one the FE adds (MowHiddenRows::filters_delete_sign). The statistics hide rows with the delete sign only when it is there. Rows the delete bitmap marks stay hidden whatever the query sets. Any other predicate of the query on the column, such as show_hidden_columns = true with WHERE __DORIS_DELETE_SIGN__ = 1, is a predicate like any other, which the statistics do not follow, so the delete-sign rows stay counted; a query that writes = 0 itself keeps exactly the rows the FE's conjunct keeps and scores as the default.
  • Cached sums for both formats. The norm length sums move out of CluceneSearcherCacheValue into a NormLengthSums that both CluceneSearcherCacheValue and SniiLogicalReaderCacheValue hold: the sum of a field's non-NULL norm lengths, and the sum of the non-NULL hidden rows' lengths for the last four sets of hidden rows of a field. A set is keyed by the hidden rows themselves (count, first, last and an xxHash64 of the row IDs), so a query whose sums are kept reads neither norms nor NULL rows. A sum is computed once, by the first query that needs it: queries that need the same sum meanwhile wait for it instead of reading the same norms, while queries of other fields or other hidden rows go on, and if computing it fails a waiting query computes it. The sums take a few dozen bytes per field and are evicted with the reader, so the searcher cache's capacity bounds them.
  • SNII reader from the searcher cache. The statistics take a segment's logical index from the searcher cache with the query path's key and cache value (SniiIndexReader::_get_logical_reader), and insert it on a miss, instead of opening it on every query. A BKD index of the same segment caches its searcher under the same key, as a VARIANT path holding numbers does (CAST(v['n'] AS BIGINT) = 1), so the statistics first skip fields without terms (snii_field_has_no_terms, from [fix](be) Score MATCH on keyword inverted indexes like SEARCH #69790) and never look such an entry up.
  • Bounded decoding. IndexSource::sum_norm_lengths_of decodes the norms of selected rows 4,096 rows at a time. The NULL row ID list SNII builds is charged to the query's memory.
  • Scoped I/O context. DorisCompoundReader::ScopedIOContext gives the reads a thread makes through sub-file streams without a context of their own the query's context, for the duration of the statistics' norm read. CSIndexInput drops the context from the shared base stream after every read, also when the read throws, so no query's pointer stays on a cached reader.

Not covered:

  • Rows deleted by a delete predicate (enable_mow_light_delete) are still counted, as the delete bitmap does not mark them. Left for a separate PR.
  • A reader that does not fit one shard of the searcher cache (its capacity / 256: about 838 KB on a BE with a 2 GB mem_limit, where a 1 M-row SNII segment's reader takes about 1.4 MB) leaves the cache at once, for the query path as for the statistics. Such a segment's norms are still summed on every query, as before this PR.
  • The query path's own CLucene scoring reads norms (SegmentTermDocs through IndexReader::norms(field)) without a per-read I/O context, as before. This PR scopes the context only around the statistics' norm reads.
  • Like the CLucene statistics before it, the SNII statistics look up and fill the searcher cache even when the query sets enable_inverted_index_searcher_cache = false, which the query path honours.
  • Rows that row-level TTL ([feature](storage) Support row-level TTL #65858) has expired are filtered by the query but still counted until compaction removes them, as the delete bitmap does not mark them. On a table with row TTL, a replica that has compacted them away can therefore score differently. Left for a separate PR, with the light deletes above.

Release note

Fix BM25 score() on merge-on-write tables with skip_delete_sign or show_hidden_columns: the statistics now count the rows written with the delete sign that the query returns.

Check List (For Author)

  • Test (release build, macOS arm64, 3 BEs on one host, enable_sql_cache = false. Run on master d420f9c41b with these commits, and again on this branch after merging master 378404eeb1, which includes [fix](be) Keep BM25 statistics per index for MATCH on one column #69901, and master 952ed6904f, which includes [fix](be) Score MATCH on keyword inverted indexes like SEARCH #69790, with the same results. The branch also merges cleanly with [fix](inverted index) Do not block scanner threads while BM25 statistics are collected #69810.)

    • Regression test: inverted_index_p0/test_bm25_score_delete_sign_statistics (new). On V2, V3 and SNII, a merge-on-write table with overwritten rows, delete-sign rows (CDC-style inserts with __DORIS_DELETE_SIGN__ = 1) and a DELETE. With skip_delete_sign = true and with show_hidden_columns = true, every returned row must score exactly as in a duplicate-key table loaded with the rows the query returns; by default, as in one loaded with the visible rows. .out results pin the scores of each setting, a show_hidden_columns query that filters __DORIS_DELETE_SIGN__ = 0 itself scores as the default, and one that filters __DORIS_DELETE_SIGN__ = 1 scores its rows as without the predicate. It fails without this PR and passes with it.

    • Existing suites: all 28 other suites that use score() (in inverted_index_p0, search, ann_index_p0, variant_p0, nereids_rules_p0 and fault_injection_p0, including test_bm25_score_replica_statistics and test_bm25_score_statistics_searcher_io_context from [fix](inverted index) Count only visible rows in BM25 statistics of merge-on-write tables #69438 and test_match_phrase_prefix_score from [fix](inverted index) Score MATCH_PHRASE_PREFIX on the tablet idf of the terms the prefix expands to #69698) pass on a 3-BE cluster: 29 suites, 0 failed; after merging master 378404eeb1, 31 suites (with [fix](be) Keep BM25 statistics per index for MATCH on one column #69901's test_match_score_multi_index and [fix](be) Score regexp and wildcard patterns with a constant like Elasticsearch #69815's pattern scoring suite), 0 failed; after merging master 952ed6904f, 32 suites (with [fix](be) Score MATCH on keyword inverted indexes like SEARCH #69790's test_match_keyword_score), 0 failed.

    • Unit Test: OlapScanOperatorDeleteSignTest.* (__DORIS_DELETE_SIGN__ = 0, also once pushed down as a storage expression, filters the rows; = 1, != 0, >= 0, a conjunct on another column, an output-only delete-sign slot, or a table without one does not), NormLengthSumsTest.ConcurrentQueriesComputeASumOnce (while one query computes a sum, queries of the same sum wait and do not compute it, a query of other rows is not blocked, and after a failed computation a waiting query computes it; 50 repeated runs), CollectionStatisticsTest.DeleteSignRowsCountWhenTheScanReturnsThem (real CLucene segment: with filters_delete_sign = false only the delete-bitmap rows are left out, without reading the delete-sign column), CollectionStatisticsHiddenRowsTest.ExcludeHiddenDocsKeepsSumsWithTheIndex (a repeated query with the same hidden rows reads neither norms nor NULL rows; other hidden rows read the NULL rows once and only their own norms), CollectionStatisticsHiddenRowsTest.SumNormLengthsOfDecodesBoundedBatches (10,000 selected rows decode in 4,096-row batches), NormLengthSumsTest.*, and DorisCompoundReaderTest.ScopedIoContextReachesStreamsWithoutTheirOwn (a sub-file stream without a context reads with the scoped one and counts to its statistics, a stream with its own keeps it, and the shared base stream holds no context after a read, also after a failing read). Run locally on macOS (ASAN): 257 tests in the BM25, collection-statistics, scanner, scan-operator, compound-reader, searcher-cache, inverted-index-reader, null-bitmap and null-predicate suites pass on master d420f9c41b with these commits, 258 after merging master 378404eeb1, and 261 after merging master 952ed6904f (with [fix](be) Score MATCH on keyword inverted indexes like SEARCH #69790's new CollectionStatisticsTest cases). Since [fix](be) Keep BM25 statistics per index for MATCH on one column #69901 keeps MATCH statistics under the field and the index id, DeleteSignRowsCountWhenTheScanReturnsThem reads them under kMatchField.

    • Manual test.

      Replicas: a table like the reproduction of [fix](inverted index) Count only visible rows in BM25 statistics of merge-on-write tables #69438 (4 indexed text fields, 3 replicas, 8 buckets, 8 loads, 4 rounds of CDC upserts, delete-sign deletes and a DELETE), on V3 and on SNII, queried with SEARCH('note_search:jobs OR task_search:jobs OR meeting_search:jobs OR profile_search:jobs') AND accountid = 12345. Before any compaction and after a full compaction of one replica of every tablet (1 rowset on that replica, 15 on the others), every one of the 1,595 rows has one score across 15 random-replica runs and use_fix_replica = 0/1/2, the scores after compaction equal those before, and each replica's scores equal those of a table freshly loaded with the visible rows (0 rows differ). With skip_delete_sign = true the two uncompacted replicas score identically; the compacted one returns 109 rows fewer, as base compaction has removed the delete-sign rows.

      BKD searcher in the cache: on SNII MoW and duplicate tables with 3 replicas, a VARIANT column with an inverted index and n holding numbers, CAST(v['n'] AS BIGINT) = 1 followed by CAST(v['n'] AS STRING) MATCH '1' ORDER BY score(), three times each, on the release build after merging [fix](be) Score MATCH on keyword inverted indexes like SEARCH #69790: no BE failure, and the matching rows score 0, as [fix](be) Score MATCH on keyword inverted indexes like SEARCH #69790 intends for a field without terms.

      Cost, against master d420f9c41b, single-replica tables in one bucket, inverted_index_searcher_cache_limit = 1 GB. Query SEARCH('content:jobs OR content:try') ORDER BY score() DESC LIMIT 100; StatisticsCollectTime and query total from the profile, two alternating rounds each:

      Table statistics, repeated query (median of 15) query total statistics, cold query (median of 8, caches cleared)
      SNII, 1.8 M rows 45.5 / 45.4 → 7.8 / 7.7 ms 105 / 106 → 85 / 82 ms 46.3 / 45.6 → 46.5 / 45.9 ms
      CLucene V2, 7.9 M rows 61.6 / 61.7 → 13.3 / 13.3 ms 153 / 152 → 139 / 140 ms 89.9 / 95.4 → 89.5 / 91.7 ms
      CLucene V2, 6 M rows + 2 M overwritten 196.3 / 196.1 → 26.0 / 25.6 ms 158 / 160 → 132 / 119 ms 224.1 / 224.2 → 218.3 / 218.2 ms

      With the default searcher cache of a BE with a 2 GB mem_limit, 1 M-row SNII segments' readers do not stay in the cache, and their statistics cost about the same as before (44 ms vs 47 ms, measured on [fix](inverted index) Count only visible rows in BM25 statistics of merge-on-write tables #69438's head).

      Concurrency: 16 clients starting together on cold caches (every BE cache cleared before each of 5 rounds), each running the query above in full and paging through it 50 rows at a time with ORDER BY score() DESC, job_id, on the V3 and SNII tables: every row has one score, and no paging repeats or misses a row. 90 s of 16 clients that cancel queries, time them out and evict the caches: no other errors, no stuck query, no BE failure, and the same check passes afterwards.

      Memory: BE tcmalloc allocated bytes (sum of the 3 BEs) before and after 1 and 4 queries on each table move within the ±150 MB the sum fluctuates by, with and without this PR, and grow no more with it.

  • Behavior changed:

    • Yes. On merge-on-write tables, queries with skip_delete_sign = true or show_hidden_columns = true (without their own filter on __DORIS_DELETE_SIGN__) now count the delete-sign rows they return in the BM25 statistics, so their scores change. Default queries score as before.
  • Does this need documentation?

    • No.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

Shivam Gupta added 3 commits October 29, 2026 11:17
…the scan returns them

With skip_delete_sign or show_hidden_columns a query on a merge-on-write
table returns the rows written with the delete sign and scores them, but
the statistics still left those rows out, so their scores differed from
those of a table holding the rows the query returns.

The FE filters these rows by adding __DORIS_DELETE_SIGN__ = 0 to the scan
and leaves the conjunct out for both settings, which it does not send to
the BE. The scan now notes whether a conjunct reads the delete-sign slot
before the conjuncts are normalized, and the statistics hide rows with the
delete sign only when it does. Rows the delete bitmap marks stay hidden
whatever the query sets.
…istics cached

Merge-on-write statistics on SNII opened each segment's logical index and
summed its norms again on every query. They now take the logical index
from the searcher cache, as the query path does, and keep the norm length
sums with it, as CLucene keeps them with its searcher. The sums move to a
NormLengthSums that both cache values hold, so they are bounded by and
evicted with the searcher cache.

The sum of a set of hidden rows is now keyed by the hidden rows themselves,
so a query whose sums are kept reads neither norms nor NULL rows on either
format. Before, every CLucene query filtered its hidden rows by the null
bitmap to build the key.

Norms of selected rows are decoded 4096 rows at a time instead of into a
buffer as large as the selection, which the memory tracker did not see.
… context

The statistics read a CLucene field's norms through the stream the reader
keeps for them, which every query of a cached searcher shares and which
carries no query's context, so the reads ignored disable_file_cache and
the remote cache write limiter and counted to no query.

DorisCompoundReader::ScopedIOContext gives the reads a thread makes through
sub-file streams without a context of their own the query's context for
the duration of the norm read. CSIndexInput drops the context from the
shared base stream after every read, also when the read throws, so no
query's pointer stays behind on a cached reader.
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@guptashivam04

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 96.83% (61/63) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.42% (32841/48708)
Line Coverage 52.39% (348904/665913)
Region Coverage 47.76% (283516/593587)
Branch Coverage 49.44% (133897/270831)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 96.83% (61/63) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.58% (36618/47202)
Line Coverage 62.97% (414875/658872)
Region Coverage 59.36% (351085/591429)
Branch Coverage 60.24% (162758/270173)

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28570 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 2934ecaaa044363a7676ba494a47dcd6abc30b62, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17593	5171	5077	5077
q2	2167	314	253	253
q3	10232	1408	820	820
q4	4679	465	339	339
q5	7424	806	525	525
q6	186	187	155	155
q7	755	805	607	607
q8	9249	1372	1137	1137
q9	5597	4236	4176	4176
q10	6823	1286	1030	1030
q11	431	256	240	240
q12	640	411	276	276
q13	18091	3641	3066	3066
q14	265	270	254	254
q15	q16	718	721	659	659
q17	1213	747	575	575
q18	7008	6212	6140	6140
q19	1243	969	593	593
q20	392	354	234	234
q21	5449	2151	2260	2151
q22	352	294	263	263
Total cold run time: 100507 ms
Total hot run time: 28570 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5590	5604	5590	5590
q2	704	588	525	525
q3	5204	5396	4875	4875
q4	2004	2164	1550	1550
q5	5273	5205	5752	5205
q6	260	200	152	152
q7	2303	2141	1930	1930
q8	3167	2911	2725	2725
q9	8122	8073	8045	8045
q10	3741	3677	3296	3296
q11	621	436	419	419
q12	711	715	523	523
q13	3684	3866	3298	3298
q14	319	329	318	318
q15	q16	710	712	650	650
q17	8324	7490	6983	6983
q18	12887	12149	12969	12149
q19	931	793	799	793
q20	2140	2155	1909	1909
q21	5809	4604	4703	4604
q22	489	446	390	390
Total cold run time: 72993 ms
Total hot run time: 65929 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151375 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 2934ecaaa044363a7676ba494a47dcd6abc30b62, data reload: false

query5	4315	578	455	455
query6	429	208	189	189
query7	4795	458	234	234
query8	323	166	152	152
query9	8804	3204	3268	3204
query10	485	316	251	251
query11	5989	3902	3563	3563
query12	150	100	86	86
query13	1248	457	348	348
query14	6136	3946	3651	3651
query14_1	3463	3441	3442	3441
query15	212	203	181	181
query16	945	419	425	419
query17	883	689	542	542
query18	2438	420	320	320
query19	208	169	158	158
query20	88	86	85	85
query21	211	128	109	109
query22	12971	13027	12887	12887
query23	13604	12867	12451	12451
query23_1	12627	12489	12575	12489
query24	7378	867	480	480
query24_1	503	490	496	490
query25	536	421	358	358
query26	1273	283	141	141
query27	2743	446	275	275
query28	4628	2057	2050	2050
query29	1586	625	473	473
query30	302	213	185	185
query31	1003	927	823	823
query32	189	95	91	91
query33	519	300	229	229
query34	904	860	525	525
query35	743	755	654	654
query36	795	814	730	730
query37	130	99	82	82
query38	1776	1751	1631	1631
query39	809	788	739	739
query39_1	740	726	764	726
query40	219	120	97	97
query41	68	63	67	63
query42	87	87	82	82
query43	351	369	313	313
query44	1374	718	696	696
query45	177	174	161	161
query46	854	983	597	597
query47	3034	2950	2754	2754
query48	305	321	222	222
query49	591	394	300	300
query50	689	279	211	211
query51	10773	10516	10398	10398
query52	78	82	72	72
query53	193	212	151	151
query54	240	204	197	197
query55	71	71	70	70
query56	241	211	271	211
query57	1646	1618	1569	1569
query58	294	271	261	261
query59	2093	2139	1998	1998
query60	277	253	230	230
query61	154	147	139	139
query62	390	323	281	281
query63	191	161	167	161
query64	2762	948	802	802
query65	3337	3294	3278	3278
query66	1798	426	308	308
query67	20069	20102	19790	19790
query68	2981	1022	612	612
query69	388	284	251	251
query70	861	772	779	772
query71	307	228	214	214
query72	2746	2535	2287	2287
query73	523	544	296	296
query74	4608	4513	4347	4347
query75	2354	2321	1912	1912
query76	2301	1031	634	634
query77	347	385	282	282
query78	9674	9651	9149	9149
query79	1027	882	542	542
query80	1209	454	362	362
query81	596	320	272	272
query82	777	149	113	113
query83	316	214	201	201
query84	319	136	110	110
query85	999	421	377	377
query86	399	241	236	236
query87	2012	1949	1813	1813
query88	3914	2804	2792	2792
query89	346	284	242	242
query90	1911	176	175	175
query91	166	152	130	130
query92	103	89	82	82
query93	1074	1027	585	585
query94	630	300	276	276
query95	580	359	304	304
query96	690	514	241	241
query97	2461	2437	2308	2308
query98	168	154	148	148
query99	770	775	635	635
Total cold run time: 236677 ms
Total hot run time: 151375 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.15 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2934ecaaa044363a7676ba494a47dcd6abc30b62, data reload: false

query1	0.01	0.01	0.00
query2	0.10	0.05	0.05
query3	0.26	0.08	0.08
query4	1.61	0.09	0.09
query5	0.26	0.24	0.25
query6	1.17	0.68	0.68
query7	0.04	0.01	0.00
query8	0.06	0.04	0.04
query9	0.40	0.34	0.31
query10	0.59	0.58	0.58
query11	0.21	0.15	0.15
query12	0.18	0.15	0.15
query13	0.49	0.49	0.50
query14	1.04	1.02	1.00
query15	0.64	0.62	0.62
query16	0.35	0.34	0.35
query17	1.16	1.13	1.17
query18	0.21	0.20	0.19
query19	2.12	1.95	1.99
query20	0.02	0.01	0.01
query21	15.47	0.19	0.14
query22	4.91	0.05	0.05
query23	16.31	0.33	0.12
query24	2.78	0.59	0.32
query25	0.10	0.08	0.04
query26	1.22	0.21	0.14
query27	0.05	0.04	0.05
query28	3.66	0.74	0.38
query29	12.47	4.09	3.22
query30	0.28	0.14	0.09
query31	2.88	0.56	0.33
query32	3.22	0.60	0.50
query33	3.13	3.22	3.20
query34	15.31	3.95	3.37
query35	3.33	3.33	3.33
query36	0.56	0.43	0.44
query37	0.10	0.07	0.06
query38	0.06	0.04	0.04
query39	0.04	0.03	0.04
query40	0.17	0.15	0.14
query41	0.10	0.03	0.02
query42	0.03	0.03	0.03
query43	0.04	0.03	0.04
Total cold run time: 97.14 s
Total hot run time: 24.15 s

@airborne12

Copy link
Copy Markdown
Member

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of PR 69854 was completed against the exact head 2934eca and converged after 2 bounded rounds. The change has two verified findings: explicit predicates that reference DORIS_DELETE_SIGN can make BM25 statistics hide rows the predicate returns, and the new NormLengthSums cache repeats full norm/NULL scans for concurrent cold misses. The delete-sign path was traced from FE filter injection through scan normalization, parallel scanners, shared tablet statistics, and collection-statistics consumption. The norm/cache and scoped-I/O paths were checked for cache identity, reader lifetime, lock ordering, exception cleanup, batching, NULL handling, and SNII/CLucene parity. No existing inline threads were present, so existing_blocking_comment_ids is empty. There was no additional user-provided focus. Changed tests and regression coverage were inspected, but no builds or tests were run because this invocation is static-only. Non-blocking coverage gaps remain for the production SNII cache branch and multi-field SNII fixture; static inspection found no additional actionable defect.

Review converged after 2 round(s).

const int slot_id = (*delete_sign)->id();
auto reads_delete_sign = [slot_id](const VExprContextSPtr& conjunct) {
return conjunct->root() != nullptr && expr_reads_slot(conjunct->root(), slot_id);
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Distinguish the default delete-sign filter from user predicates. With show_hidden_columns=true, WHERE __DORIS_DELETE_SIGN__ = 1 reaches this scan conjunct, so _conjuncts_read_delete_sign() returns true and collect_hidden_docs() removes all delete-sign rows from BM25 doc/token statistics even though those rows pass the predicate and are scored. This changes avgdl/idf and can change ORDER BY score()/LIMIT results. Detect the FE-generated = 0 filter or carry its semantics explicitly, and add coverage for nonzero predicates.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2f7b088. Only a scan conjunct that is exactly __DORIS_DELETE_SIGN__ = 0 (the FE's filter, or the same condition written by the query) now hides the delete-sign rows from the statistics. Any other predicate on the column, such as = 1, != 0 or >= 0, leaves them counted, like a predicate on any other column.

Covered by OlapScanOperatorDeleteSignTest.OtherPredicateOnDeleteSignDoesNotFilterIt and a new show_hidden_columns + __DORIS_DELETE_SIGN__ = 1 case in test_bm25_score_delete_sign_statistics on V2/V3/SNII, which fails without the change (80 rows score differently).

}
// Computed outside the lock, which other queries of the index take to read their sums.
// Two queries may both compute a sum the index does not hold yet; both get the same.
RETURN_IF_ERROR(compute(out));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Avoid duplicate first-use norm scans. This path unlocks _mutex before compute(out), and the hidden-row method does the same, so concurrent BM25 queries can all miss the same field/set and each run source.sum_norm_lengths() plus NULL-row decoding before racing to cache the result. Large segments and query bursts therefore multiply full norm I/O, CPU, and temporary buffers. Use per-key single-flight state or serialize the first computation, and add a concurrent first-use test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ea8bcaf. The first query that needs a sum computes it. Queries that need the same sum meanwhile wait for it instead of reading the norms, while other fields and other hidden-row sets are not blocked. If the computation fails or throws, the key is released and a waiting query computes the sum itself.

Covered by NormLengthSumsTest.ConcurrentQueriesComputeASumOnce (50 repeated runs on ASAN). Also checked on a 3-BE cluster: 16 clients starting together on cold caches, and a cancel/timeout stress run with cache evictions. No stuck queries, and every row keeps one score.

std::move(opened), reader_size, UnixMillis(), file_reader);
InvertedIndexSearcherCache::instance()->insert(cache_key, cache_value, &out->cache_handle);
}
auto* cache_value = out->cache_handle.get<segment_v2::SniiLogicalReaderCacheValue>();

@airborne12 airborne12 Oct 31, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] A cached SNII BKD searcher reaches this static_cast and crashes the BE, on every SNII table.

The BKD reader caches SniiBkdSearcherCacheValue under the same key. After CAST(v['n'] AS BIGINT) = 1 on a VARIANT path indexed as BKD, a scoring CAST(v['n'] AS STRING) MATCH '1' hits that entry here. Before this PR the same query only failed with logical index is not a text inverted index.

Reproduced on ASAN: UBSan reports object is of type SniiBkdSearcherCacheValue, then ASan reports a heap-buffer-overflow in SniiIndexSource::scoring_stats, on MoW and DUPLICATE tables. This was with #69790, #69901 and this PR merged and the #69790 check removed; I did not build 2934eca alone. Unit tests cannot catch it, because they take the BE_TEST branch.

Depends on: merge #69790 (adds snii_field_has_no_terms, which skips BKD fields) and #69901 (changes the MATCH statistics key) first.

After they merge, this PR needs to:

  1. Resolve the 2 conflicts in collection_statistics.cpp, keeping snii_field_has_no_terms before open_snii_stats_reader.
  2. Change DeleteSignRowsCountWhenTheScanReturnsThem to read kMatchField instead of L"1". Git merges this file cleanly, but the test fails with Not such column 1.

With both done, 32 score() suites and 1629 BE unit tests pass on ASAN. Reference: https://github.com/airborne12/apache-doris/tree/verify-bkd-69854

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged master with #69790: snii_field_has_no_terms now runs before open_snii_stats_reader, so a field indexed as BKD is skipped before the searcher cache is looked up. With your sequence (CAST(v['n'] AS BIGINT) = 1, then CAST(v['n'] AS STRING) MATCH '1' with score()) on SNII MoW and duplicate tables, the release build stays up and those rows score 0. All 32 score() suites and 261 unit tests pass after the merge.

@guptashivam04

Copy link
Copy Markdown
Contributor Author

Thanks @airborne12 for reproducing the BKD crash and for the reference branch. I'll follow your plan: once #69790 and #69901 merge, I'll merge master into this branch, keep snii_field_has_no_terms before open_snii_stats_reader in both conflicts, and switch the test to kMatchField.

Meanwhile I'm working on the two bot findings (user predicates on __DORIS_DELETE_SIGN__, and duplicate first-use norm scans). I'll push them as a new commit and reply in those threads once they're tested.

Shivam Gupta added 4 commits October 31, 2026 16:25
… for the delete-sign filter

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69438

Problem Summary: The BM25 statistics of a merge-on-write table leave out the rows written with the
delete sign when the scan filters them. The scan told that by any conjunct that reads
`__DORIS_DELETE_SIGN__`. With `show_hidden_columns = true`, a query's own predicate on the column,
such as `WHERE __DORIS_DELETE_SIGN__ = 1`, then counted as the filter: the query returned and
scored only the delete-sign rows, while the statistics left exactly those rows out, so their
scores differed from the scores of the same rows without the predicate.

The scan now treats as the filter only a conjunct that is `__DORIS_DELETE_SIGN__ = 0`, the one the
FE adds unless the query sets `skip_delete_sign` or `show_hidden_columns`. Any other predicate on
the column is a predicate of the query, which the statistics follow no more than a predicate on
another column, so the delete-sign rows stay counted. A query that writes `= 0` itself keeps
exactly the rows the FE's conjunct keeps and scores as the default.

### Release note

None

### Check List (For Author)

- Test: Unit Test / Regression test
    - OlapScanOperatorDeleteSignTest.OtherPredicateOnDeleteSignDoesNotFilterIt: `= 1`, `!= 0` and
      `>= 0` on the delete sign do not filter it, `= 0` does.
    - test_bm25_score_delete_sign_statistics: with `show_hidden_columns = true` and
      `__DORIS_DELETE_SIGN__ = 1`, every returned row scores as in a duplicate-key table loaded with
      the rows the query returns without the predicate, on V2, V3 and SNII. Without this change
      the case fails on V2 at once (80 rows score differently); the .out was regenerated with
      -forceGenOut and adds only the `__DORIS_DELETE_SIGN__ = 1` block, whose scores equal those of
      the same rows under `show_hidden_columns` alone.
    - All 29 regression suites that use score() pass on a 3-BE cluster (release build), and the
      BM25, statistics, scanner and scan-operator unit tests pass on ASAN (257 tests).
- Behavior changed: Yes. With `show_hidden_columns = true`, a query that filters
  `__DORIS_DELETE_SIGN__` other than by `= 0` scores on statistics that count the delete-sign rows.
- Does this need documentation: No
…m once for concurrent queries

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69438

Problem Summary: The norm length sums that BM25 statistics of merge-on-write tables keep with a
cached index reader were computed outside the lock, so queries that missed the same sum at the
same time each read the norms (and the NULL rows) to compute it. That happens after every load:
the load hides older rows, so queries at the new version need a new hidden-row sum for every
segment it touched, and a burst of concurrent queries read the same norms many times. Holding the
lock while computing, as the CLucene sums did before, would instead make queries of other fields
and other hidden rows of the index wait.

A sum is now computed by the first query that needs it. A query that needs a sum being computed
waits for it and reads the kept sum; queries of other fields or other sets of hidden rows go on.
If computing a sum fails or throws, the key is given up and a waiting query computes it itself, so
one query's failure never fails another.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - NormLengthSumsTest.ConcurrentQueriesComputeASumOnce: while one query computes a sum, four
      queries of the same sum wait and get it without computing, a query of another set of rows is
      not blocked, and after a failed computation a waiting query computes the sum. It passes 50
      repeated runs on ASAN, with the other 256 BM25, statistics and scanner unit tests.
    - Manual test on a 3-BE cluster (release build): 16 clients starting together on cold caches,
      5 rounds, on V3 and SNII tables with overwritten and deleted rows: every row has one score
      and paging with a tiebreaker repeats or misses no row. 90 s of 16 clients with cancelled and
      timed-out queries and cache evictions: no other errors, no stuck queries, no BE failure.
- Behavior changed: No
- Does this need documentation: No
… MATCH statistics key

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69901

Problem Summary: apache#69901 keeps the BM25 statistics of a MATCH under the field and the index id
(`match_statistics_field`), so CollectionStatisticsTest.DeleteSignRowsCountWhenTheScanReturnsThem,
which collects them for a MATCH and still read them under the field alone, failed on master with
"Not such column 1". The test now reads them under `kMatchField`, as the other MATCH tests of the
file do.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - The test fails on master without this change ("Not such column 1") and passes with it; the
      BM25, statistics, scanner and scan-operator unit tests pass on ASAN (258 tests).
- Behavior changed: No
- Does this need documentation: No
@guptashivam04

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28314 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit d791ac028babb146194cbdc3e3452cf3b108a86b, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17589	5152	5040	5040
q2	2168	313	247	247
q3	10234	1395	817	817
q4	4680	464	337	337
q5	7428	785	534	534
q6	180	182	152	152
q7	741	798	605	605
q8	9261	1352	1053	1053
q9	5559	4185	4193	4185
q10	6815	1289	1021	1021
q11	428	246	223	223
q12	633	395	273	273
q13	18138	3699	3043	3043
q14	281	273	264	264
q15	q16	744	736	669	669
q17	1198	750	586	586
q18	7257	6205	6101	6101
q19	1242	959	587	587
q20	391	335	220	220
q21	5327	2102	2256	2102
q22	343	292	255	255
Total cold run time: 100637 ms
Total hot run time: 28314 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5583	5615	5568	5568
q2	701	568	516	516
q3	5261	5452	4925	4925
q4	1983	2143	1568	1568
q5	5270	5188	5610	5188
q6	256	207	150	150
q7	2363	2114	1890	1890
q8	3140	2875	2658	2658
q9	8110	8060	8144	8060
q10	3769	3633	3271	3271
q11	644	456	407	407
q12	671	711	512	512
q13	3560	3847	3223	3223
q14	309	301	278	278
q15	q16	696	729	649	649
q17	8303	7608	6933	6933
q18	12995	12215	12902	12215
q19	901	762	782	762
q20	2153	2166	1908	1908
q21	5580	4537	4666	4537
q22	482	458	419	419
Total cold run time: 72730 ms
Total hot run time: 65637 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151563 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit d791ac028babb146194cbdc3e3452cf3b108a86b, data reload: false

query5	4306	563	440	440
query6	422	216	185	185
query7	4800	452	229	229
query8	325	160	151	151
query9	8778	3170	3183	3170
query10	477	312	250	250
query11	5958	3836	3559	3559
query12	145	91	86	86
query13	1256	463	347	347
query14	6121	3944	3676	3676
query14_1	3418	3426	3404	3404
query15	210	203	181	181
query16	950	439	403	403
query17	859	667	525	525
query18	2418	430	320	320
query19	199	159	142	142
query20	88	86	82	82
query21	216	129	107	107
query22	13007	12970	12795	12795
query23	13532	12969	12620	12620
query23_1	12627	12708	12522	12522
query24	7370	822	490	490
query24_1	481	506	484	484
query25	539	413	362	362
query26	1267	256	140	140
query27	2762	443	272	272
query28	4623	2085	2065	2065
query29	1589	588	453	453
query30	300	219	183	183
query31	1015	934	813	813
query32	154	92	99	92
query33	521	294	240	240
query34	907	865	518	518
query35	740	761	656	656
query36	859	842	758	758
query37	124	96	82	82
query38	1773	1738	1683	1683
query39	790	778	753	753
query39_1	728	721	745	721
query40	220	118	95	95
query41	72	70	69	69
query42	87	82	82	82
query43	345	368	311	311
query44	1328	715	716	715
query45	186	196	168	168
query46	844	953	561	561
query47	3061	2857	2741	2741
query48	294	302	215	215
query49	577	389	292	292
query50	689	272	210	210
query51	10207	10327	10459	10327
query52	78	78	68	68
query53	185	202	146	146
query54	232	207	174	174
query55	68	64	62	62
query56	227	219	201	201
query57	1671	1626	1570	1570
query58	279	258	253	253
query59	2159	2230	2060	2060
query60	277	243	212	212
query61	144	147	147	147
query62	388	345	282	282
query63	183	156	159	156
query64	2736	963	771	771
query65	3309	3263	3272	3263
query66	1780	421	295	295
query67	20400	20267	20022	20022
query68	2926	987	621	621
query69	395	286	241	241
query70	847	800	794	794
query71	321	244	207	207
query72	2825	2548	2371	2371
query73	515	530	301	301
query74	4641	4523	4314	4314
query75	2342	2294	1914	1914
query76	2315	1014	602	602
query77	342	355	262	262
query78	9803	9735	9196	9196
query79	1072	860	532	532
query80	1233	418	350	350
query81	608	310	276	276
query82	569	129	106	106
query83	298	189	180	180
query84	326	122	95	95
query85	870	421	378	378
query86	399	234	226	226
query87	2025	1934	1834	1834
query88	3872	2806	2787	2787
query89	331	278	240	240
query90	1933	171	170	170
query91	179	144	130	130
query92	94	87	88	87
query93	1152	1006	581	581
query94	638	311	276	276
query95	568	326	365	326
query96	682	503	239	239
query97	2454	2423	2310	2310
query98	157	157	148	148
query99	782	754	649	649
Total cold run time: 235954 ms
Total hot run time: 151563 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.36 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit d791ac028babb146194cbdc3e3452cf3b108a86b, data reload: false

query1	0.01	0.01	0.01
query2	0.09	0.06	0.05
query3	0.26	0.09	0.09
query4	1.61	0.11	0.12
query5	0.24	0.24	0.24
query6	1.18	0.68	0.67
query7	0.04	0.01	0.00
query8	0.06	0.03	0.03
query9	0.40	0.35	0.35
query10	0.61	0.63	0.61
query11	0.20	0.15	0.14
query12	0.18	0.15	0.15
query13	0.49	0.50	0.48
query14	1.04	1.01	1.01
query15	0.63	0.60	0.60
query16	0.36	0.36	0.34
query17	1.09	1.12	1.13
query18	0.21	0.19	0.20
query19	2.07	1.94	1.92
query20	0.02	0.02	0.01
query21	15.49	0.20	0.14
query22	4.97	0.06	0.05
query23	16.29	0.31	0.12
query24	2.85	0.52	0.50
query25	0.09	0.07	0.05
query26	1.10	0.21	0.16
query27	0.05	0.04	0.04
query28	3.65	0.83	0.35
query29	12.47	4.16	3.23
query30	0.28	0.13	0.09
query31	2.78	0.55	0.32
query32	3.22	0.60	0.52
query33	3.24	3.20	3.19
query34	15.33	3.99	3.37
query35	3.36	3.31	3.34
query36	0.55	0.45	0.44
query37	0.09	0.06	0.06
query38	0.05	0.04	0.04
query39	0.03	0.03	0.03
query40	0.18	0.15	0.14
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 97.02 s
Total hot run time: 24.36 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 98.88% (88/89) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.63% (35865/46198)
Line Coverage 62.94% (403019/640361)
Region Coverage 59.44% (340554/572894)
Branch Coverage 60.63% (157819/260312)

…-followups

# Conflicts:
#	be/src/storage/index/inverted/reader/collection_statistics.cpp
@guptashivam04

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 98.88% (88/89) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.23% (32078/47713)
Line Coverage 52.05% (337190/647837)
Region Coverage 47.58% (274062/576014)
Branch Coverage 49.42% (129193/261414)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 98.88% (88/89) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.69% (35910/46225)
Line Coverage 63.02% (403801/640771)
Region Coverage 59.51% (341191/573327)
Branch Coverage 60.72% (158168/260470)

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28968 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 3b2e76185d5c2ee0f389f21da0c36916eb1b4351, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17615	5425	5071	5071
q2	2156	318	285	285
q3	10200	1389	833	833
q4	4681	471	338	338
q5	7438	806	543	543
q6	184	183	152	152
q7	774	815	605	605
q8	9270	1258	1107	1107
q9	5557	4229	4214	4214
q10	6839	1305	1033	1033
q11	442	243	226	226
q12	638	406	293	293
q13	18095	3699	3103	3103
q14	280	268	237	237
q15	q16	745	721	665	665
q17	1219	763	601	601
q18	7043	6207	6130	6130
q19	1259	975	612	612
q20	379	350	229	229
q21	5969	2409	2452	2409
q22	396	328	282	282
Total cold run time: 101179 ms
Total hot run time: 28968 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	6000	5959	6104	5959
q2	708	603	587	587
q3	5658	5857	5419	5419
q4	2080	2148	1584	1584
q5	5707	5810	5668	5668
q6	241	192	150	150
q7	2322	2050	1799	1799
q8	3004	2676	2685	2676
q9	7904	7513	7467	7467
q10	3630	3586	3129	3129
q11	611	419	397	397
q12	659	659	513	513
q13	3286	3670	3083	3083
q14	297	290	279	279
q15	q16	665	689	617	617
q17	7735	7041	6863	6863
q18	12947	12194	12880	12194
q19	916	793	783	783
q20	2155	2137	1928	1928
q21	5701	4784	4710	4710
q22	497	453	414	414
Total cold run time: 72723 ms
Total hot run time: 66219 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151611 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 3b2e76185d5c2ee0f389f21da0c36916eb1b4351, data reload: false

query5	4307	587	461	461
query6	425	202	189	189
query7	4814	448	234	234
query8	326	184	158	158
query9	8778	3187	3215	3187
query10	479	316	251	251
query11	5985	3889	3573	3573
query12	164	90	86	86
query13	1247	451	323	323
query14	6162	3941	3666	3666
query14_1	3430	3446	3443	3443
query15	210	200	180	180
query16	953	467	441	441
query17	856	664	548	548
query18	2427	415	310	310
query19	198	151	135	135
query20	85	84	81	81
query21	214	128	109	109
query22	13055	13002	12880	12880
query23	13526	13058	12462	12462
query23_1	12991	12637	12596	12596
query24	7360	831	469	469
query24_1	476	478	475	475
query25	509	402	336	336
query26	1254	255	141	141
query27	2772	443	268	268
query28	4640	2045	2035	2035
query29	1545	597	447	447
query30	326	224	184	184
query31	1020	929	809	809
query32	160	86	92	86
query33	524	298	231	231
query34	933	855	511	511
query35	743	759	642	642
query36	819	809	719	719
query37	130	96	87	87
query38	1789	1742	1661	1661
query39	805	788	754	754
query39_1	740	739	752	739
query40	219	117	95	95
query41	68	63	64	63
query42	87	81	86	81
query43	347	367	315	315
query44	1311	704	704	704
query45	180	177	172	172
query46	829	969	580	580
query47	2863	2954	2708	2708
query48	306	326	226	226
query49	583	427	314	314
query50	697	290	224	224
query51	10665	10534	10411	10411
query52	82	82	72	72
query53	190	213	168	168
query54	288	196	201	196
query55	68	66	64	64
query56	227	211	209	209
query57	1661	1628	1561	1561
query58	294	270	257	257
query59	2185	2223	1983	1983
query60	291	247	229	229
query61	150	141	143	141
query62	388	328	293	293
query63	190	164	161	161
query64	2748	943	751	751
query65	3358	3290	3275	3275
query66	1779	421	303	303
query67	20043	20121	20031	20031
query68	3166	1006	619	619
query69	397	298	274	274
query70	828	775	797	775
query71	292	288	220	220
query72	2771	2633	2294	2294
query73	509	528	336	336
query74	4638	4517	4352	4352
query75	2336	2291	1928	1928
query76	2329	1102	611	611
query77	363	396	279	279
query78	9793	9668	9182	9182
query79	888	886	541	541
query80	483	421	349	349
query81	536	329	278	278
query82	209	128	107	107
query83	194	198	193	193
query84	297	120	100	100
query85	765	433	373	373
query86	290	241	226	226
query87	1976	1925	1823	1823
query88	3746	2785	2777	2777
query89	322	283	246	246
query90	2091	175	165	165
query91	169	154	134	134
query92	103	89	88	88
query93	971	1013	586	586
query94	454	311	260	260
query95	583	400	300	300
query96	684	484	237	237
query97	2410	2447	2290	2290
query98	161	151	149	149
query99	772	760	655	655
Total cold run time: 234600 ms
Total hot run time: 151611 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.4 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 3b2e76185d5c2ee0f389f21da0c36916eb1b4351, data reload: false

query1	0.01	0.01	0.01
query2	0.10	0.05	0.05
query3	0.27	0.09	0.08
query4	1.60	0.10	0.11
query5	0.24	0.24	0.24
query6	1.16	0.68	0.68
query7	0.04	0.00	0.00
query8	0.05	0.04	0.04
query9	0.41	0.33	0.35
query10	0.59	0.56	0.57
query11	0.19	0.14	0.13
query12	0.18	0.15	0.15
query13	0.49	0.49	0.50
query14	1.03	1.01	1.03
query15	0.64	0.62	0.62
query16	0.30	0.34	0.36
query17	1.15	1.16	1.12
query18	0.20	0.19	0.19
query19	2.14	1.96	2.02
query20	0.02	0.01	0.02
query21	15.50	0.19	0.13
query22	4.91	0.06	0.06
query23	16.25	0.32	0.13
query24	2.60	0.54	0.39
query25	0.10	0.08	0.05
query26	1.20	0.21	0.16
query27	0.04	0.05	0.05
query28	3.60	0.88	0.37
query29	12.50	4.15	3.25
query30	0.28	0.12	0.10
query31	2.77	0.57	0.33
query32	3.22	0.60	0.51
query33	3.14	3.20	3.19
query34	15.39	4.09	3.41
query35	3.36	3.34	3.33
query36	0.55	0.47	0.45
query37	0.09	0.06	0.06
query38	0.06	0.04	0.04
query39	0.04	0.04	0.03
query40	0.18	0.15	0.16
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.04
Total cold run time: 96.75 s
Total hot run time: 24.4 s

airborne12
airborne12 previously approved these changes Nov 5, 2026

@airborne12 airborne12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@airborne12

Copy link
Copy Markdown
Member

/review

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Selected model is at capacity. Please try a different model.
Workflow run: https://github.com/apache/doris/actions/runs/41982439008

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

yiguolei
yiguolei previously approved these changes Nov 5, 2026
…-followups

# Conflicts:
#	be/test/exec/operator/olap_scan_operator_test.cpp
@guptashivam04
guptashivam04 dismissed stale reviews from yiguolei and airborne12 via 81d5a43 November 6, 2026 04:14
@yiguolei

yiguolei commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

run buildall

@yiguolei

yiguolei commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all 19 changed files at 81d5a43 using static source inspection, two complete-review scopes and an independent risk review. One new P2 finding remains: statistics-created SNII cache entries omit hot-term lookup-cache initialization. No new P0/P1 finding was verified.

Critical checkpoint conclusions:

  1. Goal and evidence: the delete-sign policy now follows whether the scan has the FE's zero filter, while overwritten rows remain excluded by the versioned delete bitmap. Unit and V2/V3/SNII regression coverage was inspected; no runtime pass is claimed.
  2. Scope and clarity: the scan flag, shared norm sums, bounded decoding and scoped I/O context address the stated correctness and statistics-cost work. The SNII initialization omission is the inline finding.
  3. Concurrency: scanner threads share pinned readers; norm-sum maps/in-flight keys use one mutex, computation runs outside it and waiting releases it. Existing lazy reader state is separately synchronized. No new lock-order defect was substantiated. The old duplicate-first-use concern is fixed.
  4. Lifecycle and initialization: cache handles pin readers and their owning IndexFileReader; readers are destroyed before that owner. Query callbacks are synchronous. The new thread-local context pointer has constant initialization, restores its predecessor and requires no TLS destructor. Shared base context is reset before unlocking, including thrown reads.
  5. Configuration: no configuration item is added. Existing skip_delete_sign/show_hidden_columns choices are inferred from the actual scan conjunct; nonzero user predicates leave those rows counted.
  6. Compatibility: no on-disk format or wire contract changes. Source review found no new libc API dependency; no release ELF/glibc audit was performed under this static-only contract.
  7. Thrift/protobuf IDs: gensrc is untouched; cross-branch field-ID comparison is inapplicable.
  8. Parallel paths: CLucene V2/V3 and SNII use the shared sum abstraction; cloud/local scanner flow and physical segment identity remain intact. The SNII query/statistics initialization mismatch is reported inline.
  9. Special conditions: the exact eq/delete-slot/zero check matches FE predicate generation before normalization; comments explain absent/nonzero predicates and norm-less indexes. BKD/missing VARIANT paths are skipped before the typed SNII cache lookup.
  10. Test coverage: inspected scan predicate tests, NULL/hidden-row statistics, cached sums, per-key concurrent first computation and failed-compute retry, 4096-row norm batches, and explicit/scoped I/O precedence plus throwing-read cleanup. The production statistics-cache branch is excluded by BE_TEST, leaving the reported initialization path uncovered there.
  11. Expected results: the regression output's default/explicit-zero and skip/show pairs agree; deleted rows retain the corresponding returned-row scores. Fixture relationships are internally consistent. Actual numeric BM25 results and generated-output provenance were not independently reproduced.
  12. Observability: existing StatisticsCollectTime, file-cache counters and statistics/error logs cover the changed paths; no additional logging/metric defect was found.
  13. Transactions and persistence: no transaction, version publication, EditLog or replay logic changes. The read version still selects the delete bitmap.
  14. Data writes and atomicity: production changes affect read-side statistics/cache state. No write atomicity/failover change is introduced; rowset ownership remains pinned while used.
  15. FE/BE state transport: no new transmitted variable; the new flag is derived locally from existing expressions and passed to all statistics collection paths.
  16. Performance and memory: generic selected norms decode in bounded batches; large row-ID temporaries use tracked vectors, and four hidden sums are retained per field. CLucene overrides both sum methods, avoiding repeated whole-field reads from generic batching. SNII loses its existing nonresident hot-term lookup reuse when statistics populate a cold cache, as reported. No performance measurements were run.
  17. Other concerns and prior review: P1 threads 4430597044 (user delete-sign predicates) and 4430796090 (SNII BKD cache type) are fixed on this head; P2 thread 4430597050 (duplicate first-use work) is also fixed. None is reposted or retained as an existing blocking finding. No additional user focus was supplied.

Validation performed: build-support/check-build-hygiene.sh passed all pure-text checks. No builds, unit/regression tests, benchmarks or cluster execution were performed. No repository source was modified. Remaining limits are dynamic validation, binary ABI validation, numeric-output reproduction and the PR's pre-existing declared light-delete/row-TTL/cache-policy limits; there are no unresolved source-level candidates or uncovered changed files.

Review converged after 2 rounds: all three latest reviewers returned NO_NEW_VALUABLE_FINDINGS, all 19 changed files were covered, and every candidate/risk was accepted, deduplicated or dismissed with evidence.

Review converged after 2 round(s).

InvertedIndexSearcherCache::CacheKey cache_key(file_reader->get_index_file_cache_key(index));
if (!InvertedIndexSearcherCache::instance()->lookup(cache_key, &out->cache_handle)) {
auto opened = DORIS_TRY(open_snii_stats_reader(*file_reader, index, io_ctx));
const size_t reader_size = std::max<size_t>(opened->memory_usage(), 1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Enable hot-term caching before publishing the SNII reader

On a cold searcher cache, statistics populate this entry during OlapScanner::_prepare_impl, before the normal query reader opens. Unlike SniiIndexReader::_get_logical_reader, this path never calls enable_term_lookup_cache(), and later searcher-cache hits return without enabling it. For a nonresident dictionary, term_lookups_ therefore stays null for the reader's cached lifetime, so repeated hot-term queries keep reading and decoding their DICT blocks, including following non-scoring queries. Enable the lookup cache before memory_usage() and insertion here too, so its full reserved charge is included.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28646 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 81d5a43eec0984a907b3c090c606f0307247960d, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17594	5152	5032	5032
q2	2139	302	246	246
q3	10279	1453	817	817
q4	4681	458	333	333
q5	7438	814	527	527
q6	179	184	159	159
q7	744	802	585	585
q8	9239	1301	1073	1073
q9	5595	4200	4145	4145
q10	6810	1288	1013	1013
q11	409	250	224	224
q12	634	397	275	275
q13	18124	3667	3094	3094
q14	283	280	244	244
q15	q16	725	736	666	666
q17	1220	747	575	575
q18	7085	6185	6105	6105
q19	1106	961	598	598
q20	368	342	228	228
q21	5414	2505	2422	2422
q22	385	341	285	285
Total cold run time: 100451 ms
Total hot run time: 28646 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5944	5922	6053	5922
q2	725	594	578	578
q3	5686	5797	5221	5221
q4	1998	2132	1549	1549
q5	5653	5842	5583	5583
q6	230	183	142	142
q7	2194	2042	1759	1759
q8	2933	2610	2658	2610
q9	8014	7651	7456	7456
q10	3630	3551	3179	3179
q11	572	419	372	372
q12	648	673	498	498
q13	3306	3660	3103	3103
q14	285	299	268	268
q15	q16	675	656	620	620
q17	7683	6967	6892	6892
q18	12908	12112	12826	12112
q19	917	799	803	799
q20	2158	2129	1891	1891
q21	5577	4570	4652	4570
q22	530	430	439	430
Total cold run time: 72266 ms
Total hot run time: 65554 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 153735 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 81d5a43eec0984a907b3c090c606f0307247960d, data reload: false

query5	4310	517	395	395
query6	425	206	192	192
query7	4816	443	232	232
query8	326	168	171	168
query9	8781	3225	3243	3225
query10	488	313	259	259
query11	5941	3867	3567	3567
query12	145	93	98	93
query13	1249	439	335	335
query14	6168	3908	3674	3674
query14_1	3370	3439	3403	3403
query15	206	197	190	190
query16	928	463	439	439
query17	860	702	537	537
query18	2415	429	311	311
query19	198	172	133	133
query20	85	85	85	85
query21	210	128	109	109
query22	12947	12906	12685	12685
query23	13520	12868	12472	12472
query23_1	12699	12650	12613	12613
query24	7357	855	473	473
query24_1	487	487	477	477
query25	535	426	364	364
query26	1250	266	144	144
query27	2754	442	278	278
query28	4622	2037	2023	2023
query29	1574	602	462	462
query30	293	218	185	185
query31	993	929	815	815
query32	152	92	96	92
query33	536	308	246	246
query34	926	848	511	511
query35	728	764	648	648
query36	823	825	732	732
query37	129	100	88	88
query38	1755	1749	1671	1671
query39	799	800	740	740
query39_1	727	722	742	722
query40	233	120	99	99
query41	73	70	68	68
query42	99	89	87	87
query43	345	360	316	316
query44	1352	726	717	717
query45	180	175	176	175
query46	835	987	570	570
query47	3034	2929	2701	2701
query48	303	306	219	219
query49	590	389	297	297
query50	708	288	221	221
query51	10633	10625	10317	10317
query52	75	81	69	69
query53	187	214	154	154
query54	234	189	179	179
query55	68	65	65	65
query56	237	213	203	203
query57	1653	1627	1559	1559
query58	281	264	259	259
query59	2126	2180	1969	1969
query60	284	246	217	217
query61	146	152	151	151
query62	392	345	288	288
query63	188	161	165	161
query64	2785	960	783	783
query65	3359	3262	3237	3237
query66	1762	414	298	298
query67	22495	22516	22538	22516
query68	3044	1012	631	631
query69	392	293	252	252
query70	846	795	769	769
query71	307	235	220	220
query72	2698	2598	2304	2304
query73	545	525	293	293
query74	4569	4512	4314	4314
query75	2307	2293	1901	1901
query76	2329	1010	592	592
query77	350	376	276	276
query78	9733	9659	9174	9174
query79	1974	852	545	545
query80	1139	437	350	350
query81	582	317	265	265
query82	589	137	108	108
query83	290	194	186	186
query84	309	126	100	100
query85	866	433	396	396
query86	384	241	233	233
query87	2009	1919	1807	1807
query88	3902	2793	2778	2778
query89	332	284	250	250
query90	1897	173	166	166
query91	175	149	132	132
query92	118	77	94	77
query93	1121	1019	582	582
query94	575	302	277	277
query95	570	369	304	304
query96	689	510	239	239
query97	2451	2456	2380	2380
query98	163	149	148	148
query99	768	771	649	649
Total cold run time: 238925 ms
Total hot run time: 153735 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.03 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 81d5a43eec0984a907b3c090c606f0307247960d, data reload: false

query1	0.01	0.01	0.01
query2	0.10	0.05	0.05
query3	0.26	0.09	0.08
query4	1.61	0.08	0.10
query5	0.23	0.24	0.24
query6	1.20	0.68	0.68
query7	0.04	0.01	0.01
query8	0.05	0.04	0.04
query9	0.39	0.35	0.33
query10	0.57	0.59	0.57
query11	0.20	0.15	0.14
query12	0.19	0.14	0.15
query13	0.48	0.49	0.48
query14	1.02	1.01	1.02
query15	0.61	0.60	0.60
query16	0.35	0.35	0.35
query17	1.14	1.09	1.14
query18	0.19	0.20	0.19
query19	2.13	1.95	2.00
query20	0.02	0.02	0.01
query21	15.50	0.19	0.14
query22	4.89	0.05	0.05
query23	16.23	0.32	0.12
query24	1.06	0.24	0.16
query25	0.10	0.06	0.04
query26	0.76	0.21	0.17
query27	0.03	0.03	0.03
query28	3.68	0.77	0.36
query29	12.48	4.12	3.30
query30	0.28	0.13	0.10
query31	2.77	0.56	0.33
query32	3.23	0.59	0.51
query33	3.30	3.37	3.21
query34	15.25	3.95	3.37
query35	3.34	3.34	3.31
query36	0.55	0.46	0.43
query37	0.09	0.07	0.06
query38	0.04	0.04	0.03
query39	0.04	0.03	0.03
query40	0.17	0.14	0.14
query41	0.08	0.04	0.03
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 94.74 s
Total hot run time: 24.03 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 98.88% (88/89) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.69% (32428/47908)
Line Coverage 52.56% (342669/651981)
Region Coverage 48.07% (278795/579967)
Branch Coverage 50.12% (132357/264096)

@guptashivam04

Copy link
Copy Markdown
Contributor Author

run buildall

@yiguolei

Copy link
Copy Markdown
Contributor

Do not need deal with skip_delete_sign = true, this is just for debug.

@airborne12

Copy link
Copy Markdown
Member

run buildall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants