mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
master
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jack
|
7cca01bc6c |
[fix](be) Score MATCH on keyword inverted indexes like SEARCH (#69790)
### What problem does this PR solve? Issue Number: None Related PR: #69051 Problem Summary: score() of a MATCH on an untokenized (keyword) inverted index is always 0, while SEARCH on the same field scores BM25: INDEX idx_redirect (redirect) USING INVERTED -- one row ('BP') SELECT score() FROM t WHERE redirect MATCH 'BP'; -- 0 SELECT score() FROM t WHERE SEARCH('redirect:BP'); -- 0.2876821 Elasticsearch scores a match, term or match_phrase query on a keyword field with BM25 in query context: no norms and no frequencies, so freq = dl = avgdl = 1 and the score is the idf of the whole value (0.2876821 above, 0.6931471 for N=4, n=2). Only a bool filter scores 0. SEARCH already gives exactly these values. MATCH on keyword behaved like a filter, so `body MATCH 'apple' AND tag MATCH 'x'` ranked as if the tag clause were not there. Root cause: #69051 made the engine score keyword postings (unit frequency and norm) but kept MATCH out on purpose: can_score_match required an analyzed index, and MatchPredicateCollector returned early for indexes without an analyzer, so no statistics were collected for them. Fix: a keyword (non-analyzed) index is eligible for MATCH scoring regardless of support_phrase, and MatchPredicateCollector collects its whole value as one term (case and spaces kept), the same term the reader looks up and the same as SearchPredicateCollector collects. Analyzed indexes keep the existing support_phrase / lowercase-normalizer rule. Equality and IN stay pure filters. A keyword MATCH_PHRASE_PREFIX scores the constant 1 of a single-term prefix; MATCH_REGEXP is unchanged. |
||
|
Jack
|
c31cd7f1cb |
[improvement](be) Read SNII dictionary keys in place in walks and block opens (#69887)
### What problem does this PR solve? Issue Number: None Related PR: #69600 Problem Summary: A regexp without a literal prefix, such as `output MATCH_REGEXP 'transi.*'`, expands by walking every term of the SNII dictionary and testing each key against the pattern. On a 99M-row table, about 40% of the SNII stack samples of that query were spent decoding dictionary entries, twice as much as the pattern match: - `visit_prefix_range` and `visit_term_range` decoded every key into a fresh `DictEntry`: the whole struct reset, a new `std::string`, and a `Status` for every varint. - `DictBlockReader::open` fully decoded every anchor of the block, about 1 entry in 16, only to get its key. Every block open pays for this, exact term lookups included. Both now read an entry's length and front-coded key on the raw cursor. A walk rebuilds the key in one buffer it keeps across entries and a rejected key skips its body by the entry length; `open()` reads only the keys of the anchors. An entry that does not read cleanly goes to the checked decoders, which report what is wrong with it as before, and a walk still decodes the body of every entry it accepts with the checked decoder. The on-disk format and the write path do not change. `open()` no longer decodes the bodies of the anchors. The block's crc32c, verified just before, covers them, and they are checked when they are decoded, like the body of every other entry. | Query (99M rows) | Hot, new/previous | Cold, new/previous | |---|---|---| | regexp `'transi.*'` | 0.69 | 0.77 | | regexp that matches no term | 0.67 | | | regexp `'.*ing'` | 0.72 | | | regexp `'e.*'` (stops at the expansion limit) | 0.90 | | | phrase query, term lookup cache off | 0.78 | | | phrase query, term lookup cache on | 0.99 | 1.01 | | AND of two terms | 0.99 | 1.01 | | the other 22 queries | around 1.0 (reruns of those above 1.05: 0.94-1.03) | | Hot: both versions interleaved in one BE process. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. The build also had #69879 (the term lookup cache) and #69882. With that cache off, which is master today, queries open dictionary blocks more often: the phrase query gains as the 0.78 row shows, and the five other lookup-heavy queries measured that way stayed at 0.96-1.04. |
||
|
Jack
|
62cf8c7588 |
[improvement](be) Read SNII phrase-prefix batches on the calling thread when they hit the page cache (#69882)
### What problem does this PR solve? Issue Number: None Related PR: #69648 Problem Summary: #69648 reads a SNII batch on the calling thread when the index file is local, the batch has at most `snii_inline_read_max_segments` (4) physical segments and 256 KiB, and the moving average of recent reads is under 20 us, i.e. they hit the page cache. A phrase prefix reads the postings of every term its prefix expands to, in batches of 5 to 16 segments that stay well under 256 KiB, so those still went segment by segment through the shared buffered-reader prefetch pool: on a hot 99M-row table, about a fifth of the SNII time of such a query was spent on the pool's lock and latch (`ThreadPool::do_submit`, `CountDownLatch::wait` in `read_batch`). The default of `snii_inline_read_max_segments` becomes 16, which covers those batches, and two changes keep larger inline batches safe when the data is not in the page cache: - A batch read inline hands the segments it has left to the pool as soon as one of its reads is slow. When the average is low but this batch's data is not cached, the batch pays one read on the calling thread and overlaps the rest. A single segment left is still read inline. - A read counts by how much longer it took than copying its bytes out of the page cache (at 4 bytes/ns, the low end for one thread), for both the average and the slow test. A page-cache hit of 64-256 KiB spends tens of microseconds on the copy alone, which pushed the average over 20 us and sent about 5% of a phrase query's small batches to the pool. On the 99M-row table, the phrase prefix's batches of 5-16 segments are now all read inline when hot, and the small batches of a phrase query that still went to the pool dropped by 77%. In a cold run (BE restarted, index files evicted from the page cache), the average was at least 40 us whenever such a batch was decided, so they all went to the pool, as before. | Query (99M rows) | Hot, new/previous rule | Cold, new/previous rule | |---|---|---| | phrase prefix 'retry budget dep' | 0.79 / 0.85 / 0.96 (three runs) | 0.96 | | other phrase queries | 0.87-0.95 | 1.01 (A6) | | the other 19 queries | 0.98-1.03 | | Hot: both rules interleaved in one BE process. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. |
||
|
Jack
|
c4b11b53e1 |
[fix](be) Keep BM25 statistics per index for MATCH on one column (#69901)
### What problem does this PR solve? Issue Number: None Related PR: #69790 Problem Summary: score() over MATCH clauses that read different inverted indexes of one column fails: INDEX idx_english(name) USING INVERTED PROPERTIES("parser" = "english", ...), INDEX idx_standard(name) USING INVERTED PROPERTIES("parser" = "standard", ...) SELECT id, score() FROM t WHERE name MATCH 'apple' USING ANALYZER english AND name MATCH 'apple' USING ANALYZER standard ORDER BY score() DESC LIMIT 10; -- Scoring predicates for one field select different inverted indexes BM25 statistics are keyed by field (column unique id plus suffix path), but each index of a column holds its own terms and lengths: over Apple and apple, an index that folds case has df(apple) = 2 and one that does not has df(apple) = 1. One entry cannot serve both, so the collector refuses the query. Fix: a MATCH keeps its statistics under the field and the index id. CollectInfo records the field as indexed for opening the index, since the map key no longer is one. The readers bind their source to the real field and pass the statistics name to run_leaf, so the scorers and similarities need no change: they already look statistics up by the name they are given. Each clause now scores with the statistics of its own index, and the combined score is the sum of what each clause scores alone. SEARCH keeps its own statistics names and its rejection of two indexes on one field; only MATCH changes. A MATCH and a SEARCH on one index keep separate entries with the same values. |
||
|
Jack
|
eb16c4d81f |
[improvement](be) Keep hot SNII term lookups across queries (#69879)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A SNII dictionary larger than the resident threshold (256 KB) is read block by block on demand. Every exact term lookup reads the term's DICT block (64 KB target), decompresses it with zstd, checks its CRC and parses its anchors, and the request-scoped DictBlockCache keeps nothing across queries. Every query therefore pays this again for every term in every segment. On a 99M-row AgentLogs table with 722 segments, counting one frequent term spent 68 ms of index time on these lookups against 16 ms for V3, and stack samples of postings-only and phrase queries showed 7-14% of SNII time in decompressing dictionary blocks alone. A SNII reader that the searcher cache keeps now keeps its recent exact lookups as well: a bounded LRU of up to 32 terms per reader, each at most 64 bytes long with at most 256 bytes of inline postings, which also remembers terms its dictionary turned out not to hold. `lookup()` and `lookup_batch()` check it before they read a block, so a batch reads only the blocks of the terms it does not hold. The cache's full footprint, about 25 KB, is reserved in the reader's searcher cache charge when the reader is inserted, as norms are, so the memory stays accounted. Only readers kept by the searcher cache enable it, and only when their dictionary is read on demand. The entries live inside the reader, so they go away with it and never outlive the file they describe. Caching the decoded blocks instead was not chosen: an exact lookup touches one 64 KB block per segment, so blocks would cost about 46 MB per hot term and column on that table, while a kept lookup costs a few hundred bytes. Keeping blocks in the searcher cache would also count against its fd-derived entry limit and push readers out. Prefix and pattern walks over the dictionary are not covered. Hot latency on the 99M-row table, cache off/on interleaved in one BE process: | Query | Off -> On | |---|---| | phrase 'retry budget depleted' (DS2) | 0.60 | | AND of two terms in two columns (A4) | 0.74-0.79 | | one term in two columns (A3) | 0.80 | | count of one frequent term (A2) | 0.84 | | regexp, its matched terms looked up again (DS9) | 0.89 | | phrase prefix (DS4) | 0.93 | | the other 19 queries | 0.92-1.05, within run-to-run noise | Against V3 on the same data (SNII/V3), DS2 went from 1.29 to 0.81, A4 from 1.24 to 0.95, A3 from 1.21 to 1.01 and A2 from 1.07 to 0.97. |
||
|
Jack
|
12f49adad2 |
[improvement](be) Add sorted docids to roaring bitmaps a container at a time (#69895)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: An equality filter on an indexed string column, such as `tenant = 'x'`, collects its whole posting list into a roaring bitmap, and a value can cover tens of percent of the table. `RoaringDocIdSink::append_sorted` gets those documents in sorted windows that mostly fall in one 64K container, but `Roaring::addMany` goes through the container's type for every document. On a 99M-row table that was about 20% of the SNII stack samples of a filtered phrase prefix query. `append_sorted` now splits a span at container boundaries and fills each container directly: - A bitset container gets one write per 64-bit word, with the bits gathered in a register, because neighbouring documents mostly share a word and a store per document would wait on the one before it. - An array container gets its tail appended in one loop, up to its limit of 4096. The next document turns it into a bitset, which then takes the rest. - Anything else goes through `addMany` as before: run containers, documents that fall inside an existing array, and spans that are not strictly ascending, which the sink does not rule out and a damaged posting could produce. The containers end up the same as with `addMany`. Two things to look at in review: - The sink is shared by every inverted index format, so V3 queries take the same path. - The code writes to CRoaring's container structs. `null_bitmap.cpp` already includes those headers, but only reads their sizes, so an upgrade of CRoaring has to keep this code in mind. CRoaring 2.1.2 has no public call that adds a sorted run to a container. | Query (99M rows), new/previous | SNII hot | V3 hot | SNII cold | |---|---|---|---| | phrase with tenant, app, type and status | 0.80 / 0.80 / 0.78 | 0.90 | 0.95 | | phrase with a latency filter | 0.78 / 0.77 / 0.78 | 0.78 | | | MATCH_ALL with the same four filters | 0.79 / 0.77 / 0.81 | 0.95 | 0.98 | | filters and a latency range, no text | 0.83 / 0.87 / 0.88 | 0.80 | | | phrase prefix with tenant and app | 0.93 / 0.92 / 0.92 | 0.93 | 0.98 | | top-50 with tenant and payload equalities | 0.77 | 0.82 | | | top-50 tool failures | 0.93 | 0.77 | | | the other 18 queries | 0.89-1.04 | 0.91-1.04 | | Hot: both versions interleaved in one BE process, three runs where three are shown, on a host with a load of about 100 to 140 from other builds. An earlier round at a load of 230 to 400 read up to 1.41 on some queries that do not gain; their reruns gave 0.95-1.07. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each; the change only saves CPU, so cold runs stay about level. The build also had #69879, #69882, #69887 and #69891, with `snii_inline_read_max_segments` at 16. |
||
|
Jack
|
4c1e0ddfe2 |
[improvement](be) Read and collect SNII BKD range hits in bulk (#69908)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A range filter on a numeric SNII index reads the leaves that lie wholly inside the range one at a time, gathers their doc ids, radix-sorts them and adds them to a roaring bitmap. On a 99M-row table, 71% of the SNII stack samples of a query with a wide `latency_ms` range were in this path: 15% in the sort, 9% in `addMany`, and about 27% in reading, mostly per-call overhead, since a leaf is a few KB and takes one positioned read of its own. - Whole-leaf hits lie next to each other in `bkd_data`, so one read now takes up to about 1 MiB of them. The bytes read are the same, in far fewer calls, and sequentially when they are not cached. - A wide range sets the doc ids in a flat bitmap instead, which needs no sort. It is loaded into a Roaring through the portable format, a published spec, because CRoaring 2.1.2 has no call that takes a bitset. - A flat bitmap costs a pass over all its words, so it takes doc ids only up to 256 per point, which holds it to 32 bytes per point. The doc ids decide, not the index's document count, which leaves out rows with no value: a narrow range, or a range on a column that is mostly NULL, keeps the sort. - The bitmap also stops at doc id 2^28, which is 32 MiB. The doc ids of a leaf past that take the sort as before, so a damaged leaf cannot make it allocate more. - The leaf decoder reuses its vector of value runs instead of allocating one per leaf. The on-disk format and the write path do not change. | Query (99M rows), new/previous | Hot | Cold | |---|---|---| | filters and `latency_ms BETWEEN 2000 AND 6000`, no text | 0.42 / 0.39 / 0.37 (212 ms to 85 ms) | 0.87 | | phrase with `latency_ms >= 3000` | 0.62 / 0.61 / 0.58 | 1.00 | | phrase with `latency_ms >= 8000` | 0.90 / 0.89 / 0.87 | 1.00 | | phrase with a 7-day date range | 1.01 / 0.98 / 1.00 | | Hot: both versions interleaved in one BE process on a quiet host, three runs each. The 21 queries of the bench with no range do not reach this code and read 0.86-1.02 in one run, which is the noise of that run. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each; the wide range gains and the others wait on other reads. All 25 queries return the same results with both versions. The build also had #69879, #69882, #69887, #69891 and #69895. A wide range briefly holds about three times its result in memory: the flat bitmap, the portable image and the bitmap loaded from it. That is one bit per row of the segment each, and it is freed when the range ends. The chunks of gathered doc ids it replaces took 8 MiB at most. Like those chunks, the bitmap and the image are plain vectors that the memory tracker does not see. |
||
|
Jack
|
050698f414 |
[improvement](be) Add up SNII position counts a run at a time in selective decode (#69891)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A phrase query decodes the positions of its candidate documents a window at a time. In a PFOR window the position counts of all the documents come first, and they are decoded to find where the positions of a selected document start, however few documents are selected. `decode_selected_pfor_count_ranges` went through every count of the window, with the running checks and a test for the next selected document at each one. On a 99M-row table that loop was 16% of the SNII stack samples of a phrase prefix query, nearly all of it in the loop rather than in the PFOR decode, because such a query selects no document in most runs. A run of counts is now added up whole, with no branch per document, so the compiler can vectorize it, and then walked from one selected document to the next, adding up only the counts in between. The sum of a run is kept in 64 bits and checked against the cap before any of its documents is used, so a window above the cap is rejected with the same error. The on-disk format and the write path do not change, and neither do the positions or the window shape returned. | Query (99M rows) | Hot, new/previous | Cold, new/previous | |---|---|---| | phrase prefix `'retry budget dep'` | 0.96 / 0.94 / 0.95 / 0.95 (four runs) | 0.99 | | phrase with a latency filter | 0.97 / 0.94 / 0.97 / 0.98 | 0.99 | | phrase `'unable to open'` in input or output | 0.98 / 0.96 / 0.98 / 0.99 | 0.92 | | the other 22 queries | around 1.0, no trend (0.90-1.06) | | In stack samples of the phrase prefix, with the same binary and both versions switched at run time, `decode_selected_pfor_count_ranges` fell from 16.1% to 3.4% of the SNII samples. Hot: both versions interleaved in one BE process, on a host with a load of about 100 to 150 from other builds, so differences of a few percent in a single run are noise. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. The change only saves CPU, so cold runs, which mostly wait for reads, stay level as expected. The build also had #69879, #69882 and #69887, with `snii_inline_read_max_segments` at 16. |
||
|
Jack
|
1106a37488 |
[improvement](be) Cut per-document work in SNII docid decode (#69909)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: An equality filter reads a posting list of millions of documents through `decode_window_prefix_plan`, so small costs per document, per PFOR run or per window add up. In the stack samples of a filtered phrase prefix query on a 99M-row table: - The window loop cleared its vector, so every window zero-filled it again before decoding wrote over it (4% of the SNII samples). - The sum of the docid deltas checked for a zero delta and for overflow at every document (about 7%, with the sum itself). - Each PFOR run read its exception count, and each exception its two fields, through the out-of-line varint decoders, although they are nearly always one byte (about 5%). The window loop now decodes over the vector, so a resize zero-fills only what it adds. A window's deltas are checked whole, with no branch per document, before they are summed: the same checks, and the same error code when one fails. If a window breaks more than one of them, the zero delta is reported first, whichever came first in the window. `pfor_decode` and `pfor_skip` use `get_varint32_fast`, which handles a one-byte value inline and leaves anything else to the checked decoder. The on-disk format and the write path do not change. | Query (99M rows), new/previous | Hot | Lowest run, new/previous | Cold | |---|---|---|---| | filtered phrase prefix | 0.94 / 0.96 / 0.86 | 0.95 / 0.93 / 0.93 | 0.98 | | phrase with tenant, app, type and status | 0.92 / 0.92 / 0.98 | 0.92 / 0.95 / 0.95 | 0.95 | | phrase with a latency filter | 0.95 / 0.91 / 0.99 | 0.94 / 0.92 / 0.96 | | | MATCH_ALL with the same four filters | 0.94 / 0.92 / 1.10 | 0.94 / 0.93 / 0.93 | | | top-50 with tenant and payload equalities | 0.94 / 0.94 / 0.95 | 0.95 / 0.94 / 0.99 | 1.00 | | the other queries | 0.93-1.03 | | | Hot: both versions interleaved in one BE process on a quiet host, three runs each. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 8 runs for the phrase prefix, 5 for the others. The build also had #69879, #69882, #69887, #69891 and #69895. |
||
|
Jack
|
6991e76632 |
[fix](be) Score regexp and wildcard patterns with a constant like Elasticsearch (#69815)
### What problem does this PR solve? Issue Number: None Related PR: #69052 Problem Summary: score() of pattern queries disagrees with Elasticsearch and with itself: -- 'Inverted indexes are suitable for full text search', -- 'The parser splits text into multiple terms', ... SELECT score() FROM document WHERE SEARCH('content:/.*ext/'); -- BM25 sum SELECT score() FROM document WHERE SEARCH('content:*ext'); -- BM25 sum SELECT score() FROM document WHERE SEARCH('content:te*'); -- 1 SELECT score() FROM document WHERE content MATCH_REGEXP 'ext'; -- 0 Elasticsearch scores a query_string /regexp/, wildcard or prefix, and a regexp, wildcard or prefix query, with a constant 1 per matching row (default rewrite constant_score_blended); measured on 9.5.5 with the rows above: 1.0 for both matching rows. Only an explicit "rewrite": "scoring_boolean" sums the BM25 of the expanded terms. Next to a scored clause in a bool must, a regexp adds exactly 1. #69052 made SEARCH regexp and wildcard sum the BM25 of every expanded term, i.e. scoring_boolean always on, while prefix stayed constant. That sum also takes idf and avgdl from the current segment, not from the tablet statistics MATCH uses: two inserts of the same text scored 0.2876821 and 1.3862942 for identical 'apple' rows, while MATCH 'apple' scored 0.6931471 for both. MATCH_REGEXP never requested a scoring context, so it scored 0 like a filter. Fix: 1. Revert the scoring part of #69052: ExpandWeight scores every pattern with the constant scorer again. The test fixture extensions that #69051 built on are kept. 2. MATCH_REGEXP requests scoring on every index; the regexp expansion then publishes the constant 1. The MATCH statistics collector skips it, since a constant reads no statistics and the pattern is not text to analyze. |
||
|
Jack
|
7832338fba |
[fix](fe) Pick the first matching Variant field pattern when choosing an index, like the BE (#69730)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A VARIANT column can declare predefined fields whose patterns overlap, for example the glob 'a.*' and the name 'a.b'. The BE writer (variant_util.cpp) routes a subcolumn to the first predefined field whose pattern matches its path and builds the index of that field. `OlapTable.getInvertedIndex` looped over all the fields without stopping and kept the last match, so a query on the same path chose the index of a different field than the one the data was written with, and fell back to another index or to a scan. The loop now stops at the first match, the same order the BE uses, so a write and a query on one path land on the same index. ### Release note Fix the index chosen for a Variant subpath when the column declares overlapping field patterns. ### Compatibility - FE only, and nothing is persisted: `getInvertedIndex` computes its answer from the table schema on every call. - A table whose predefined field patterns do not overlap sees no change, since a path with a single match has the same first and last match. - A table with overlapping patterns now resolves a path to the first matching field. The BE already built the index of the first matching field (`variant_util.cpp`), so the index that holds the data is the one a query now looks for, and no re-index is needed. Queries on such paths that missed the index and fell back to another index or a scan can use it. ### Performance The loop over the predefined fields now stops at the first match, so it never does more work than before. |
||
|
Jack
|
6f6a826621 |
[improvement](be) Decode SNII zstd and raw .prx windows faster (#69770)
### What problem does this PR solve? Issue Number: None Related PR: #69649, #69769 Problem Summary: A phrase query decodes the positions of the selected documents of every .prx window it reads. Timed per window on a 10M-row AgentLogs load, a zstd window cost 18.6 us against 1.4 us for a PFOR window: 11.7 us to decode the plain payload, 4.8 us to decompress, and about 2 us more because every window created and freed its own zstd context (about 96 KB) and a zero-filled output vector. Two commits: 1. **Decode the plain payload without a Status per document.** `decode_payload_csr_selective` made at least one out-of-line call returning a Status for every document of the window, selected or not (`get_varint32_fast`, `skip_varints`, `decode_delta_run`). The loop now keeps the cursor in a register and handles the usual cases inline: a single-byte count, walking past the deltas of a document that is not selected, and a run of single-byte deltas. Anything else -- a multi-byte value, a run that does not fit, a prefix sum that would overflow -- goes to the checked decoders from the same cursor, so they decide and report as before. The output vector grows as before; nothing is reserved up front. 2. **Reuse one zstd context and buffer per index source.** `ZstdDecompressor` keeps one context, created at the first zstd frame, and one output buffer that only grows and is not zero-filled (decompression writes every byte before it is read). The buffer comes from the Doris allocator (`DorisUniqueBufferPtr`), so the decoding task's memory tracker accounts for it while it is held. `SniiIndexSource` owns one, and the cursors it opens pass it to the .prx decoder through `PrxDecodeContext`. The cursors of a source decode on its thread one window at a time, and the decoded payload is consumed before the next window is read. A source lives for one query on one segment, so a query holds at most one context and one buffer per segment it reads, and only if it decodes a zstd window; before, each window created and released them. Compaction decodes through an allocation gate, which keeps the old path, as does a cursor given no decompressor. Hot queries, old against new read path in one process (run-time switch in an experiment build, 6 paired cycles), on 99M AgentLogs rows written with the smallest frames, as every segment written before #69649 is (mostly zstd windows): DS7 (unfiltered phrase) 0.87x, A6 0.89x, A10 0.93x, DS4 (phrase prefix) 0.94x, A9 0.95x, DS1 0.96x; repeating the old path against itself gave 0.93-1.05. The gain follows the zstd windows a query reads: the phrase queries of AgentLogs written with #69649 read PFOR windows and do not change, and on TextBench, whose large windows stay zstd, the time per window is the decompression itself and the change is within noise. Every query returns the same result. ### Release note None |
||
|
Jack
|
0571eb6a58 |
[improvement](be) Let each SNII index spend a size budget on faster .prx windows (#69769)
### What problem does this PR solve? Issue Number: None Related PR: #69649, #69770 Problem Summary: #69649 stores a .prx window as PFOR instead of zstd when zstd saves less than `snii_prx_zstd_min_saving_percent` (30) of the PFOR frame, because every read of a zstd window pays a decompression. What that costs and buys depends on how much zstd saves on the data. Replaying the 1.6M windows of 10M AgentLogs rows, 30 spends 8% more bytes on the windows zstd compresses and cuts their decode time by 42%; on the windows of TextBench service logs it spends 5% and cuts 6%; on data whose windows zstd shrinks by just under 30% it stores every window as PFOR. No single percentage fits all data. Each index of a segment now spends a size budget instead: `PrxZstdBudget`, owned by its `LogicalIndexWriter` and reached through `PrxWindowLimits`, so the resident and the spilled encoder, and the ordinary and the compaction build, all use it. A window whose smallest frame is zstd is stored in its faster frame (PFOR, or raw when smaller) when - its saving is below the share whose trades, over the windows the index has seen, fit 8% of their smallest frames, which puts the budget on the windows zstd shrinks least, and - the bytes the index has actually traded, this window included, still fit 8% of the smallest frames seen. This makes 8% a bound for any order of windows: fed the TextBench windows from the largest saving to the smallest, the share alone spends 36%. Until it has seen 64 windows the share is 30, the previous default. The windows counted are those that get a zstd frame (a plain payload of at least 512 bytes), the only ones whose codec a share decides; singleton and short windows are written as before and counted neither in the allowance nor in the warmup. The choice uses the frames the writer builds anyway: nothing is compressed twice or scanned ahead, and the same input gives the same bytes. `snii_prx_zstd_min_saving_percent` now defaults to -1 (the budget); 0-100 keeps a fixed share, 0 the smallest frame. The budget is 8% rather than less: with it the AgentLogs phrase queries below read nearly the same frames as with 30, while in the replay a 5% budget cuts the decode time by 31% instead of 39%. Same binary, one stream load per table, size against the smallest frames (0): | data | index, 30 | index, budget | table, 30 | table, budget | |---|---|---|---|---| | AgentLogs 99M rows | +4.13% | +3.65% | +1.92% | +1.69% | | AgentLogs 10M rows | +4.59% | +3.37% | +2.13% | +1.57% | | TextBench 12M rows | +0.89% | +0.79% | +0.51% | +0.45% | - The phrase queries read the same frames with 30 and with the budget: at 99M rows all PFOR for DS1, DS6, DS7, A6 and A9 (for example 107,307 frames for DS7), and about 11,170 zstd frames of about 259,000 for A10 with both; on TextBench most windows stay zstd either way (582 against 583 zstd frames of 599 for 'connection reset'). - Hot queries in one process, budget data against 30 data with the same read path (paired cycles: 6 for the 25 AgentLogs queries at 10M rows, 10 for the 15 TextBench queries): 0.96-1.06 and 0.97-1.05; every query returns the same result. - Against the data written before #69649 (smallest frames), the 11 AgentLogs phrase queries at 99M rows take 0.83x in geometric mean with the budget data (DS7 0.63x, A6 0.67x, A9 0.70x, DS4 0.74x, A10 0.77x), as with 30 (0.83x). This run also had the read-path change of #69770, which does not touch the PFOR windows these queries read. - Data bytes are identical, and the index written with 0 and with 30 has exactly the size the previous binary writes. - Write cost: the choice adds about 48 ns per window, 79 ms over the 1.6M AgentLogs windows. ### Release note SNII now chooses per index how many position windows to store in the faster but larger PFOR form, spending at most 8% of the size of the windows zstd compresses; `snii_prx_zstd_min_saving_percent` defaults to -1 for this, and 0-100 still sets a fixed percentage. |
||
|
Jack
|
24dc7762f1 |
[improvement](be) Account the NULL docids and point buffer of the SNII BKD writer (#69728)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: `SniiBkdIndexColumnWriter`, the numeric, date and IP index writer, installed no memory reporter. Its point buffer, and its NULL docid vector, which grows with the row count, were invisible to the process-wide SNII build tracker (the `SniiIndexBuild` MemTracker and the `snii_index_build_mem_consumption` gauge) that the text writer feeds. The writer now creates a reporter in the unregistered population, since a forced spill cannot reclaim a BKD build, and hands it to the BkdBuilder. It charges the NULL docids through the growth routine the text writer already used, which is extracted to `reserve_docids_for_append` next to `MemoryReporter` so that the two writers share it. It gives the vector and its charge back as soon as `finish()` has moved the docids into the bitmap, because IndexBuilder keeps finished writers alive until the whole rowset is closed. This only adds accounting. The reporter has no cap, so it cannot fail a build, and its callback charges a plain MemTracker, not a limiter. ### Release note The SNII index build memory metric now also counts the point buffers and NULL rows of numeric, date and IP indexes. ### Compatibility - No on-disk format change and no reader change: the BKD writer writes the same bytes as before. - The reporter only mirrors byte counts into the `SniiIndexBuild` MemTracker and the `snii_index_build_mem_consumption` gauge through a callback. It has no cap, so a reservation cannot fail (only an int64 overflow of the counter could), and the callback charges a plain MemTracker, never a MemTrackerLimiter, which would count the same bytes twice because the allocator hook already attributes them. So a build cannot fail where it passed before. - Nothing is exchanged between nodes, so mixed versions are not affected. The text writer uses the extracted helper with the same behavior: the larger capacity is charged before the allocation and the old charge is returned after the move. ### Performance - Only the capacity check of `reserve_docids_for_append` is inline in its callers. The growth step is out of line (`NO_INLINE`), since it runs O(log n) times per writer. Compiled with `-O2` and `-O3`, the text writer's `_reserve_null_docids_for_append` goes from about 200 instructions, with a six-register prologue on every call, to about 10 instructions with no prologue when the capacity is enough. An earlier version of the helper, with the growth step inline, was not inlined into its callers and cost the text writer one extra call frame per call, about 2% in the first row of the benchmark below. - Appending NULL docids in runs of random length, 100 million rows, best of 5 repetitions per process, interleaved, one pinned core. `plain` is the BKD writer before this change, `before` the text writer's routine before, `after` the shared helper: | docids per call | plain | before | after | after / before | |---|---|---|---|---| | 1 to 20 | 60.3 ms | 51.8 ms | 50.5 ms | 0.976 | | 1 to 200 | 217.9 ms | 244.7 ms | 251.0 ms | 1.026 | | 1 to 20000 | 363.2 ms | 325.9 ms | 320.1 ms | 0.982 | The text writer is unchanged within the noise of this machine (medians vary by up to 7%). The BKD writer pays for the accounting only in the middle row, 33 ms for 50 million NULL docids, about 0.7 ns each; in the other two rows it is faster than before. |
||
|
Jack
|
31a2697f21 |
[improvement](be) Run-optimize the SNII NULL bitmap before serializing it (#69727)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: `NullBitmapWriter` never ran `runOptimize()` before serializing the NULL bitmap, while the V2 inverted index writer does. Runs of NULL rows, which is the usual shape of a path that is absent from most rows of a sparse VARIANT column, were stored as plain array or bitset containers: up to 8 KiB per 65536 rows instead of a few bytes, once per index. `serialization_sizes()` now optimizes first, and `finish()` sizes itself through it, so the sizes that callers reserve against and the bytes written still agree. For a 20-million-row segment, an all-NULL bitmap goes from 2.5 MB to 4.3 KB, and a bitmap of alternating NULL and non-NULL runs of 1 to 5000 rows goes from 2.5 MB to 19.8 KB. A random bitmap is unchanged. Opening the all-NULL section goes from 1.04 ms to 0.01 ms. The cost is one pass over the containers per call, about 0.5 ms for a random 20-million-row bitmap, and it is paid twice per index at finish. ### Release note SNII indexes on columns with long runs of NULL rows take less space. ### Compatibility - The bytes are the standard Roaring portable serialization, as before. The only difference is that a container may now be a run container (cookie 12347), which the V2 writer's NULL bitmaps have always been allowed to contain. - A reader needs nothing new. `parse_null_bitmap` already accepts both portable cookies, `decoded_memory_bytes` already sizes `run_container_t`, and the SNII readers call `runOptimize()` on the in-memory copy after loading, so a reader holds the same structure whether or not the file had runs. `null_bitmap.cpp` and `null_bitmap.h` are identical on master and on the branch-4.2 tip, so a released BE reads files written with this change, and the change backports without conflicts. - Sizing: `serialization_sizes()` optimizes before it measures and `finish()` sizes itself through it, so the `DORIS_CHECK_EQ` between the sizes and the bytes written holds. The new unit test serializes runs through both and reads them back. ### Performance Offline benchmark of the real `null_bitmap.cpp` (`-O3`, one pinned core, 20-million-row bitmap, best of 3 interleaved rounds): | shape | bytes old -> new | `serialization_sizes` (new) | `finish` old -> new | reader `open` old -> new | `copy_to` old -> new | `col IS NOT NULL` row set (and-not) old -> new | |---|---|---|---|---|---|---| | all NULL | 2,509,225 -> 4,340 | 0.68 ms | 2.53 -> 0.02 ms | 1.04 -> 0.01 ms | 0.48 -> 0.01 ms | 0.41 -> 0.02 ms | | alternating runs of 1 to 5000 rows | 2,509,225 -> 19,810 | 0.77 ms | 2.49 -> 0.05 ms | 1.04 -> 0.03 ms | 0.48 -> 0.02 ms | 0.73 -> 0.03 ms | | random 1% | 402,379 (same) | 0.05 ms | 0.37 -> 0.42 ms | 0.18 -> 0.20 ms | 0.09 -> 0.09 ms | 0.33 -> 0.39 ms | | random 10% | 2,503,417 (same) | 0.45 ms | 2.39 -> 2.87 ms | 1.03 -> 1.11 ms | 0.48 -> 0.49 ms | 0.74 -> 0.70 ms | | random 50% | 2,509,225 (same) | 0.48 ms | 2.50 -> 2.98 ms | 1.04 -> 1.11 ms | 0.48 -> 0.50 ms | 0.74 -> 0.71 ms | - Write side: one pass over the containers per `serialization_sizes()` call, 0.05 to 0.8 ms for 20 million rows. It runs twice per index at finish: `LogicalIndexWriter::finalize_build` sizes the section to reserve memory for it, then `finish()` sizes it again. For a bitmap with runs the second pass has nothing left to convert (`finish` above is 0.02 to 0.05 ms); for a random one it repeats the scan. On a random bitmap, where there are no runs to find, that is the whole effect: about +0.5 ms per pass, per index and segment. - Read side: no change for bitmaps without runs. Bitmaps with runs are smaller to read from disk and cheaper to open and combine. - One operation is slower on run containers: `NullBitmapReader::is_null` (1,000,000 random probes, 48.9 -> 77.0 ms for the alternating shape, since a run container is searched, not indexed). It has no production caller; the readers use `copy_to` and `append_docids`. |
||
|
Jack
|
70cdb58e94 |
[fix](be) Reject overflowing lengths and indices when decoding SNII integers (#69726)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Three decode primitives of the SNII index format take numbers read from the file and do arithmetic on them without checking for overflow. - The FRQ prelude added the untrusted `sbdir_len` to the header length before comparing it with the buffer, and summed the super-block lengths into the window region length unchecked. A crafted length wraps the sum to a small value, so the bounds check passes while a Slice of a huge length is cut from a buffer that holds no such bytes, and the block decoder then reads past the end of it. `memory_required` already compared `sbdir_len` with the remaining bytes before calling `verify_covered_crc`, but `open` did not. The overflow check now lives in `verify_covered_crc`, which both call, the super-block sum uses the file's own `checked_add_u64`, and the final region check compares against the room left instead of adding to the start offset. - `decode_varint64` accepted a tenth byte with payload above bit 63. The extra bits were shifted out, and the encoding decoded to a silently truncated value. The check sits on the terminating byte, so the loop over the continuation bytes is unchanged. - `pfor_decode` and `pfor_skip` added each exception index delta to a 32-bit running index and tested the sum afterwards, so a delta could wrap the index back into the block and be accepted. The running index is now 64 bits wide: the index is below the block size before every add and a delta is at most 2^32 - 1, so the sum cannot wrap. Nothing here is reachable from data the writers produce. The checks make a damaged or crafted index file fail with a corruption status instead of being decoded into a wrong value or an out-of-bounds read. |
||
|
Jack
|
a47d4613d2 |
[improvement](be) Read small SNII batches on the calling thread when reads hit the page cache (#69648)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: DorisSniiFileReader::read_batch submits every physical segment of a batch with more than one segment to the shared buffered-reader prefetch thread pool and waits on a latch. When the index file is in the page cache the reads take microseconds and 99% of the batches of a phrase workload are under 256 KB, so the submit, wake-up and wait cost more than the reads, and a query that issues thousands of batches from 16 scanner threads queues on the pool's lock. On 99M AgentLogs rows, 83% of the threads inside the SNII query path were blocked in ThreadPool::do_submit for the date-range phrase query. A batch of at most config::snii_inline_read_max_segments (default 4, 0 disables) segments and 256 KiB is now read on the calling thread when the index file is on the local filesystem and the moving average of recent small physical reads is below 20 us. Remote and cached-remote readers, larger batches and slow (page-cache miss) reads still fan out to the pool, because overlapping disk reads gain from it: with a fixed rule instead of the average, the cold run of two phrase queries was 4-8% slower. The serial read rounds counter counts one round per segment for a batch read inline. Measured on 99M AgentLogs rows (branch-4.2 build; the changed code is the same on master), same process, 25 queries, inline off against on, 16 samples each: date-range phrase query 0.39x, any-of-three-phrases top-N 0.70x, no query above 1.02x. Cold runs (restart, page cache evicted, three repetitions) of eight queries are within 2% of the pool. |
||
|
Jack
|
21f37fe182 |
[fix](inverted index) Score MATCH_PHRASE_PREFIX on the tablet idf of the terms the prefix expands to (#69698)
### What problem does this PR solve? Issue Number: None Related PR: #68758 Problem Summary: score() over MATCH_PHRASE_PREFIX gave scores that ignored the prefix and differed between inverted index storage formats. 1. The prefix of a phrase prefix never entered the idf. "apple r" scored on idf(apple) alone, and a rare expansion such as "rose" ranked like a common one such as "red". Lucene's multi-phrase query, and so Elasticsearch, sum the idf of the phrase terms and of every term the prefix expands to. The statistics collector now registers the prefix; every segment expands it, counts the document frequency of each expanded term in the tablet statistics, and records which terms the prefix expanded to. The scorer adds their idf to the phrase's, so all idfs come from the same tablet statistics and the score does not depend on how rows are split into segments. With the reported table, `apple r` now scores 1.4508328, the value Elasticsearch 9.5 gives; on a 10-row table, `apple r`, `apple ro` and `apple g` match Elasticsearch to four decimals, over one segment and over three. 2. A prefix of one token scored 0 on V2/V3 (CLucene did not publish scores for expansion leaves) and 1 on SNII. It now scores 1 on every format, as the constant score of Lucene and Elasticsearch prefix queries. At 0, `score() > 0` dropped every row the prefix matched. 3. No test covered score() over MATCH_PHRASE_PREFIX across storage formats. Add unit tests for the collector, the segment expansion, the tablet idf and the constant score, and a regression suite over V2, V3 and SNII. Two existing tests asserted the old behavior and are updated: the SNII public phrase prefix scoring test (the idf now counts the expanded term) and the CLucene one-token phrase prefix test (it asserted no score). A prefix collected by a SEARCH predicate has no recorded expansion and keeps scoring the phrase terms alone. |
||
|
Jack
|
428c24187c |
[improvement](be) Let SNII skip zstd for .prx windows where it saves little over PFOR (#69649)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: The auto .prx codec stores each position window in the smallest of PFOR, zstd and raw. Every read of a zstd window pays a decompression that PFOR does not, and the choice ignores that cost. On 99M AgentLogs rows 74% of the position windows read by an unfiltered phrase query were zstd, and decoding them took 58% of the index execution time of the query. PrxWindowLimits gets zstd_min_saving_percent, set from the new mutable config snii_prx_zstd_min_saving_percent. A readable PFOR frame is replaced by the zstd one only if zstd is smaller by at least that percentage of the PFOR frame. The default is 30; 0 picks the smaller frame as before and writes a byte-identical index (99M rows: 61,886,854,661 bytes with and without this change). Readers handle every codec, so the setting affects only segments written afterwards, and compaction rewrites with whatever is set then. The compression is still computed, so the write cost does not change. Effect of the setting, 10M rows (index size, DS7 = unfiltered phrase on the output column, position decode time from the query profile): 10 -> +0.02% size, no change for DS7; 20 -> +0.5%, no change; 30 -> +2.1%, decode 252 ms -> 85 ms; 50 -> +6.3%, no further gain; 100 (never zstd) -> +8.5%. 30 is the smallest value that removes the zstd windows of frequent terms, which zstd beats PFOR by 20-30% on and which phrase queries read most. On 99M rows with 30: index +1.9% (63.07 GB against 61.89 GB, still below the 65.70 GB of V3), load time within 2% of a control load, hot queries in the same process 0.63x (unfiltered phrase), 0.67x (phrase in input or output), 0.75x (phrase prefix), 0.77x, 0.87x, the others within 1.02x (one query measured 1.06x once and 1.00-1.05x in three repeats); cold runs 10-18% faster for the phrase queries. The best value depends on the data, since it is how much zstd beats PFOR on the window contents; it was calibrated on one corpus. |
||
|
Jack
|
655b46eb3e |
[fix](be) Preserve SNII NULL-docid memory accounting (#66880)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: SNII released the ingestion writer NULL-docid memory charge before flush, then copied the still-live vector into `LogicalIndexWriter` without a `Reservation`. A one-million-NULL segment retained a 4 MiB docid vector while the build reporter observed only about 674 KiB of bitmap scratch, and the handoff briefly held two complete vectors. This PR carries the existing move-only vector and `Reservation` through `IndexFileWriter` and `SniiCompoundWriter`, precharges geometric vector replacements during growth, and releases the charge only after the NULL bitmap consumes the docids. A real one-million-NULL compound build verifies reporter current/peak bytes and terminal zero balance. |
||
|
Jack
|
5964f9f263 |
[fix](fe) Run an ALTER that also needs heavy work as a heavy schema change instead of a light index change (#69444)
### What problem does this PR solve? Issue Number: None Related PR: #68674 Problem Summary: When an ALTER TABLE statement combines an index change that can run on the light index change path with a clause that needs a heavy schema change, the light path wins and the heavy work is never done: ```sql -- v was INT, idx is an INVERTED index: the shadow column __doris_shadow_v BIGINT is published and the -- schema change job is recorded as FINISHED, but no BE conversion job runs ALTER TABLE t MODIFY COLUMN v BIGINT NULL, DROP INDEX idx; ALTER TABLE t MODIFY COLUMN v BIGINT NULL, ADD INDEX i2 (x) USING INVERTED; -- enable_add_index_for_new_data = false: bf_v2 must be built for the existing data, but it is only -- published for new data ALTER TABLE t ADD INDEX bf_v2 (v2) USING BLOOMFILTER, DROP INDEX idx; ``` The same holds for every column or property clause that cannot run as a light schema change (dropping a key column, reordering columns, sequence mapping tables, table properties). On a changelog table such a statement also bypassed the check that only light changes are allowed, because that check looked at the candidate flag `lightIndexChange`. Root cause: in `SchemaChangeHandler#process` every clause that is not light sets `lightSchemaChange = false`, index clauses included, so nothing records that a clause needs a heavy schema change. The dispatch then takes the light index change branch whenever an index clause can be light. Fix: the clause loop also records `requiresHeavyChange` for column and property clauses that cannot be light and for an `ADD INDEX` that cannot be light. After the loop, the light index change path is only used when no clause needs heavy work; otherwise the statement runs as a heavy schema change job (`createJob`), which applies the index changes as well, exactly as it does when `enable_light_index_change` is off. The changelog check uses the path that is actually taken. No statement is newly rejected (other than on changelog tables, where the heavy path was never allowed). Notes: - A `DROP INDEX` without light support still needs no heavy work: in a light index change only its metadata is dropped (same-statement NGRAM_BF to BLOOMFILTER replacement in local mode keeps working). - The light DROP INDEX pre-check from #68674 still runs before the clause loop, because whether a clause needs heavy work is only known after it is processed. In cloud mode it therefore still requires a compute group for such a statement, as before this change. |
||
 JackandClaude Opus 4.8
|
72738b4dc3 |
[fix](test) run search score TopN case in nonConcurrent group (#67402)
Related PR: #67363 (same fix on branch-4.1), #67327 (backported the suite to branch-4.1), #65821 (introduced the suite on master) Problem Summary: `test_search_score_topn_predicates` (added on master by #65821) runs in the shared **concurrent** P0 group. On branch-4.1 it fails intermittently on its very first query, right after `CREATE TABLE` + `INSERT` + `sync`, with: ``` errCode = 2, detailMessage = [INTERNAL_ERROR]SearchExpr should not be executed without inverted index ``` This was observed on branch-4.1 CI (2 failures out of 4 runs) and fixed there by #67363, which moves the case into the `nonConcurrent` group. master carries the **identical** suite grouping (`"p0"`), so it is exposed to the same intermittent failure when the case races other P0 cases. This PR applies the same one-line fix to master to keep it out of the concurrent group. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Jack
|
dbdd07dc7e |
[fix](index) Enforce SNII compatibility corpus and simplify comments (#68607)
### What problem does this PR solve? Related PR: #67537, #68584, #67657, #68758, #69192 Problem Summary: The SNII golden corpus test skipped unless `SNII_GOLDEN_DIR` was supplied, leaving existing wire-format compatibility checks out of ordinary BE unit-test runs. This PR bundles eight synthetic index/expectation pairs and verifies them by default across disabled, cold, and warm query-cache modes. CMake supplies the bundled corpus directory to the test source. This lets the test find the files when `run-be-ut.sh` resets `DORIS_HOME` to the test binary directory. `SNII_GOLDEN_DIR` still selects an external corpus or an explicit write destination. Write mode requires that override and adds the Apache license header to generated expectations. The expectations follow the current shared query engine: analyzed multiword equality intersects its terms, and phrase queries requiring positions on a docs-only index return `NOT_IMPLEMENTED_ERROR (3)`. The original eight index images remain unchanged. The remaining source and protobuf edits simplify comments and rename one metadata test without changing its assertions. A token comparison excluding comments and whitespace confirms that the production and protobuf changes do not alter behavior. The branch includes the upstream IColumn writer API migration. |
||
|
Jack
|
6979f7fa1e |
[improvement](be) Skip dictionary entry bodies of non-matching terms in SNII term expansion (#69600)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: MATCH_REGEXP, wildcard, suffix and contains patterns without a literal prefix make SniiIndexSource::expand_terms enumerate the whole dictionary of every segment. LogicalIndexReader::visit_prefix_terms always passed an empty accept_key to DictBlockReader::visit_prefix_range, so the body of every dictionary entry was decoded (decode_dict_entry_rest) and the caller then rejected almost all of them with the required-text test and the pattern match. DictBlockReader already supports a key-only accept_key that skips the body of a rejected term. visit_prefix_terms now takes an optional accept_key, and expand_terms passes its term test (required text, then pattern match) as that predicate. Terms are tested exactly as before, only earlier, and the visitor still sees only accepted terms, so max_expansions and early stop count the same terms. Other callers keep the default (accept all). Measured on 99M AgentLogs rows (722 segments, tenant and status filters, token regexp 'transi.*', hot median): SNII 6028 ms -> 945 ms, V3 unchanged at about 590 ms. Result counts are identical to V3 for regexp, leading-wildcard regexp and phrase-prefix queries. Patterns with a literal prefix (for example '^transi.*') seek into the dictionary and are unchanged: SNII and V3 are level at about 100 ms before and after. Gdb sampling showed 58% of the time in decode_dict_entry_rest before and about 5% after. |
||
|
Jack
|
adecd627ab |
[fix](fe) Reject DROP INDEX ... PARTITION combined with other ALTER clauses (#69442)
### What problem does this PR solve? Issue Number: None Related PR: #68674 Problem Summary: `ALTER TABLE ... DROP INDEX idx PARTITION (p)` only submits partition scoped index jobs and keeps the index in the table metadata. When it is combined with other clauses in one statement, part of the statement is silently not executed or the index files are deleted on the wrong partitions: ```sql -- returns success, vi is still INT: the partition scoped index job path ignores every other clause ALTER TABLE t MODIFY COLUMN vi BIGINT NULL, DROP INDEX idx_v1 PARTITION (p1); -- returns success, x is not added ALTER TABLE t ADD COLUMN x INT, DROP INDEX idx_v1 PARTITION (p1); -- the full-table drop selects the light index path, which deletes idx_v1 on every partition -- while the table metadata still lists idx_v1 ALTER TABLE t DROP INDEX idx_v1 PARTITION (p1), DROP INDEX idx_v2; -- the partitions of both clauses are merged and every job carries both indexes, -- so idx_v1 and idx_v2 are deleted on p1 and p2 ALTER TABLE t DROP INDEX idx_v1 PARTITION (p1), DROP INDEX idx_v2 PARTITION (p2); ``` Root cause: `SchemaChangeHandler#process` records a partition scoped drop with `buildIndexChange`, whose dispatch branch only calls `buildOrDeleteTableInvertedIndices` and ignores the other clauses; the light index path does not carry the partition targets; and `buildOrDeleteTableInvertedIndices` matches every partition of any clause and gives each job all altered indexes. `BUILD INDEX` is a standalone statement, so `DROP INDEX ... PARTITION` is the only way to mix partition scoped index jobs with other clauses. Fix: `process()` rejects a statement that contains `DROP INDEX ... PARTITION` together with any other clause, before any clause is processed, with `DROP INDEX on partitions can not be combined with other clauses in one ALTER statement, please run them in separate statements.` The check only looks at the clause types, so it runs before the clause loop and a rejected statement leaves the table unchanged. A partition scoped drop alone keeps its behavior. |
||
|
Jack
|
8ebb90998a |
[fix](fe) Treat an unfinished index change job of the same index as a conflict in either direction (#69443)
Related PR: #68674 Problem Summary: In local (non-cloud) mode, an unfinished index change job only conflicts with a new one on the same partition when both go in the same direction. An unfinished `BUILD INDEX idx` therefore does not block `DROP INDEX idx` (full-table or `PARTITION`), and an unfinished `DROP INDEX idx PARTITION (p)` does not block `BUILD INDEX idx` on `p`: ```sql BUILD INDEX idx_v1 ON t; -- the job is still running ALTER TABLE t DROP INDEX idx_v1; -- accepted ``` The two jobs are submitted to the schema change thread pool independently, and the BE tablet locks only make them mutually exclusive, not ordered. If the build runs after the drop, it writes `idx_v1` back into the rowsets, although the table metadata no longer has that index (or, for the opposite order, the index the user just built is deleted again). Cloud mode already treats any unfinished job on the partition as a conflict. Root cause: `SchemaChangeHandler#hasIndexChangeJobOnPartition` matches existing jobs in local mode with `IndexChangeJob#hasSameAlterInvertedIndex(isDrop, indexes)`, which only reports a job of the same direction. Fix: `IndexChangeJob#hasAlterInvertedIndex(indexes)` matches a job that builds or drops any of the indexes, in either direction, and `hasIndexChangeJobOnPartition` drops its direction parameter. The light `DROP INDEX` pre-check, `DROP INDEX ... PARTITION` and `BUILD INDEX` all use this check, so each of them is rejected while a job of the same index is unfinished on the partition, with the existing error message; it can be retried once the job finishes. ### Release note In non-cloud mode, `DROP INDEX` and `BUILD INDEX` are rejected while an index change job of the same index in the other direction is unfinished on the partition, instead of running both jobs in no particular order. |
||
|
Jack
|
f1fd1b432f |
[improvement](fe) Scan index change jobs once in the light DROP INDEX pre-check (#69441)
### What problem does this PR solve? Issue Number: None Related PR: #68674 Problem Summary: The light `DROP INDEX` pre-check added in #68674 (`SchemaChangeHandler#checkNoIndexChangeJobOnPartitions`) calls `hasIndexChangeJobOnPartition` once per partition, and every call copies and scans the whole retained `indexChangeJobs` history (finished jobs are kept for days). For a table with P partitions and a history of H jobs that is O(P * H) work while the table write lock is held, on top of the existing per-partition check in `buildOrDeleteTableInvertedIndices`. A recent `BUILD INDEX` on a table with many partitions leaves at least one record per partition, which makes it quadratic in the number of partitions. The pre-check also reported a conflict with the `BUILD INDEX` wording `partition xxx has been built specified index. please check your build stmt.`, which is misleading for a `DROP INDEX`. Fix: the pre-check takes one snapshot of the job history and scans it once, matching jobs on any of the table's partitions. The match condition is extracted into `isConflictingIndexChangeJob` and shared with `hasIndexChangeJobOnPartition`, so both checks keep the same rule. The pre-check reports `partition xxx of table t has an unfinished index change job, please retry after it finishes.` The `BUILD INDEX` / `DROP INDEX ... PARTITION` check and its message are unchanged. |
||
|
Jack
|
2524893ea3 |
[fix](nereids) Support minimum score filters without TopN (#69292)
### What problem does this PR solve? Issue Number: N/A Related PR: #69399 (merged; the plan copier fix this PR needs), #69419 (merged; fixes the pre-existing MATCH predicate inference that review of this PR found) Problem Summary: `score()` only worked as `ORDER BY score() LIMIT n` directly above the scan whose MATCH defines it. A minimum score filter without that TopN evaluated the unbound `score()` placeholder as zero and silently dropped every row: ```sql SELECT id FROM documents WHERE content MATCH 'apple' AND score() > 0.00001; -- returned nothing ``` `score()` is computed by the olap scan that evaluates the MATCH and SEARCH predicates of its SELECT. This PR binds `score()` to that scan at the start of rewrite, makes the scan's expressions read the score, and rejects every other use with an error that names the rule. **Supported.** A SELECT that reads one table and has a MATCH or SEARCH predicate in its WHERE clause can use `score()`: - in the WHERE clause, as one minimum score: `score() > x` or `score() >= x` (also `x < score()`, `x <= score()`, and with a FLOAT or DOUBLE cast); - in the select list, window functions and ORDER BY, if the SELECT does not aggregate (with GROUP BY, including GROUPING SETS, ROLLUP and CUBE, aggregate functions or DISTINCT, `score()` can only be the minimum score). A scored SELECT in a subquery, view, CTE or set operation still needs `ORDER BY score() LIMIT n`, as before. The TopN keeps the predicates of the outer query out of its scan, e.g. an outer MATCH cannot add to its score. A correlated subquery cannot call `score()`, because decorrelation removes that TopN. **Rejected.** A join, a subquery or a lateral view in the scored SELECT; a view, CTE or other relation that is not an olap table (a table stream fails during planning, because its scan is replaced by scans of the table and of its changes); any other filter on the score (`score() < x`, `score() = x`, lossy casts, ORs, a second bound); `score()` in aggregates, GROUP BY, HAVING or QUALIFY; MATCH or SEARCH in HAVING or QUALIFY of a scored SELECT, also through an alias; `score()` in a correlated subquery; a SELECT without its own MATCH or SEARCH; a nested scored SELECT without the TopN. **How.** - BE: the scan reads the score in its own expressions. `SegmentIterator` prepares the score of every row of its row bitmap before the first batch. It marked only stored columns of a pushed-down expression as common-expression columns, so an expression on the score alone, e.g. `score() > 0.5`, was never evaluated (and the scanner had already removed it from its conjuncts), and an expression that read the score next to other columns, e.g. a TopN filter on `CAST(score() AS INT)`, evaluated `Score()` itself, which returns 0. Now the score column is read with the other common-expression columns, before the expressions run. - FE, binding: `BindScoreToOlapScan` replaces `CheckScoreUsage`. It runs first in rewrite, while subqueries, views and CTEs still have their own plan nodes, so the analysis classes are as on master. For each SELECT that calls `score()`, it checks the rules above, adds a score column to the scan (score limit 0: the scan scores every row it returns) and replaces the `score()` calls of the SELECT with that column. A table is a relation, also under the filters analysis puts on it (the delete sign filter of a UNIQUE table, row policies). Binding after analysis also covers `* REPLACE (score() AS c)`, alias functions over `score()` and `ORDER BY` a position, and a UDF named `score` is never mistaken for it. - FE, rewrite: the minimum score is an ordinary predicate of the scan. `PushDownScoreTopNIntoOlapScan` pushes a TopN on the score into the scan, and with it the minimum score, exactly as before this PR (with a zero score limit when SEARCH has another predicate), because the scan keeps the best rows of a segment only when no predicate follows. TopN filter pushdown and MV rewrite need no `score()` cases: a TopN filter reads the score like any other column, and a materialized view without the score cannot answer a predicate on it. - This PR needs the plan copier fix of #69399, which is merged: copying a scan with virtual columns gave each one two ids and kept the old slot in the output. - The BE keeps the score unchanged by MATCH projections: `SegmentIterator` evaluates them without the score collector. - `SearchExpression` can be rendered as SQL, which the shape plan logged at debug level and `EXPLAIN SHAPE PLAN` need. ### Release note `score()` works in the select list, window functions and ORDER BY of a SELECT on one table whose WHERE clause has MATCH or SEARCH, with an optional minimum score `score() > x` or `score() >= x`, without `ORDER BY score() LIMIT`. A scored SELECT in a subquery, view, CTE or set operation still needs `ORDER BY score() LIMIT`. Other uses of `score()` are rejected instead of evaluating it as 0. |
||
|
JackandClaude Opus 5.5
|
709fe0a7b0 |
[fix](nereids) Do not infer MATCH and SEARCH predicates across column equalities (#69419)
Related PR: #69292 (where review found it) Problem Summary: Predicate inference copies a predicate on one column to every column that is equal to it. When the predicate contains MATCH, the copy changes the result: ```sql CREATE TABLE t (id INT, c1 TEXT, c2 TEXT, INDEX i1 (c1) USING INVERTED PROPERTIES ("parser" = "english"), INDEX i2 (c2) USING INVERTED) DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES ("replication_num" = "1"); INSERT INTO t VALUES (1, 'apple red', 'apple red'), (2, 'banana', 'banana'); SELECT id FROM t WHERE c1 = c2 AND (c1 MATCH_ANY 'apple' OR c1 MATCH_ANY 'pear'); -- no rows; row 1 is expected ``` `InferPredicateByReplace` added `(c2 MATCH_ANY 'apple' OR c2 MATCH_ANY 'pear')` to the scan. A MATCH depends on the inverted index of its column: `c2` has no parser, so a MATCH on `c2` only matches a whole value, and the copy dropped row 1. The same happens when the MATCH is inside a comparison, e.g. `CAST((c1 MATCH_ANY 'apple' OR c1 MATCH_ANY 'pear') AS INT) > 0`, and with `multi_match(c1, 'any', 'apple')` and `search()`, which also run through the inverted index of their column. When both columns have the same index, the copy is scored too. With `'apple red'` and `'apple green'` in both columns, `score()` of the same filter with `ORDER BY score() DESC LIMIT 10` doubles from 0.4700036 to 0.9400072. Adding `score() > 0.6` returns both rows, although their score is below the minimum. `InferPredicateByReplace.infer` now skips every input that contains `Match`, `MultiMatch`, `Search` or `SearchExpression`, whatever it is nested in, next to its existing `NoneMovableFunction` and non-deterministic checks. Their result depends on the inverted index of their column, not only on its value, so their column must not be replaced by an equal one. Other predicates next to them are still inferred. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Jack
|
2fb774e8cd |
[fix](fe) Check conflicting index change jobs before dropping index metadata (#68674)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: On the light index change path, `DROP INDEX` can fail with `partition xxx has been built specified index` when an unfinished IndexChangeJob conflicts on the same partition. Although the statement returns an error, the index metadata has already been removed and `SHOW INDEX` no longer lists it, so the client cannot tell whether the DDL took effect. Reproduce: - Cloud mode (any unfinished index change job on the partition conflicts): create a table, then quickly run `CREATE INDEX iv ... USING INVERTED; DROP INDEX iv; CREATE INDEX ng ... USING NGRAM_BF; DROP INDEX ng;`. The last `DROP INDEX` fails, yet `SHOW INDEX` is empty afterwards. - Local mode (only a job for the same index and direction conflicts): `ALTER TABLE t DROP INDEX idx PARTITION (p1)` and, while that job is unfinished, `ALTER TABLE t DROP INDEX idx`. Root cause: in `SchemaChangeHandler#modifyTableLightSchemaChange`, a light `DROP INDEX` first calls `updateBaseIndexSchema` and writes the edit log, and only afterwards submits its delete jobs in `buildOrDeleteTableInvertedIndices`, which runs the conflict check (`hasIndexChangeJobOnPartition`) and, in cloud mode, the compute group lookup (`getCloudClusterName`). When either throws, the metadata change is not rolled back. Fix: `SchemaChangeHandler#process` runs both checks for every full-table `DROP INDEX` that takes the light index change path before any ALTER clause is processed, so a rejected statement leaves the table unchanged. They have to run before the clause loop rather than just before `updateBaseIndexSchema`, because clauses such as `DROP COLUMN` update table metadata in place (the bloom filter columns) while they are processed. The light-drop eligibility rule is extracted into `isLightDropIndexSupported` and shared by the pre-check and the clause loop. The conflict check reuses `hasIndexChangeJobOnPartition` with the existing error message; the later check in `buildOrDeleteTableInvertedIndices` is unchanged. The same pre-check also rejects a light `ADD INDEX` and a light full-table `DROP INDEX` in one statement (`Can not add and drop indexes in one ALTER statement, please run them in separate statements.`). The light path builds the delete jobs of all altered indexes with one direction flag decided by the last index clause, so `DROP INDEX old, ADD INDEX new` left the files of `old` behind and `ADD INDEX new, DROP INDEX old` also deleted the files of `new`. Only the case where both clauses take the light path is rejected; a statement whose ADD or DROP is not light keeps its behavior (e.g. replacing NGRAM_BF with BLOOMFILTER in local mode). |
||
|
Jack
|
88511083a6 |
[fix](nereids) Fix deep copy of scans with virtual columns (#69399)
### What problem does this PR solve? Issue Number: N/A Related PR: #69292 (depends on this fix) Problem Summary: A CTE fails to plan when the optimizer inlines it after one of its scans got a virtual column: ```sql WITH c AS ( SELECT id, score() AS s FROM t WHERE c1 MATCH 'apple' ORDER BY s DESC LIMIT 2 ) SELECT * FROM c; -- ERROR: A expression contains slot not from children ``` A score or ANN TopN pushed into a scan, and a MATCH in the select list, add a virtual column to the scan. After rewrite, the optimizer inlines a CTE that has one consumer left (`Optimizer.normalizeCtePlan` → `CTEInliner`) by deep copying its rewritten body, so the copy includes these scans: - `score()` and `l2_distance_approximate()` are nondeterministic, so the rewrite does not inline a CTE that calls them, even with one consumer. Once the TopN pushdown has replaced them with a virtual column of the scan, the optimizer inlines the CTE. So every CTE with `ORDER BY score() LIMIT n` or an ANN TopN fails, with default settings. - A CTE with MATCH in its select list reaches the optimizer when the rewrite removes all but one of its consumers, e.g. `... UNION ALL SELECT * FROM c WHERE 1 = 0`. `LogicalPlanDeepCopier` copied a scan with virtual columns wrongly: - it gave each virtual column two new ids: the slots above the scan were mapped to the first one, and the copied scan output the second one; - it copied the virtual columns in list order, so a virtual column that refers to a later one (CSE with `experimental_enable_virtual_slot_for_cse=true`, e.g. `[v2 := v1 * v1, v1 := x + y]`) kept the old id of the later one; - the copied scan still output the original virtual column, with its original id, next to the new one; - the score and ANN order keys of the copied scan still used the original slots. So the plan above the copied scan read a slot that the scan does not output. The fix: - The copier gives every virtual column its new id first and then copies their expressions, which reuse those ids, so virtual columns may refer to each other in any order. It also maps the score and ANN order keys to the new slots. Relations without virtual columns are copied as before. - `LogicalOlapScan.withVirtualColumns` replaces the virtual columns of the scan, but kept their old slots in the output next to the new ones. It now replaces them there too and keeps the other output slots, which the copier has already mapped. Computing the output of the copy again would not work: it gives VARIANT subpath slots new ids. `AdjustNullable`, the other caller that replaces existing virtual columns, no longer keeps two slots for a virtual column whose nullability changes. #69292 binds `score()` to its scan before CTEs are inlined, so it needs this fix; it carries the same change until this PR is merged. |
||
|
Jack
|
3c6af92291 |
[feature](inverted index) Add index_disk_usage table function (#67977)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Doris reports the total inverted index size of a rowset or table (for
example `information_schema.rowsets.INDEX_DISK_SIZE` and `SHOW DATA`),
but SQL users cannot see how much space each index takes, or how that
space splits between the term dictionary, postings, positions and
statistics. Deciding whether to drop `support_phrase`, change a parser
or switch the index storage format therefore requires inspecting index
files by hand on each backend.
This PR adds the table function `index_disk_usage`:
```sql
SELECT index_name, column_name, structure, storage_format,
SUM(total_bytes), SUM(dict_bytes), SUM(posting_bytes), SUM(position_bytes), SUM(stats_bytes)
FROM index_disk_usage("database" = "db1", "table" = "t1")
GROUP BY 1, 2, 3, 4;
```
- Parameters: `database` and `table` (required), `partitions` and
`indexes` (comma-separated names), `level` (`tablet` by default,
`rowset` or `segment`), and `position_detail` (default `false`).
- Columns: `PARTITION_NAME`, `MATERIALIZED_INDEX_NAME`, `TABLET_ID`,
`BACKEND_ID`, `ROWSET_ID`, `SEGMENT_ID`, `INDEX_ID`, `INDEX_NAME`,
`INDEX_TYPE`, `COLUMN_NAME`, `INDEX_SUFFIX`, `STRUCTURE`
(`TERM`/`BKD`/`ANN`/`CONTAINER`), `STORAGE_FORMAT`
(`V1`/`V2`/`V3`/`SNII`), `SEGMENT_COUNT`, `ROW_COUNT`, `TOTAL_BYTES`,
`DICT_BYTES`, `POSTING_BYTES`, `POSITION_BYTES`, `STATS_BYTES`,
`OTHER_BYTES`, `STATS_SOURCE`.
How it works:
- FE resolves the table, its formal partitions and the tablets of the
base and rollup indexes, and checks the `SHOW` privilege. In local mode
the partition visible versions are read together with the tablets under
the table lock; in cloud mode they are fetched from meta-service after
the lock is released, with the batched snapshot version API. Each tablet
is sent to one queryable replica through a new `INDEX_DISK_USAGE`
metadata scan range; a cloud replica routed to a smooth upgrade source
backend fails the query, because that backend cannot run this scan. A
`partitions` or `indexes` filter that names nothing is rejected.
- BE captures the rowsets visible at that version (syncing rowsets first
in cloud mode) and reads only index file metadata of each segment:
- V1/V2/V3: CLucene compound directory entries are attributed by
extension (`tis`/`tii` to dictionary, `frq` to postings, `prx` to
positions, `nrm` to statistics, everything else to other). BKD files
form a `BKD` row with only `TOTAL_BYTES`. The shared V2/V3 compound
header is reported as a `CONTAINER` row.
- SNII: the logical index directory and core metadata are read. The
dictionary region, sampled term index, dictionary block directory and
BSBF count as dictionary; the posting region as postings; core metadata
and norms as statistics; the null bitmap as other. SNII interleaves
positions with postings, so `POSITION_BYTES` is NULL unless
`position_detail=true` decodes the posting region to split them out.
Docs-only indexes report 0 positions.
- Rows are aggregated to the requested level. Within a segment,
`TOTAL_BYTES` of all rows adds up to the index file sizes; unknown
components are NULL.
- Every index file read carries the IO context of the query (query id,
reader type and the remote scan cache write limiter), so file cache
accounting and `file_cache_query_limit_bytes` apply to this scan like to
any other scan of the query.
- A request covering more tablets than the session variable
`index_disk_usage_max_tablets` (default 20000) is rejected, and
`position_detail=true` is additionally limited by
`index_disk_usage_position_detail_max_tablets` (default 10000).
Collecting these statistics at write time and an FE-aggregated
`information_schema` table are left to follow-up PRs.
### Release note
Add the table function `index_disk_usage` to show the on-disk size of
each inverted or ANN index, broken down into dictionary, posting,
position and statistics bytes, at tablet, rowset or segment level.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `external_table_p0/tvf/test_index_disk_usage_tvf`: V2, V3 and SNII
tables with two rowsets each; component presence per structure,
per-rowset totals reconciled with
`information_schema.rowsets.INDEX_DISK_SIZE`, all three levels, SNII
`position_detail`, index filtering and parameter errors. Passed on a
local single FE/BE cluster.
- [x] Unit Test
- BE: `IndexDiskUsage*` (V1/V2/V3/SNII collectors, docs-only and
positional indexes, `position_detail`, BKD, index filtering, missing
files, level aggregation) and `SniiSegmentReaderLogicalIndexesTest`: 22
passed.
- FE: `IndexDiskUsageTableValuedFunctionTest` (parameters, privileges,
partition/index resolution, visible version read outside the table lock,
`position_detail` tablet limit) and `IndexDiskUsageScanNodeTest`
(replica selection and grouping by backend): 20 passed.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
- The new table function needs a page in doris-website; not submitted
yet.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
|
||
|
Jack
|
1a0f8940b4 |
[fix](regression) Pin Variant mode in search document ID test (#69248)
### What problem does this PR solve? Issue Number: None Related PR: #68758, #69244 Problem Summary: Fuzzy sessions can enable `default_variant_enable_doc_mode` for `test_search_segment_document_ids`. Its table then uses document mode, while the `parse_to_variant()` values do not. INSERT fails with `Conversion from Variant(... enable doc mode = false) to Variant(... enable doc mode = true) is not supported` before the document ID assertions run. Explicitly disable document mode in this fixture so its V2 and SNII checks do not depend on the inherited session default. This addresses the [NonConcurrent failure in build 1072798](http://43.132.222.7:8111/viewLog.html?buildId=1072798&tab=buildLog). |
||
|
Jack
|
4016f207bf |
[fix](be) Support BM25 scoring on keyword SEARCH indexes (#69051)
### What problem does this PR solve? Issue Number: None Related PR: #68758 Problem Summary: SEARCH can match a keyword inverted index, but projecting score() can drop matched rows or report an unsupported index. Keyword indexes omit norms and may omit frequencies; they still need usable BM25 defaults. Preserve whole keyword values, including case and whitespace. Use frequency and encoded norm 1 for docs-only/no-norm postings. Legacy indexes without total-term-frequency metadata use average length 1; SNII keeps its physical statistics. Analyzed segments missing required scoring metadata and numeric BKD scoring retain their existing restrictions. This branch incorporates master's unified MATCH/SEARCH engine from #68758. Keyword norm handling lives in the CLucene postings cursor, including partition switches and WAND bounds; SNII cursors provide docs-only frequencies and norms. The obsolete query_v2/SNII scoring implementations are removed with master. Keyword matching and missing-metadata behavior follow Elasticsearch while retaining Doris's existing BM25 formula and collection-statistics conventions. #### P1 follow-up: keyword filters and internal conjunctions 1. A keyword index explicitly declared with `parser=none` can inherit `support_phrase=true` from FE defaults. Ordinary MATCH filters then entered scoring although the statistics collector skipped that field. Both IndexReaderHelper query-type overloads now share the canonical analyzer-and-phrase eligibility check used by statistics collection. The implementation is out of line to keep analyzer dependencies out of the header. Named-analyzer and normalizer behavior is preserved. Equality and keyword MATCH filters keep the SEARCH-only baseline score. 2. Scored SNII AND grouped a docs-only keyword term into a path that counts positions. Such sources now use the existing individual term scorer with implicit unit frequency; position-bearing sources retain batched scoring. Internal `SEARCH('body:apple AND tag:x')` continues to score both fields. |
||
|
Jack
|
7a9d745388 |
[fix](inverted index) Align phrase position defaults with analysis (#69199)
### What problem does this PR solve? Issue Number: CIR-22263 Related PR: #69192 Problem Summary: FE persists `support_phrase=true` for new tokenized indexes and leaves exact indexes docs-only. BE must treat an omitted property as false because older persisted analyzer indexes can omit it despite having docs-only postings; this also preserves `TOKENIZE` output. Built-in `lowercase` normalization emits one term, so phrase predicates can use docs-only postings and BM25 can use unit frequency and document length. Custom normalizers can emit multiple terms, so new indexes retain positions by default, including when a legacy custom policy shadows the built-in name. This PR changes phrase-position defaults and normalizer compatibility. PR #69192 adds direct SNII compaction of compatible docs-only postings. Neither change addresses the tokenized T2 merge hotspot identified in CIR-22263. |
||
|
Jack
|
31085da6b3 |
[fix](be) Score regexp and wildcard search terms with BM25 (#69052)
### What problem does this PR solve? Issue Number: None Related PR: #68758 Problem Summary: Regexp and wildcard search() clauses return constant score 1 even when scoring is requested. The old RegexpWeight has been replaced by ExpandWeight in master's unified MATCH/SEARCH engine. Pass the scoring flag through ExpandQuery. For regexp and wildcard expansions, create a BM25 scorer for each matching term using the bound IndexSource's statistics, including physical token counts from docs-only SNII sources and a unit-length fallback only when legacy token metadata is absent and sum contributions from terms matching the same document. Scored weights use scorers rather than the row-listing shortcut, which discards frequencies. Queries without scoring retain the batched bitmap path. This branch merges #68758 and moves the fix and its tests onto the new interfaces. Existing prefix/phrase-edge scoring and the pure-wildcard field:* existence rewrite are preserved. Tests with repeated and multiple matching terms reject the previous constant-score behavior. SNII source fixtures provide explicit scoring statistics and assert the expected BM25 score; pattern-only helpers remain unscored. |
||
|
Jack
|
8db05407dc |
[fix](inverted index) Stream compact docs-only SNII postings (#69192)
### What problem does this PR solve? Issue Number: CIR-22263 Problem Summary: SNII streamed compaction rejects compatible docs-only (T1) postings when the destination does not request phrase positions, then rebuilds those indexes from raw columns. This PR merges compatible T1 postings directly and retains raw rebuild for mixed or incompatible sources. Gram sources also retain raw rebuild because they can omit common postings and need their gram scheme preserved. Cloud compaction can assign nonzero physical destination segment IDs. The streamed SNII path now looks up its destination writer by physical ID, matching the existing CLucene path; looking it up by ordinal caused a BE crash in Cloud P0. This removes one repeated fallback seen in the CIR logs. The main T2 merge hotspot for tokenized indexes remains a separate performance question. Phrase-position defaults are handled in #69199. ### Release note Compatible docs-only SNII indexes use streamed compaction. ### Check List (For Author) - Test - [x] Unit Test: 17 SNII eligibility tests and the nonzero-segment-ID compaction test passed on the updated commit; 18 compaction-focused BE tests passed before the follow-up fix. - [x] Regression test: `test_storage_format_snii` passed on the combined source tree before this PR was split. - [x] Manual test: complete FE and BE builds and local health checks passed before the split. - Behavior changed: - [x] Yes. Compatible T1 postings are streamed; gram sources rebuild from raw columns. - Does this need documentation? - [x] No. The complete PR diff passes the English description check and clang-format 16 on the merged target tree. The updated commit is awaiting remote CI. |
||
|
Jack
|
f937d950d4 |
[improvement](be) Speed up dense unscored inverted index conjunctions (#69172)
### What problem does this PR solve? Issue Number: N/A Related PR: #68758 Problem Summary: #68758 runs an unscored AND of terms on one field, such as MATCH_ALL, as a chain: the rarest term's rows are listed into an array and the other terms narrow them into another. On dense terms this was slower than the bitmap intersection it replaced on V2 segments, and its arrays grew with the segment (review comment M2 on #68758): 1. A term held by every row cannot remove any row, but the chain still read its postings. For `MATCH_ALL 'http status'`, where both terms are held by every row, it copied every row id twice and rebuilt the result bitmap from the array: 14.4 ms on a 2M-row V2 segment, about 10 times what it took before #68758. 2. A term left alone, such as `get` in `get http` once `http` is skipped, was listed into an array of all its rows before the result bitmap was built from it. For a term in 90% of 2M rows that cost about 1,200 extra page faults per query. 3. Intersecting a block through a bit set tested each candidate with a branch. For two unrelated terms each in half the rows (`alpha beta`), the branch mispredicted about half the time, and the intersection took 30% of the scan thread's cycles. 4. The arrays held up to 8 bytes per row of the segment in each scanner. This PR: - skips a term whose document frequency equals the source's document count, so a conjunction whose terms are all held by every row returns every row, or every candidate, without reading postings; - lists a lone term's blocks straight into the result bitmap, a dense block as a range; - intersects a block through its bit set without a branch per candidate: every candidate is written and kept only when its bit is set; - streams a conjunction of several terms on a source that reads on demand, such as a V2 segment: the cheapest term lists at most 2^16 of its rows at a time, the others narrow them, and the rows left go straight into the result, so no array holds more than a batch. The other terms keep their current block from one batch to the next, so each block is still decoded once. A batching source, such as SNII, keeps the chain, which reads each term's blocks in one round. Scored queries keep their path, and results are unchanged. Index filter time of MATCH_ALL on a 2M-row V2 segment, as the median ratio of 8 paired runs on fresh backends; each column is a separate comparison and lower is faster: | Query | Rows holding each term | vs #68758 | vs before #68758 | |---|---|---|---| | `http status` | 100%, 100% | 0.01 | 0.10 | | `rare http` | 0.05%, 100% | 0.31 | 0.10 | | `sparse http` | 0.5%, 100% | 0.23 | 0.31 | | `get http` | 90%, 100% | 0.60 | 0.95 | | `alpha beta` | 50%, 50% | 0.56 | 0.72 | In the comparison with #68758, the `http status` filter drops from 16.9 ms to 0.15 ms and its end-to-end time to 0.63, and the end-to-end time of `alpha beta` to 0.81. SNII segments take the same paths except the stream; there these queries take 0.02 to 0.83 of #68758 and 0.02 to 0.93 of their time before #68758. Seven more shapes, from `rare alpha` and `sparse alpha` to `alpha beta get` and the empty `get post`, take 0.20 to 0.89 of their time before #68758 on V2 and 0.57 to 1.00 on SNII. Two-term MATCH_ALL queries on short text take 0.86 to 1.01 of #68758, and the 18 queries used to accept #68758 (MATCH_PHRASE, MATCH_ANY and SEARCH phrases on V2 and SNII) stay between 0.97 and 1.03 of it, every 95% interval including 1. On V2 the stream holds at most two arrays of 2^16 rows per scanner, and it cuts the page faults per query of `alpha beta` from 458 to 192 and those of `get alpha` from 484 to 47. Its scan threads run within 1% of the chain's instructions, and a cheapest term of at most 2^16 rows is listed in one batch, as the chain lists it. ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [x] Unit Test - [x] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> Unit test: `ListedTermsTest.AConjunctionSkipsATermHeldByEveryRow` and `ListedTermsTest.AConjunctionOfTermsHeldByEveryRowHoldsEveryRow` fail before the first commit, because the chain read the term held by every row, and pass after it. `ListedTermsTest.AConjunctionOnAStreamingSourceStreamsItsTerms` replaces `AConjunctionOnAStreamingSourceChainsItsTermsToo`, which recorded that such a source chained its terms, and fails before the last commit. `ListedTermsTest.ALoneTermListsItsRowsOrThoseAmongTheCandidates`, `CursorChainedPostings.IntersectionHandlesRandomDensities` and the `CursorChainedPostings.AStream*` tests cover the lone-term listing, the intersection and the stream; each fails when the code it covers is broken on purpose. Manual test: a 2M-row corpus with terms held by 100%, 90%, 50%, 10%, 0.5% and 0.05% of the rows was loaded into a V2 table and an SNII table. Each of 8 pairs started two fresh backends, one per build, swapping which backend ran which build on alternate pairs; each query ran 4 warm-up and 16 measured times on each build, alternating between the builds. Every result matched aggregates computed independently from the corpus. Scan-thread CPU time, cycles, instructions and page faults were sampled with `perf stat` over 200 runs of each query on each build. - Behavior changed: - [x] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [x] No. - [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> |
||
|
Jack
|
35b4fc18f0 |
[fix](be) Prevent score leakage across Boolean search branches (#69053)
### What problem does this PR solve? Issue Number: N/A Related PR: #68758 Problem Summary: Inverted-index leaves publish scores before an enclosing Boolean predicate knows which rows match. A row returned through one OR branch can receive scores from another branch whose AND condition failed, or from a negated term. Collect scores separately for each compound predicate and merge only scores for rows in its final positive bitmap. NOT discards its scores. A scored compound with incomplete index evaluation returns an explicit error. Defer restores the parent collector on every exit. This branch merges master and preserves its unified MATCH/SEARCH execution. The score collector and its includes now use `storage/index/inverted/query/scoring`; the obsolete reader-to-scorer hand-off documentation was removed. Existing Boolean evaluation changes and non-null ReadSchema test setup are retained. ### Elasticsearch comparison Before this master synchronization, an Elasticsearch 8.11.3 experiment used the regression suite's ten rows, one primary shard, the English analyzer, and default BM25. Equivalent `query_string` and `bool` + `match` queries returned the same scores. Eleven Doris query variants were compared by row id with the captured ES results, including independently measured leaves and branches, failed AND, reversed AND, and NOT. - An AND branch contributes only when the entire branch matches. - OR adds scores from its matching positive branches. - NOT contributes zero; rows returned only through the negative branch have zero score. - All eleven variants have the same matching ids and scores within an absolute tolerance of `1e-5` on this fixture. These are the [ES Boolean scoring rules](https://www.elastic.co/docs/reference/query-languages/query-dsl/query-dsl-bool-query). The result establishes equivalence for the tested Boolean trees and corpus. It does not establish universal cross-engine BM25 numeric compatibility. Doris also retains an explicit unsupported error for scored compounds that need row-level fallback after incomplete index evaluation. |
||
|
Jack
|
14fde1c1dc |
[refactor](be) Unify MATCH and SEARCH execution across inverted index formats (#68758)
### What problem does this PR solve?
Issue Number: None
Related PR: #67180, #67538, #67918
Problem Summary:
MATCH and SEARCH each had their own Boolean, phrase, term-expansion and
scoring executors, written separately for CLucene (V1/V2) and SNII
indexes. Every query optimization had to be implemented once per
executor, the formats drifted apart in behavior, and cross-field queries
could mix document IDs from different physical partitions.
This PR runs both on one engine:
- **One logical form.** MATCH (`query/logical`) and SEARCH
(`exprs/function/search`) lower their queries to the same logical nodes.
- **One query engine.** Boolean logic (AND/OR/NOT, minimum_should_match,
WAND), term expansion (prefix, wildcard, regexp), exact and sloppy
phrases and BM25 scoring are implemented once, independent of the index
format.
- **Formats behind an SPI.** CLucene and SNII only provide dictionaries,
postings, positions, norms, statistics and batched I/O through
`query/spi`. The old per-format executors are removed.
- **Layered source tree.** `be/src/storage/index/` is reorganized by
layer, and `build-support/check-header-deps.py` enforces the allowed
include directions.
Index formats and writers are unchanged, and existing indexes remain
readable. FE is unchanged.
#### Source layout
```text
be/src/storage/index/
├── index_*.h/.cpp shared index files: file reader/writer, compound reader, storage formats
├── ann/ bloom_filter/ zone_map/
└── inverted/
├── analysis/ analyzers, tokenizers, char and token filters, normalizers, gram schemes
├── common/ descriptors, caches, statistics, profiles, single flight
├── reader/ text index reader, index selection, collection statistics
├── query/ format-independent query engine
│ ├── logical/ MATCH lowering and logical nodes (gram/ builds gram queries)
│ ├── engine/ Query/Weight/Scorer: boolean, term, phrase, expansion, WAND, union, collect
│ ├── exec/ doc-id sets and sinks, term waves, chained conjunctions
│ ├── phrase/ exact and sloppy phrase matchers
│ ├── scoring/ BM25
│ └── spi/ the source interface each format implements
├── clucene/ CLucene (V1/V2) reader, searcher, writer, compaction
│ └── spi/ CLucene index source and postings cursor
└── snii/ SNII: format/, encoding/, io/, bkd/, reader/, writer/, compaction/, query/
be/src/exprs/
├── vsearch.h/.cpp VSearchExpr, the SEARCH expression node
└── function/search/
├── function_search.* SEARCH entry: Boolean assembly, DSL result cache, NULL handling
├── search_lowering.* SEARCH DSL to logical nodes; binds each field to an index
├── function_search_leaf.* compiles each leaf into a query on its bound index
├── search_predicate_collector.* collects scored leaves for BM25 statistics
└── variant_inverted_index_search.* SEARCH on VARIANT subcolumns and NESTED groups
```
#### Performance
The 18 production SQL cases (V2 and SNII: point, range, absent-term and
phrase queries) were compared with master on fresh dual-BE pairs: 8
pairs, 16 samples per case and binary, alternating order. Every case is
within the 5% limit for both `InvertedIndexFilterTime` and client wall
time; the largest ratios are 1.032 and 1.038. SNII phrase cases take
25-60% less FilterTime. TPC-H, TPC-DS and ClickBench are within noise.
<details><summary>Per-case median ratios (PR / master)</summary>
Run on 2026-10-14 against master
|
||
|
Jack
|
e103c10b56 |
[fix](fe) Reject trailing tokens in tokenize properties (#68701)
### What problem does this PR solve?
`NereidsParser.parseProperties` parsed `propertyItemList` without
requiring the parser to reach the end of the input. As a result,
`tokenize` silently accepted invalid tokens after a valid property
assignment. For example:
```sql
SELECT tokenize('alpha beta', '"parser" = "unicode" trailing_tokens');
```
The ignored suffix could hide a misspelled or incorrectly assembled
property string.
### What changed?
- Add `propertyItemListWithEof` and use it for standalone property
parsing. SQL statements that embed `propertyItemList` keep their
existing grammar.
- Preserve valid single and multiple properties, including trailing
whitespace and comments.
- Reject trailing identifiers, semicolons, closing parentheses,
unexpected symbols, and assignments without a separating comma.
`tokenize` returns its existing property format error.
- Add parser and `Tokenize` unit tests, plus two error cases in the
`test_tokenize` regression suite.
|
||
|
Jack
|
75d1f03a7c |
[fix](inverted index) Preserve tokens from all array elements in MATCH fallback (#68661)
### What problem does this PR solve? Without an inverted index, MATCH on an array of strings can silently miss rows when the matching token appears before the final array element. For example, ['alpha token', 'tail only'] fails to match MATCH_ANY 'alpha'. The array branch of FunctionMatchBase::analyse_data_token overwrites the accumulated tokens with each element's analyzer output, retaining only the final element's tokens. This change accumulates tokens from all array elements for MATCH_ANY, MATCH_ALL, and MATCH_REGEXP. MATCH_PHRASE, MATCH_PHRASE_PREFIX, and MATCH_PHRASE_EDGE evaluate each array element separately so phrases cannot span element boundaries. Their existing token-matching algorithms are extracted into file-local helpers. The new unit test covers all six MATCH variants, first and final elements, an empty final element, empty arrays, custom analyzers, keyword parsing, and phrase boundaries. |
||
|
Jack
|
0dcd2a31a7 |
[fix](be) Restore segment index expression unit tests (#68695)
### What problem does this PR solve? Related PR: #61092 Problem Summary: `SegmentIteratorApplyIndexExprTest` has been excluded from the BE unit test build since it was introduced. Its fixture creates a 100-row segment but leaves the candidate row bitmap empty. `_apply_index_expr()` skips virtual column index expressions when no rows remain, so seven cases never exercise their downgrade and error-propagation assertions. This PR initializes the fixture bitmap with all segment rows and removes the test file from the CMake exclusion list. It sets the mock expression node type to `MATCH_PRED` so the now-reachable `is_virtual_slot_ref()` check reads initialized state. The tests also verify that a null index context skips evaluation, that evaluation continues after a handled downgrade, and that a missing index error propagates when fallback is disabled. With only the CMake change, 7 of the original 11 cases failed locally. With the fixture and mock changes, all 12 cases pass. |
||
|
Jack
|
0a3595e618 |
[improvement](index) Bound SNII posting workspace across spill and encoding (#67657)
### What problem does this PR solve? Related PR: #67537 (merged into master) **Under review: the error-cleanup and query-performance items below remain open.** This branch is rebased onto master at `e429e59bdfc` after #67537 merged. The master comparison now contains only the two spill-specific commits (46 changed files). The CommonGrams deletion and released-format alignment are already in master through #67537, so those removed lines do not recur in this PR diff. Problem Summary: SNII expands compact postings into complete document, frequency and position arrays during spill, run merging and final encoding. A frequent term or a very long document can therefore create a large allocation while the original posting arena is still live. Limiting the first spill alone moves the peak into the merge or encoder. Repeatedly merging an increasing run prefix also amplifies temporary I/O. This change reserves one hard posting workspace before allocating, shared across the writer's spill, merge and final encoding: - Append compact fragments to one spool with independently sealed, checksummed run ranges. The run directory itself can spill. - Merge contiguous groups using fan-in constrained by workspace and available file descriptors. Intermediate passes copy encoded fragments; final consumption combines boundary documents in their original position order. - Replay large position payloads and stage DD, window metadata and inline DD/PRX within the same budget. Ordinary admitted windows retain the existing resident encoder. - Keep input arena, dictionary keys/ranks, norms, NULL metadata, allocator retention and OS page cache outside this posting-workspace limit. It is an algorithmic workspace bound, not a whole-BE RSS limit. - Preserve sequential read-ahead across small-term boundaries. The added regression originally read 578,549,868 bytes from a 271,632-byte run because every term discarded the same 64 KiB cache. The persistent SNII layout and window policy established by #67537 are retained. Persistent readers are unchanged. Large streaming ZSTD frames can have different compressed bytes, so reader compatibility alone is not treated as evidence of equal query performance. |
||
|
Jack
|
7c2f99b1bc |
[refactor](index) Move collection similarity to shared index code (#68602)
### What problem does this PR solve? Problem Summary: `CollectionSimilarity` is used by both inverted-index and SNII query paths, but its source and header live under `storage/compaction`. Move the implementation and its unit test to the shared `storage/index` directory and update direct includes. No scoring logic changes. |
||
|
Jack
|
709d859d72 |
[feature](inverted index) approximate gram index for LIKE/REGEXP push-down via ngram tokenizer mode=sparse|dense on SNII (#67538)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
`LIKE '%literal%'` and `REGEXP` on text columns always scan every row
today. This PR adds an **approximate gram index** for them, built on the
existing inverted-index machinery, so that a regex/LIKE first prunes
rows through the index and the original expression is then re-evaluated
only on the surviving candidates. Results are always identical to the
non-indexed evaluation: the index only ever produces a superset of the
matching rows, and every index-side failure degrades to "no
acceleration".
No new index type, tokenizer type or `parser` value is introduced. The
feature is switched on by giving the built-in `ngram` tokenizer a
`mode`:
```sql
CREATE INVERTED INDEX TOKENIZER gram_sparse_tok
PROPERTIES ("type" = "ngram", "mode" = "sparse", "min_gram" = "3", "max_gram" = "4");
CREATE INVERTED INDEX ANALYZER gram_sparse PROPERTIES ("tokenizer" = "gram_sparse_tok");
CREATE TABLE logs (id BIGINT, msg STRING,
INDEX idx_msg (msg) USING INVERTED PROPERTIES ("analyzer" = "gram_sparse"))
DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 8
PROPERTIES ("replication_num" = "1", "inverted_index_storage_format" = "SNII");
SELECT count(*) FROM logs WHERE msg REGEXP 'rpc error: code = (Unavailable|Internal)';
```
`max_gram` only sizes the dictionary (going from 16 to 4 cuts it by
about 60 % while the posting lists stay the same), so 4 is the value to
use. `density` may be given, but is normally solved per segment (see
below).
How it works (BE):
1. **Gram library** (`be/src/storage/index/inverted/gram/`):
`GramScheme` (parameters), `GramExtractor` (ASCII runs are cut into byte
grams — a dense sliding window, or sparse content-defined grams where a
byte pair is a boundary when its 16-bit hash falls below the density —
with duplicates within a row removed), `GramQuery` (AND/OR/ALL/NONE tree
with simplification), a RE2-subset regex parser, and `RegexGramCompiler`
(Cox 2012 style derivation of the grams a match *must* contain, for both
REGEXP and LIKE).
Non-ASCII text is not indexed. A non-ASCII code point contributes no
gram and separates the ASCII runs around it, on the write side and the
query side alike, so the non-ASCII part of a literal adds no constraint
(the candidate set only ever widens) and a wholly non-ASCII pattern runs
as a plain scan. An earlier revision indexed one term per code point; on
a CJK corpus each of those terms sits in a few percent of the rows,
which is above the stop-gram line described below, so they filtered
nothing while costing most of the index, and every measured alternative
(code-point pairs, content-defined chunks of code points) pushed the
dictionary alone past the size budget of the whole index.
The compiler's one hard contract is that it never drops a matching row.
Two tests hold it: a differential fuzz test against RE2, and a property
test that walks a corpus containing ill-formed UTF-8, NUL bytes, rows
around the `max_len` boundary and 4 KiB rows across a scheme matrix
varying `mode`, `lower_case`, `min_gram`, `max_gram` and `density`,
asserting for every row the engine matches that the compiled query still
keeps it a candidate.
Where the derivation cannot be made reliably the compiler degrades to
ALL and prunes nothing: an unparsable pattern, a lookbehind assertion,
an oversized character class or set, a literal that is not well-formed
UTF-8 (its grams would not line up with how the index split the row),
and a query tree carrying more gram leaves than the configured budget.
2. **Write path**: `NGramTokenizerFactory` returns a `GramTokenizer`
when `mode` is set; gram-family analyzers are recognised by the SNII
writer, which forces a docs-only index for them (`support_phrase` is
ignored). Analyzers that carry token filters or char filters are
deliberately treated as *not* gram-family, because their terms would no
longer correspond to the raw column value. The tokenizer is reused
across rows and the buffers it retains are charged to the writer's
memory reporter. Three decisions are taken per segment, from the
segment's own rows, and recorded with it:
- **The density is solved, not configured**
(`enable_gram_index_adaptive_density`, default on). Before the first row
is cut, the writer observes the first 4 MB of the column and picks the
sparsest boundary density at which 95 % of all 12-byte windows still
contain a whole gram (`gram_index_min_literal_bytes`,
`gram_index_density_coverage_permille`,
`gram_index_density_sample_bytes`). The promise is "a literal of 12
bytes can be found", and the density that keeps it is a property of the
column, not of the tokenizer: 0.31 on English log lines, 0.18 on URL
paths, 0.22 on JSON agent logs in the corpora below. The tokenizer's
`density` is the fallback when there is nothing to observe, and
whichever value was used is written into the segment's scheme.
- **Stop-gram.** A gram present in more than 0.15 % of a segment's rows
keeps its dictionary entry, with its df, but its posting list is not
written (`DictEntry::posting_dropped`). Such a gram reads as "every
row": an AND simply leaves it out, and an OR containing one is the whole
segment. That is what the query-side cost gate would have decided for it
anyway — reading that posting list costs more than the rows it removes —
so LIKE/REGEXP lose nothing, while the index keeps its dictionary and
sheds 80–92 % of its bytes on the three ASCII corpora measured below.
The line is derived from the same digest the gate reads, not configured,
and segments below 2,000 rows keep every posting. Query types that need
exact posting lists (`MATCH`, `EQUAL`, …) are guarded at the planner and
at the reader: a dropped posting there returns
`INVERTED_INDEX_EVALUATE_SKIPPED` and the predicate falls back to
row-level evaluation, rather than reading a zero-length posting as an
empty set. A metadata directory holding such an entry declares
`required_features = 2` (`kFeatureDroppedPostings`), so a reader without
this code refuses the container as unsupported and evaluates the
predicate without the index instead of misreading the locator-less
entry.
- **High-df digest.** A gram-family segment records the df of its most
common terms (keyed by the bloom hash, a few thousand entries at most)
together with the ceiling every unrecorded term sits below. The query
side derives its candidate budget from that ceiling and estimates a
query's result before touching any dictionary block, which on remote
storage is the difference between deciding for free and paying one round
trip per gram. It costs about 0.3 % of the index size and build time
within noise; non-gram indexes carry none of it.
3. **Query path**: a new `InvertedIndexQueryType::GRAM_BOOLEAN_QUERY`
evaluates a `GramQuery` on the SNII index. `FunctionLike` /
`FunctionRegexpLike` implement `evaluate_inverted_index`: constant
pattern → compile → gram query → candidate bitmap flagged as
**approximate**. Approximate results go into a separate table in
`IndexExecContext`; `SegmentIterator` only intersects them into
`_row_bitmap` when the conjunct root is the function itself, never marks
the column as "index evaluated", and keeps the conjunct for
re-evaluation. `NOT LIKE` / `NOT REGEXP` / OR-nested predicates are
therefore never pruned — and, because their approximate result could
never be read back, the index evaluation is skipped for them altogether
rather than performed and discarded. Push-down happens only for SNII
readers; CLucene-format readers reject the new query type.
Reads are issued per query node, not per gram: one batched dictionary
resolution (concurrent waves of DICT block reads, each term first
checked against the segment's resident bloom filter) whose entries are
memoised, then one batched posting plan/fetch. On a metered reader an OR
node costs a constant number of serial read rounds instead of two per
gram. A node is given up before any posting is read when its estimated
candidate count exceeds the budget: three times the digest's ceiling,
or, on a segment without a digest, `gram_index_max_candidate_ratio_bp`
basis points of its rows (default 15) once it has at least
`gram_index_candidate_ratio_min_rows` rows. The estimate for an AND is a
damped product of its grams' selectivities rather than its rarest gram
alone, and a sub-query's estimate feeds the AND that contains it. Giving
up yields the segment's whole docid range, so it can never prune a row.
4. **Metadata**: the writer records the gram scheme it actually used —
including the density the segment settled on — in
`SniiCoreMetadataPB.gram_scheme`, and the query side compiles a pattern
against the scheme read back from the physical index it is about to
read, so the two can never disagree: a policy dropped and recreated with
different properties does not affect segments already written. Analyzed
queries (`MATCH_*`) are held to the same rule: the reader compares the
scheme the current analyzer cuts with against the segment's persisted
scheme and returns `INVERTED_INDEX_EVALUATE_SKIPPED` when they differ,
and such queries bypass the result cache, whose key carries no scheme
identity.
5. **Compaction and schema change**: gram indexes are docs-only, so the
SNII index-merge fast path does not apply. Compaction and every
data-rewriting schema change rebuild the index from the converted
column, re-solving the density and re-deciding stop-gram for the new
segment; light schema changes carry the existing index files over.
`test_gram_compaction` and `test_gram_schema_change` pin both the
answers and the pruning across a full compaction and across light and
rewriting schema changes, including a change of the indexed column's own
length and type.
FE:
- `NGramTokenizerValidator` accepts and validates `mode`
(`auto|sparse|dense`, where `auto` is `sparse`), `density` `[0.001, 1]`,
`lower_case`, and the gram-family ranges of `min_gram`/`max_gram`
(`mode` absent keeps the legacy behaviour byte for byte). Numeric
properties must be spelled in ASCII, because BE parses them with
`strtol`/`strtod`.
- `IndexPolicyMgr` rejects gram tokenizers combined with a token filter
or a char filter, mirroring the BE rule exactly (use the tokenizer's own
`lower_case=true` instead of a `lowercase` filter, because folding must
happen before gram boundaries are computed).
- `InvertedIndexUtil` requires `inverted_index_storage_format = SNII`
for gram-family indexes, rejects `support_phrase = true` and index-level
char filters, and defaults `support_phrase` to `false` (also for `CREATE
INDEX` / `ALTER TABLE ADD INDEX`).
Observability: `RowsGramIndexFiltered`, `GramIndexCandidateRows` and
`GramIndexGateGaveUp` in the scan profile; BE config
`enable_gram_index_regexp` (default `true`) is the kill switch.
Measured on four corpora (cloud mode on S3 with a 16 GB file cache, one
BE; index size is `INDEX_LENGTH / DATA_LENGTH` from
`information_schema.tables`):
| corpus | rows | index / data, stop-gram off | index / data, as shipped
|
|---|---|---|---|
| English log lines (textbench) | 3 M | 1.10 | **0.28** |
| URL paths (httplogs) | 3 M | 0.87 | **0.05** |
| JSON agent logs, mixed script | 300 K | 0.85 | **0.09** |
| Chinese short text (weibo) | 500 K | 0.05 | **0.04** |
Query wall clock, index on against index off on the same table with the
SQL cache and the condition cache disabled: a selective pattern with an
indexable literal (`context deadline exceeded`, 13 rows out of 3 M) runs
14× faster with the column in the page cache, 5× when it has to come
from the file cache and 130× when every page has to be fetched from S3;
patterns that hit 1–10 % of the rows, or whose literals yield no gram,
run at 0.83–1.11× because the gate refuses them before a posting is
read. Loading with the gram index takes 1.3–4.4× the time of loading
with no index at all, which is between parity and about a third more
than a full-text inverted index on the same column.
Known limitations of this first step: a session with
`enable_function_pushdown` on does not use this index for LIKE at all:
the scan node turns the LIKE into a storage-layer `LikeColumnPredicate`
and, on a duplicate-key or merge-on-write table (where every column
counts as a key column), drops the conjunct from the expression tree, so
nothing is left for the index to answer -- on a table without an
NGRAM_BF index that leaves the predicate with no index at all, and
REGEXP is unaffected because only `like` is offered to that path;
non-ASCII literals are not indexed, so a query on a CJK column runs as a
scan; a literal too short for the solved density to cut a gram out of it
(typically under 8 bytes, e.g. `\.exe` on URL paths) yields no gram and
runs as a scan, and short-literal workloads should use `mode=dense`;
`(?i)` needs a `lower_case=true` tokenizer (with `lower_case=false` the
pattern is evaluated without the index); LIKE with a custom `ESCAPE`,
lookbehind assertions and patterns that are not well-formed UTF-8 are
evaluated without the index; the pattern is compiled once per segment;
stop-gram is a fixed policy with no switch, and a segment's dropped
postings come back only by rebuilding it; the speed-up depends on
pattern selectivity and on how much of the column would otherwise be
read (the index cuts scanned bytes by orders of magnitude on selective
patterns; on a fully page-cached single node the wall-clock gain is
smaller).
**Downgrading a cluster that has used this feature is not supported.** A
gram tokenizer policy sets `min_gram`/`max_gram` outside the range an
older BE accepts, and an older BE rejects the analyzer with
`INVALID_ARGUMENT` ("The difference between max_gram and min_gram in
NGram Tokenizer must be less than or equal to: [ 1 ]"). After a
downgrade, any write that has to build the index for that column (stream
load, insert, compaction, schema change) fails, and queries that analyse
a query string against that analyzer (`MATCH*`, `TOKENIZE`) fail; plain
`LIKE`/`REGEXP` still return correct results by scanning. The BE itself
still starts and other columns are unaffected. A policy that sets only
`mode` is worse rather than better: an older BE accepts it and silently
tokenizes with the legacy `ngram(1, 2)` semantics instead of failing.
|
||
|
Jack
|
47b2e1d3d3 |
[improvement](inverted index) Push the candidate row bitmap down into phrase queries (#67180)
### What problem does this PR solve?
Problem Summary:
The cost of a phrase-family inverted index query (MATCH_PHRASE /
MATCH_PHRASE_PREFIX multi-term path) is proportional to the whole
segment's
postings and positions, regardless of how small the surviving candidate
set
already is. On a production log table a 9-minute time window left only
22.4%
of each segment's rows after short-key pruning, yet every phrase
conjunct
still walked the full segment: per-query profile showed 2,657 segments,
11,913s of InvertedIndexSearcherSearchExecTime (99.7% of scan cost) and
125 GiB of index reads for a query whose final result was 0 rows. A
microbenchmark against the same code path calibrates the cost model to
~0.9µs per co-occurrence candidate, so evaluation cost tracks
candidates,
not results.
Fix: expose the scan's current candidate row bitmap to index queries
through
IndexQueryContext (the same SegmentIterator -> reader handshake channel
the
count-on-index fast path already uses):
- `IndexQueryContext::candidate_rows`: set by SegmentIterator around the
index-apply phase when the candidate bitmap is smaller than
`num_rows * inverted_index_candidate_pushdown_ratio` (new BE config,
default 0.3). Non-positive finite values disable the feature; values
above 1
or non-finite values are rejected by the config validator. The use site
checks
the domain before arithmetic. The pointer is reset on every exit path
via the
existing DEFER. `_row_bitmap` only shrinks during the applies, so
restricting
to its current state stays correct for every later conjunct.
- `RoaringDocIdIterator`: a read-only DISI adapter over the candidate
bitmap. PhraseQuery joins it into the leapfrog intersection (its
doc_freq() is the cardinality, so a small candidate naturally becomes
the
lead), while matchers keep only real term iterators -- a classic
two-phase iterator: candidates drive the approximation, terms keep the
position semantics. The single-term path is unchanged (no restriction,
same semantics).
- Query cache: only a query that actually consumes the candidate
produces a
partial bitmap and skips cache insertion. Cache lookups stay enabled --
a
cached full-segment bitmap intersected later is still correct and
cheaper.
Non-consuming queries continue filling the cache (see review round 2/3).
SNII indexes take the same candidates (added after the CLucene path
above):
- The snii::query phrase and phrase-prefix entry points accept a Roaring
candidate bitmap. After term planning, a candidate set within 8x of the
rarest term drives the docid intersection through SNII's existing
prefilter
path, so windowed postings are decoded only for windows covering
candidates;
a far larger candidate set filters the intersection result before PRX
decode, so a rare phrase never pays for every candidate.
- Phrase-prefix keeps or enables its tail-union prefilter under
candidates
when the union is cheaper than repeated leading-term candidate probes;
small candidate sets bypass it when candidate probes are cheaper.
- SniiIndexReader consumes candidates only for multi-term MATCH_PHRASE
and
MATCH_PHRASE_PREFIX, follows the same consumption-keyed cache policy,
and
keeps candidate-restricted bitmaps out of single-flight.
- SEARCH roots are evaluated without the candidate, like compound roots.
Measured effect (`PhraseCandidatePushdownBench`, committed as a
DISABLED_ UT:
one synthetic log corpus indexed through the production column writer as
CLucene V2 and as SNII, queried through the production readers with the
result
cache disabled for both lookup and insertion; RELEASE build,
1,000,000-row
segment. Each speedup is the ratio of medians from three independent
runs, with
ten thread-CPU samples per query in each run; every point asserts
restricted ==
full ∧ candidates):
| format | query | full ms | random 1% | random 10% | random 30% | range
1% | range 10% | range 30% |
|---|---|---:|---:|---:|---:|---:|---:|---:|
| SNII | exact_2 | 11.470 | 2.22x | 1.56x | 1.12x | 36.53x | 8.93x |
3.27x |
| SNII | exact_4 | 22.331 | 2.74x | 2.00x | 1.54x | 40.02x | 8.47x |
3.20x |
| SNII | prefix_2 | 3.799 | 1.28x | 1.47x | 1.11x | 11.73x | 3.45x |
2.15x |
| SNII | prefix_5 | 1.634 | 3.01x | 2.28x | 2.11x | 3.77x | 2.94x |
1.95x |
| SNII | rare_exact | 0.150 | 1.12x | 0.95x | 1.10x | 1.00x | 1.01x |
1.01x |
| V2 | exact_2 | 18.606 | 3.02x | 1.70x | 1.06x | 64.16x | 7.72x | 2.92x
|
| V2 | exact_4 | 21.953 | 2.90x | 1.59x | 1.12x | 65.14x | 9.30x | 3.19x
|
| V2 | prefix_2 | 4.584 | 3.14x | 1.54x | 1.15x | 23.39x | 7.58x | 3.11x
|
| V2 | prefix_5 | 1.529 | 2.78x | 1.40x | 1.18x | 6.51x | 4.09x | 2.14x
|
| V2 | rare_exact | 0.064 | 1.14x | 0.98x | 1.07x | 1.02x | 1.02x |
1.00x |
`range` candidates model a time range over time-sorted rows, `random` a
selective non-index predicate. The exact and prefix queries improved at
the
0.3 default in this corpus; the rare exact query stayed near break-even.
The
previous V2 rare-query speedup included a cache insertion that should
have been
disabled. The machine is shared, so small differences near 1x can move
between
runs. The earlier single-shape table (200k-doc CLucene segment, phrase
`big
red`) is superseded by this reproducible benchmark:
BUILD_TYPE_UT=RELEASE PHRASE_CANDIDATE_BENCH_DOCS=1000000 \
PHRASE_CANDIDATE_BENCH_ITERATIONS=10 GTEST_ALSO_RUN_DISABLED_TESTS=1 \
./run-be-ut.sh --run \
--filter='PhraseCandidatePushdownBench.DISABLED_RestrictedVersusFullPhrase'
-j <N>
### Release note
Inverted index: phrase and phrase-prefix queries on CLucene and SNII
indexes
now restrict doc-list intersection and position verification to the
scan's
surviving candidate rows when the candidate set is small (BE config
inverted_index_candidate_pushdown_ratio, default 0.3).
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Same query results; phrase evaluation cost now tracks
the candidate set. Only candidate-restricted phrase results skip
fulltext query-cache insertion. -->
- Does this need documentation?
- [x] No.
- [ ] Yes.
---
Review round 1 (automated review, addressed in the second commit):
1. Scoring norm source: `_norm_source` is pinned to the first real
postings
iterator, so BM25 norms never come from the candidate adapter. Red-first
test: under a candidate, two documents of different lengths scored
identically (2.9729729) before the fix vs. the correct distinct baseline
scores after it.
2. Engage-gate timing: the decision is extracted into
`_refresh_candidate_pushdown()` and re-evaluated after column-level
index
predicates, so an entry bitmap above the ratio that indexed predicates
shrink below it still publishes `candidate_rows` (threshold-crossing
test
added).
3. Config domain safety: validator (finite, <= 1.0) plus a use-site
`std::isfinite`/domain guard before the multiply and integer conversion.
4. Default calibration: 0.1 -> 0.3. The motivating workload keeps 22.4%
of
each segment after short-key pruning; the previous default never engaged
the pushdown for exactly that case. A finer crossover sweep is planned
as
a follow-up benchmark.
Review round 2/3 (automated review, addressed in the follow-up commits):
5. Cache policy is now keyed to actual candidate consumption: a
reply-direction
`candidate_rows_consumed` flag is set only by the candidate-joining
phrase
path and reset per search, so non-consuming queries (MATCH_ANY/ALL,
term,
regexp, single-term phrase, untokenized range) keep filling the query
cache
with their full-segment bitmaps while a candidate is engaged. Red-first
tests: cold-miss/second-hit for MATCH_ANY under a published candidate,
plus
a cross-reader stale-flag case (phrase then untokenized range).
6. Three-valued correctness: a candidate-restricted TRUE bitmap paired
with the
full-segment null bitmap could spuriously trigger VCompoundPred's AND
shortcut (NOT(A AND B) with nullable A: FALSE mis-typed as NULL, row
dropped). Compound roots are now evaluated with the candidate suppressed
in
both the conjunct and virtual-column loops; top-level single-predicate
consumption keeps the restriction (exact within the candidate). A
SQL-level
nullable-compound regression case is planned as a follow-up.
7. The `_norm_source` invariant check moved out of the per-document
scoring
loop (release-mode checks stay out of hot loops per AGENTS.md).
Update (SNII support, benchmark, regression test; rebased onto current
master):
8. Rebase fix: on current master
`segment_iterator_candidate_pushdown_test.cpp`
no longer expands `#define protected public` (the segment headers arrive
through column_reader.h first), so -Wunused-macros failed the BE UT
build;
the unused macro is removed.
9. SNII candidate pushdown, as described above. Red-first unit tests:
restricted SNII phrases returned the full-segment result, consumed
nothing,
were cached and shared through single-flight; a dense candidate set
turned
off the phrase-prefix tail-union prefilter (10,156 vs 1,143 leading
candidates decoded). The benchmark then drove two refinements: filtering
instead of driving when candidates vastly outnumber the rarest term (no
O(candidates) cost for rare phrases) and retaining affordable tail-union
prefilters under candidates.
10. Follow-ups from earlier rounds are done: the crossover benchmark
above,
and the SQL-level nullable-compound case in
`test_phrase_candidate_pushdown` (V2 and SNII tables, pushdown on, off,
and on again over cached full-segment results, plus a merge-on-write
table
with deletes).
11. Review of the SNII change: candidate suppression now checks the
whole
expression tree, so a compound predicate or SEARCH below another root
(for example `search(...) IS NOT NULL`) no longer runs its leaves under
the candidate (red-first test); the SNII streaming exact verifier and
the
repeated-term route, which fail loudly on a leftover non-candidate doc,
are
now exercised with driving and filtering candidates.
12. CLucene full-result cache insertions now use the existing
option-aware
helper in both fulltext and string readers. A disabled-cache test failed
before the fix because a later enabled query hit the entry from the
disabled run; it passed afterward. This also makes V2 benchmark samples
symmetric when the result cache is disabled.
13. The symmetric benchmark exposed an SNII phrase-prefix regression at
sparse
random candidates. A profile test showed 1,998 leading candidates under
a 10% candidate set versus 1,143 with the unrestricted tail-union
prefilter. The planner now retains that prefilter when its postings cost
no more than bounded candidate probes; the profile test passed, and
three
RELEASE runs moved `prefix_5` at 1% random candidates from 0.19–0.29x
to 2.93–3.04x. The one-candidate bypass test still passes.
|
||
|
Jack
|
ef72b93c67 |
[feat](inverted-index) Add IK tokenizers to custom analyzers (#67918)
### What problem does this PR solve? Custom inverted-index analyzers could not use IK smart or max-word tokenization with their configured character and token filters. This adds the `ik_smart` and `ik_max_word` tokenizer factories and policy validation. Composed pipelines need source-byte provenance across normalization, splitting, concatenation, trimming, and reset/reuse. Opt-in offset tracking propagates through IK, keyword, standard, ICU normalization, word-delimiter, and pinyin processing. Separate rune end offsets preserve gaps removed by delimiter concatenation. Review fixes preserve exact saved policy bindings across replay, lifecycle operations, MATCH serialization, provider dispatch, physical reader selection, and scoring-statistics collection. FE matches explicit built-in IK to its default max-word configuration. BE selection keys distinguish mode, lowercase configuration, and effective outer character filters without changing provider names, Thrift, or stored index formats. Encoded configurations cannot collide with arbitrary policy names. For backend-first upgrades, BE now binds an old FE's normalized analyzer name to the canonical saved policy and accepts a lowercase metadata key only when it resolves to the same policy. Exact metadata remains preferred. The compatibility alias cannot select a differently bound policy or a built-in analyzer with different tokenization. Named character-filter identities account for transformations absorbed by the resolved tokenizer while preserving meaningful ordered transformations. The two new global-policy suites run in `nonConcurrent` instead of contributing to concurrent P0 policy-quota exhaustion. Classic writer and query boundaries now convert lazy custom-analyzer failures to non-OK `Status` results. This includes ARRAY token-stream creation before `add_document()`, which clears the partial document and closes the writer on error instead of allowing a Doris exception to escape. FE identity construction also preserves case-folding absorption through the default `icu_normalizer` `nfkc_cf` filter, preventing equivalent lower-then-fold and fold-only pipelines from receiving different identities. FE policy validation now checks an exact replayed policy before falling back to a normalized built-in analyzer or normalizer. This prevents a legacy wrong-type `IK` or `LOWERCASE` policy from passing FE validation and failing later in BE. `CREATE ... IF NOT EXISTS` also returns for an existing replayed `ik_smart` or `ik_max_word` policy before applying the reserved built-in-name check, while fresh creations with those names remain rejected. Nested tokenizer, token-filter, and character-filter validation uses the same exact-first policy lookup as BE and enforces the expected component type. Replayed case-colliding policies can no longer bind a different saved component or the wrong filter family. When ICU token normalization changes text, its output carries a conservative source map to the original token span; unchanged tokens continue to delegate their precise upstream maps, and reset/reuse clears rewritten-map state. The latest AI-review fixes reject duplicate non-IK analyzer selectors before full identity comparison, including the implicit default selector, so outer character-filter differences cannot create two indexes that the query syntax cannot distinguish. Basic, CharGroup, Empty, ICU, and NGram tokenizers now publish document-relative source boundaries for offset-aware Pinyin. ICU resolves monotonically increasing UTF-16 break boundaries with a constant-size UTF-8/UTF-16 cursor, transformations that change rune counts use a conservative whole-source span, and NGram byte offsets preserve full `UChar32` values. The memory fixes remove full-input dense offset tables from ICU tokenization and normalization. The tokenizer preserves exact spans with its incremental cursor. The normalizer retains opt-in provenance, stores no provenance when unused, and represents a changed token with one compact conservative source span when enabled; Pinyin consumes that span without forcing dense maps. The current review fixes prevent rune-expanding ASCII folding and lowercase filters from forwarding invalid exact provenance, avoid allocating Pinyin rune and source arrays in the default ignore-offset mode, canonicalize FE analyzer identities using effective component defaults, and convert lazy scoring-analyzer failures back into `Status` results. The final CI follow-up aligns the duplicate-analyzer unit test with the intentional selector-based diagnostic introduced by those validation fixes. Current candidate: `31450230c09d6ba10b6f0c2e8de70f3765948543`. ### Release note Add IK smart and max-word tokenizers to custom inverted-index analyzers. Correct composed-tokenizer source offsets, preserve custom-analyzer MATCH bindings during backend-first upgrades, select the matching physical IK configuration, and fix effective duplicate-index identities. ### Check List (For Author) - Test: - Native FE and BE ASAN builds passed in the isolated task worktree. The BE build included the GLIBC 2.17 compatibility gate and its CDC client stage. - The official BE ASAN runner passed all 9 focused writer, reader, and analyzer failure-path tests, including the ARRAY pre-`add_document()` exception boundary. The complete `InvertedIndexWriterTest` suite passed 20/20. - The official FE runner passed all 22 `AnalyzerIdentityBuilderTest` tests. After policy-validation repairs, it also passed the replay/image coverage and all 40 `PolicyValidatorTests`; `./build.sh --fe` completed successfully with zero Checkstyle violations. - The official BE ASAN runner passed 77/77 tests across `IndexPolicyMgrTest`, `PinyinFilterTest`, and `ICUNormalizerFilterFactoryTest`, including component type collisions and ICU-normalizer-to-pinyin source spans before and after reset. - A broader official BE ASAN run passed 354 tests in 10 related suites. A later tokenizer/filter run passed 150/150, and the two new offset-focused Pinyin tests passed 2/2 on the final source content. - The memory-fix candidate passed 203/203 related ASAN tokenizer and filter tests, including deterministic large-input allocation coverage for the ICU tokenizer and both disabled and enabled ICU-normalizer provenance paths. The full ASAN BE build then passed. - An official FE run passed 118 tests in four classes, including real index selection and MATCH-to-Thrift translation. The selector-ambiguity coverage passed 58/58 in `InvertedIndexPropertiesTest`. - The current review-fix candidate passed 147/147 related ASAN BE tests covering Pinyin, ASCII folding, lowercase, and scoring collection. A full ASAN BE build passed after the C++ changes. - The latest review-fix head passed 112/112 targeted BE ASAN tests across ICU, Keyword, NGram, and Pinyin coverage. It also passed 86/86 FE tests across `AnalyzerIdentityBuilderTest` and `InvertedIndexPropertiesTest`; the full 80-module Maven reactor and Checkstyle completed successfully. - The final candidate passed all 22 `SchemaChangeHandlerTest` tests and the complete 80-module Maven reactor after updating its stale duplicate-analyzer diagnostic expectation. - Discriminating native RED/GREEN tests cover legacy exact reader metadata, normalized index metadata, policy-name collisions, a reserved built-in-name collision, canonical-key precedence, legacy normalizers, scoring selection, encoded-policy-name collisions, ambiguous non-IK selectors, document-relative Pinyin offsets for every custom tokenizer, retained allocations for large ICU inputs, rune-expansion provenance, default Pinyin allocation behavior, explicit component defaults, and scoring analyzer failures. - `test_ik_custom_analyzer`, `test_analyzer_identity_semantics`, and existing `test_ik_analyzer` passed earlier against the task-owned native ASAN cluster. The targeted analyzer-identity regression passed 1/1; the Basic CREATE/ALTER cases added by the latest review fix were not run locally. - The current candidate contains 26 PR commits relative to merge base `bf127c2d9ca1cdbb70c604320120fea1f1e33002`; exact merge-tree preflight against target snapshot `3750a819a2af157ad539d947bc301ff0f806b928` passed before publication. - The full committed PR English audit passed for 78 source files. Thirty-seven precisely reviewed Unicode input and expected-token literals remain as test data; no descriptive text was exempted. - Clang-tidy 16 reported no changed-line warnings in the modified production sources. Three ASAN test translation units could not be analyzed because the local clang-tidy resource headers lack the sanitizer headers; no suppression was added. - Exact-current-head merged-snapshot clang-format 16 and all FE reactor Checkstyle modules passed in the shared publication gate before push. - Local results are not a claim that fresh full CI or AI review passed. Those runs are requested and evaluated against the published candidate. - Behavior changed: Yes. Custom analyzers support IK tokenization; saved policy bindings, physical index selection, source offsets, effective duplicate-index identities, and analyzer selection remain consistent across query and DDL paths. - Does this need documentation: No new syntax is introduced. Existing analyzer configuration syntax is reused; no documentation changes are included. Component-level replay, serialization, dispatch, reader selection, scoring selection, and current-version end-to-end regression were validated. A full mixed-version cluster upgrade was not run. Existing IK internal lowercasing behavior is unchanged; the uppercase SQL probe is not treated as proof of a lowercase-disabled runtime fix. |
||
|
Jack
|
8d9e9c3f6d |
[fix](inverted index) Persist empty index files and stop writing orphan ones (#67859)
### What problem does this PR solve? Issue Number: N/A Related PR: #67702 (closed) Problem Summary: A segment whose schema owns an inverted index always owns an index file, even when no logical index had anything to write: an all-NULL VARIANT column extracts no subcolumn, so no CLucene directory is ever opened and the container is closed with nothing in it. That empty container is committed by `close()`, not by `create_file()`, and `IndexFileWriter`'s empty-file path only closed three named implementations -- `StreamSinkFileWriter`, `S3FileWriter`, `PackedFileWriter`. `LocalFileWriter` and `HdfsFileWriter` were left open, and `~LocalFileWriter` aborts -- and DELETES -- a writer it was never asked to close. So on local storage the file silently disappeared and `show_nested_index_file` reported `E-6003 ... not found` where `E-6004 ... is empty` was the truth. Closing through `FileWriter::state()` instead of a type whitelist fixes that, but on its own it is not enough: it makes one latent `IndexBuilder` invariant violation real, and it leaves a second `IndexBuilder` bug -- already reachable today on the sink-node and remote paths, where the empty file does persist -- untouched. **`IndexBuilder` writes a container for every segment it rewrites**, including when the ALTER leaves the schema with no index at all: dropping the last index, or adding an index whose column does not exist. Until now the local writer's destructor happened to delete those files. Once they are closed they persist as orphans that nothing ever links, copies, uploads, removes or checksums -- `BetaRowset` decides whether to touch the container by asking `has_inverted_or_ann_index()` of the rowset's own schema, and both segment-writer paths (`SegmentFlusher::_create_segment_writer`, `VerticalBetaRowsetWriter`) already gate creation the same way. `IndexBuilder` was the one caller that did not. **`IndexBuilder` refuses to read an empty container.** `update_inverted_index_info()` tolerates `E-6003` ("this rowset predates the index") but not `E-6004` ("this rowset's index is empty"), even though both mean the same thing there -- there is nothing to carry over, build everything from the raw columns. `BUILD INDEX` / `ADD INDEX` / `DROP INDEX` on a table with an all-NULL VARIANT index therefore fails the whole ALTER with `[E-6004]inverted index file ... is empty`. So: three changes, one invariant. > a rowset's schema owns an inverted/ANN index **iff** every one of its segments > owns an index file, possibly zero length 1. `IndexFileWriter::begin_close()` / `finish_close()` close the empty container through `FileWriter` for any non-null, non-CLOSED writer, preserving the two-phase asynchronous protocol and propagating close errors. 2. `IndexBuilder::handle_single_rowset()` writes no container when the output rowset schema owns no index -- on the drop path and the add path alike. 3. `IndexBuilder::update_inverted_index_info()` reads an empty container exactly the way it reads a missing one. Consumers already accept `E-6004`: queries downgrade unconditionally (`SegmentIterator::_downgrade_without_index`), index compaction marks the segment ineligible and rebuilds, `CollectionStatistics` skips it. SNII is untouched -- its container always carries a header, so it is never zero length, and its `has_container` probe stays `E-6003`-only. #67702 made change 1 and then updated `regression-test/plugins/plugin_curl_requester.groovy` and `test_drop_index_with_format_v2.groovy` to expect `E-6004` after dropping every index. That expectation describes the orphan file, so this PR leaves both at `E-6003` and removes the orphan instead. ### Release note Persist empty V2/V3 inverted index files through every FileWriter implementation instead of only the remote ones; stop failing ALTER on a segment whose index file is legitimately empty; stop leaving an unreferenced index file behind when the last index of a table is dropped. |
||
|
Jack
|
6a79f954e2 |
[feat](inverted-index) Support configurable ngram size difference (#67917)
### What problem does this PR solve? Issue Number: Related PR: None Problem Summary: Custom ngram tokenizers hard-code the allowed difference between max_gram and min_gram to 1. This prevents valid wider ngram ranges while offering no explicit override. This PR adds max_ngram_diff, keeps 1 as the backward-compatible default, requires ASCII integer syntax, and validates the same 0 through 255 range in FE and BE. The upper bound limits per-position token fan-out. Newly created policies also cap absolute custom ngram sizes at 1024, while an explicit persisted compatibility marker preserves marker-less policies accepted before that cap during rolling upgrades. Invalid replayed policies cannot block valid replacements. Because max_ngram_diff controls policy admission but does not affect emitted tokens, valid policies exclude it from analyzer identity so equivalent analyzers cannot bypass duplicate-index detection. ### Release note Allow custom ngram tokenizers to configure the maximum difference between max_gram and min_gram with max_ngram_diff values from 0 through 255. |
||
|
Jack
|
59859162cd |
[style](index) Translate SNII comments and test diagnostics to English (#67808)
### What problem does this PR solve? Related PR: #67537 Problem Summary: The SNII code and tests touched by #67537 contain Chinese comments and golden-corpus diagnostics. Translate that explanatory text to English, including the related FE policy and protobuf comments. Preserve all Unicode test inputs, executable logic, query expectations, and the on-disk format. Also clarify two comments against the existing behavior: the control-byte sample is not the full internal marker, and missing golden expectations fail verification. |
||
|
Jack
|
9f3b084a75 |
[refactor](index) Remove CommonGrams and realign the SNII on-disk layout with the released format (#67537)
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: `CommonGrams` (the token filter, its word set, the term-key escaping codec, the per-segment metadata, the hybrid docs-only/positioned query planner and its result-cache gate, the SPIMI pair-key fast path, 17 profile counters, 4 BE switches, the FE DDL validation and the `score()` admission rule) was an experiment that never reached a released build, yet it had grown roots through the whole SNII stack and had diverged the v1 on-disk layout from what the released 3.1-based writer produces. This PR removes it end-to-end and realigns the physical format so that a node running this code can read segments written by the released writer, and vice versa, without a format version bump. What changes: - **Removed**: everything CommonGrams-specific in BE (`be/src/storage/index/inverted/common_grams/`, the `common_grams` token filter, the SNII term-key escaping codec, `SniiCommonGramsMetadataPB`, the hybrid query planner and its cost model, the SPIMI pair-key path, the frequency-less spill-run shape) and in FE (the `common_grams` token filter type, the CommonGrams policy validation, the `score()` admission rule). Loading an existing policy of that type marks it invalid with an error log instead of crashing FE. - **Term keys** are now the raw analyzed bytes everywhere. User terms that overlap the internal `\x1F` phrase-bigram namespace bypass the index (the same rule the released writer already implied). - **Scoring (BM25) is kept** and decoupled from CommonGrams: a segment is scorable iff it has positions *and* norms. Tokenized indexes with positions now always write norms (an optional region referenced from the core metadata; no new `index_config`). `tf` is the number of positions the term has in the document. Older segments without norms answer `NOT_SUPPORTED` for scoring only; a normal compaction reconstructs norms from the postings (byte-identical to a rebuild), so they become scorable after one compaction. The WAND / block-max paths are removed together with the per-window max statistics they depended on. - **On-disk layout (S1)**: `kFormatVersion` stays 1. The dict entry / dict block / frq prelude / frq pod encoders and decoders are byte-for-byte the released layout again: prelude flag bit0 = `has_prx`, no freq regions, no per-window `max_freq` / `max_norm`, no per-entry term statistics, no `frq_docs_len` / `inline_dd_disk_len` / `freq_meta`, no `kNoTermStats` dict-block flag. `sum_total_term_freq` and the norms reference live in optional proto fields the released reader ignores. Segments carrying the experimental CommonGrams metadata fields (or the experimental scoring `index_config`) are rejected as `NOT_SUPPORTED` with a rebuild hint. - Two follow-up fixes that fell out of the layout change: the phrase-query "position work" estimate now parses prx frame headers without verifying CRCs (`read_prx_frame(..., verify_crc=false)`), so a phrase query no longer checksums every candidate prx window twice; and the frequency-less spill-run record shape is gone (the production SPIMI path always materializes frequencies). Compatibility evidence (real binaries, golden corpus of 8 sample indexes / 179 query expectations / 513 checks across cache-off, cache-cold and cache-warm): - this branch reading segments written by the released 3.1-based writer: 8/8 open, 513/0 - segments written by this branch, checked against the expectations recorded before the removal: 513/0 (query semantics unchanged) - the released 3.1-based binary reading segments written by this branch: 8/8 open, 513/0 (rollback-safe: the old reader ignores the norms field) Size: 178 files, +2321 / −20981 (be/src −8184, be/test −10066, fe −946). ### Release note The experimental `common_grams` token filter and its scoring tier are removed. BM25 scoring on a tokenized SNII index now requires the norms region, which the writer always emits for tokenized indexes with positions; segments written before this change become scorable after one compaction. |
||
|
Jack
|
40ff995f89 |
[improvement](inverted index) Short-circuit pushed-down conjuncts once the row bitmap is empty (#67171)
### What problem does this PR solve?
Problem Summary:
On log-search workloads a query typically carries several pushed-down
conjuncts, e.g. two cheap untokenized MATCH terms plus one expensive
`msg MATCH_PHRASE_PREFIX '...'`:
```sql
SELECT * FROM logs
WHERE _ctime_ >= ... AND _ctime_ <= ...
AND _namespace_ MATCH 'ns' -- untokenized term, ~free
AND _pod_name_ MATCH 'pod-xyz' -- untokenized term, ~free
AND msg MATCH_PHRASE_PREFIX '...' -- whole-segment postings + positions
ORDER BY _ctime_ DESC LIMIT 1000
```
`SegmentIterator` evaluated ALL pushed-down conjuncts against the index
first and only intersected their bitmaps into `_row_bitmap` afterwards,
so the phrase conjunct paid its full whole-segment postings/positions
cost even when an earlier selective conjunct had already emptied the
candidate bitmap. Reproduced on a production log table: one query
returning 0 rows burned 11,291 CPU seconds and 124 GiB of local index
reads per physical SQL, virtually all of it inside phrase evaluation
(profile: 2,657 segments, InvertedIndexSearcherSearchExecTime = 99.7% of
scan cost, ScanRows = 0).
The empty-bitmap short circuit already exists on the neighbouring paths:
column predicates (`continue_apply` in `_apply_inverted_index`) and
compound AND inside a single expression (`VCompoundPred` COMPOUND_AND
early exit). But top-level AND conjuncts are flattened into separate
expr contexts and take `_apply_index_expr`, which had neither
progressive intersection nor a short circuit, so the most common query
shape never benefited.
Fix: intersect each consumed index result into `_row_bitmap` as soon as
it is produced, and stop evaluating further conjuncts once the bitmap is
empty. A skipped conjunct stays pushed down, so the row-level path keeps
its exact semantics over the now-empty candidate set (zero rows read,
zero cost). Consumed conjuncts are erased from
`_common_expr_ctxs_push_down` only after the ANN range-search pass,
which iterates the same list. A new profile counter
`InvertedIndexConjunctsShortCircuited` reports how many conjuncts were
skipped.
Behavior notes:
- Index results are now intersected before the ANN range-search pass, so
ANN range search executes on the already-narrowed bitmap. AND semantics
are commutative, and a smaller candidate set is what its small-candidate
fallback is designed for.
- Condition cache digest semantics unchanged: a short-circuited (thus
still pushed-down) conjunct keeps the digest cleared, so partial index
results are never cached as full coverage.
### Release note
Inverted index: once earlier pushed-down conjuncts empty the candidate
row bitmap, remaining conjuncts (e.g. expensive MATCH_PHRASE_PREFIX) are
no longer evaluated against the index.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Index results are intersected progressively (before the
ANN pass); short-circuited conjuncts fall back to the row-level path
over an empty candidate set. Same query results, strictly less index
work. -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
Review round 2 (automated review, addressed in the third commit):
1. Dead work after the short circuit is now bypassed on an empty
candidate
bitmap: virtual-column MATCH projections skip their whole-segment index
evaluation, the ANN range-search pass breaks out before
loading/searching,
and ANN TopN falls back at entry (all three previously ran even with
zero
surviving rows when the small-candidate fallback thresholds are
disabled).
2. The consumed-list clear no longer drops the proven all-false
condition-cache entry: `_index_conjuncts_proved_empty` keeps the digest
alive for the exhausted case (the empty result is correct for the full
conjunction), while a counter-case test pins the normal fully-consumed
path to its existing digest-zeroing behavior.
|
||
|
Jack
|
fa95a70656 |
[fix](be) Restore SNII proto field numbers to the shipped layout (#67135)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: SNII shipped before its metadata messages reached upstream, and the version upstreamed in #66052 had INSERTED fields in the middle of two of them rather than appending. protobuf identifies a field by its tag, so the two layouts now disagree about what a tag means, and every SNII segment already written decodes incorrectly under the upstream numbering: SniiStatsPB shipped upstream (#66052) tag 4 null_count sum_total_term_freq tag 5 -- null_count SniiSectionRefsPB shipped upstream (#66052) tag 3 null_bitmap norms tag 4 bsbf null_bitmap tag 5 -- bsbf Neither misread fails. SniiStatsPB tags 4 and 5 are both uint64, so a null count is returned as the collection token sum and feeds avgdl -- a plausible wrong number with no error anywhere. In SniiSectionRefsPB the null bitmap's offset/length is taken for the norms region and bsbf is lost entirely. This PR renumbers both messages back to the shipped layout and appends the two upstream additions after it: sum_total_term_freq becomes 5 and norms becomes 5 in their respective messages. SniiCommonGramsMetadataPB needed no change -- its upstream fields 8-12 were appended, which is the shape every future addition must follow. A new case set asserts the field numbers through protobuf reflection rather than through a byte digest, so the next accidental insert fails with a message naming the field and both tags instead of an opaque checksum mismatch. The twelve SniiWriterGoldenBytes digests were RE-HARVESTED: field tags and the field-number ordering are part of the serialized Core metadata that every SNII segment carries, so all of them moved -- including kGoldenKeywordDocsOnly, whose index has neither norms nor a null bitmap. That one moving is the evidence the change reaches every segment shape rather than one lane. COMPATIBILITY: this changes how existing bytes decode, deliberately. Segments written by an upstream build carrying #66052 are not readable afterwards and must be rebuilt; the format is not yet released upstream, so that population is development data only. Segments written by the shipped build become readable, which is the point. |
||
|
Jack
|
e59320b957 |
[fix](inverted index) update clucene to reject terms containing an embedded NUL byte (#67179)
Bumps contrib/clucene from abe2b71a1c1 to 08d45e541fe, picking up apache/doris-thirdparty#368. Problem ------- BE dies with SIGSEGV while writing an inverted index when a term contains a raw NUL byte in the middle (JSON \u0000 decodes to this). The crash reproduces on any table with an inverted index over such data, and restarts in a loop. Root cause ---------- In SDocumentWriter.cpp the two paths disagree on what a term is: * addPosition() hashes and dedups with eq(p->term_, term), i.e. the FULL string_view length, so "svc\0A" and "svc\0B" are two distinct postings; * comparePostings() stops at CLUCENE_END_OF_WORD, so the very same two postings compare EQUAL. A run of mutually "equal" but distinct postings makes quickSort()'s partition loop peel exactly one element per call, so the recursion depth becomes O(n) (measured depth/N = 1.000). Each frame is 128 bytes, so ~65k levels exhaust the 8 MiB thread stack. The stack is so far gone that the kernel cannot build a signal frame and force-kills the process (si_code=SI_KERNEL), which is why no handler output or stack trace ever reaches the log. Fix --- doris-thirdparty#368 rejects such terms in addPosition(), before they can enter the postings hash, so the degenerate equal-run can no longer form. The row now fails with a clear CLuceneError, which InvertedIndexColumnWriter::add_document() already converts into INVERTED_INDEX_CLUCENE_ERROR, instead of taking the BE down. Verification ------------ Reproducer: 80k terms shaped "svc\0<i>", untokenized inverted index, 8 MiB stack. before: SIGSEGV, exit 139 after : 80000 rows rejected, exit 0, 0 postings reach the sort Performance on clean data (the guard is one compare + a never-taken branch): perf instructions, 1M tokens : 10,116,997,256 -> 10,118,997,132 = +2.00 instructions/token (+0.020%) index output (.fnm/.frq/.nrm/.tii/.tis) : byte-identical |
||
|
Jack
|
40e1463f8c |
[fix](be) Validate SNII count fast-path document frequency (#66877)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: SNII's count-only shortcut trusted the persisted dictionary document frequency without checking it against Core document statistics. A corrupted 10-row segment whose posting contains two documents but whose dictionary entry records df=100 could fabricate a 100-id bitmap in both nullable and non-nullable paths, producing an inflated count after segment masking. Validate df against both total and indexed document counts before fabricating the count-shaped bitmap and report the cross-section mismatch as index corruption. Add CRC-valid nullable and non-nullable fixtures that rewrite only df while preserving real two-document postings. ### Release note Fix SNII count-only queries to reject corrupted document frequencies outside the segment document domain. |
||
|
Jack
|
80c1a3a276 |
[fix](be) Match SNII wildcards by UTF-8 code point (#66861)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: SNII evaluated wildcard question marks byte by byte, so `a?b` could not match `a猫b`, while V3 converts `?` to a RE2 dot and matches one Unicode code point. The SNII matcher now validates UTF-8 and advances its existing reusable dynamic-programming rows only at code-point boundaries. This preserves the bounded two-buffer scratch design, makes invalid patterns return `INVALID_ARGUMENT`, and aligns SNII query results with V3 without changing the writer or stored bytes. ### Release note SNII wildcard question marks now match one UTF-8 code point, consistent with V3. |
||
|
Jack
|
76cbb6bf79 |
[fix](be) Read instead of recursing when the ANN IVF list cache is absent (#67024)
### What problem does this PR solve? Issue Number: None Related PR: #61160 Problem Summary: An IVF-on-disk search with no `AnnIndexIVFListCache` installed never returns — it recurses until the stack is exhausted. **Reproduction**: build an `IVF_ON_DISK` index, save it, load it, and search it without calling `AnnIndexIVFListCache::create_global_cache()` first. Under ASAN the process dies with `AddressSanitizer: stack-overflow`; on a large stack it simply spins. **Root cause**: `CachedRandomAccessReader::borrow()` falls back to `RandomAccessReader::borrow()` when `AnnIndexIVFListCache::instance()` is null. faiss documents that base implementation as *"allocates a buffer and calls read_at()"*, and `CachedRandomAccessReader::read_at()` is overridden to call `borrow()` — so the two call each other without bound: ``` CachedRandomAccessReader::borrow (no cache) -> faiss::RandomAccessReader::borrow (base: allocate + read_at) -> CachedRandomAccessReader::read_at (override: calls borrow) -> CachedRandomAccessReader::borrow -> ... ``` The branch reads like a fallback but cannot work at all. **Reachability, stated plainly**: `exec_env_init` installs the cache unconditionally at BE startup, so a running BE does not hit this today. What it does hit is unit tests. Every existing IVF-on-disk case in `faiss_vector_index_test.cpp` opens with `AnnIndexIVFListCache::create_global_cache(...)`, and a new test that does not know to do so gets a stack overflow rather than a diagnosable failure — which is how this was found, while writing an unrelated ANN container test. It also becomes live the moment the cache is made optional or its lifetime is shortened. **Fix**: the no-cache path reads the region itself under `_io_mutex` and returns an owning `ReadRef`. No cache means no cache accounting, so it uses a plain buffer rather than a `DataPage`; the buffer lives exactly as long as the ref the caller holds. ### Release note None |
||
|
Jack
|
511a8a0f89 |
[fix](be) Normalize SNII prefix syntax before analysis (#66874)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: `SEARCH ... PREFIX` values retain their trailing DSL `*` marker in FE. SNII classifies a custom keyword analyzer as analyzed, but that analyzer preserves the marker as a literal byte. Consequently, a predicate such as `fail*` sent `fail*` to the phrase-prefix reader instead of the indexed prefix `fail` and returned no rows, while V3 matched the expected documents. This PR strips exactly the trailing DSL marker before SNII phrase-prefix analysis. Non-analyzed keyword fields continue to use `WILDCARD_QUERY` with the marker intact, ordinary WILDCARD clauses are unchanged, and the V3 path is unchanged. The change is query-only: it does not alter index writes or the on-disk format, so existing SNII indexes are fixed after upgrade without rebuilding. TDD evidence: - RED: a real custom keyword tokenizer/analyzer preserved `fail*`; `FunctionSearch::build_leaf_query` sent `fail*` and produced an empty bitmap instead of `{0, 2}`. - GREEN: the same path sends `fail`, retains `MATCH_PHRASE_PREFIX_QUERY`, and produces `{0, 2}`. - All 98 `FunctionSearchTest` cases pass. - `./build.sh --be -j 192` passes with ASAN. - `build-support/check-format.sh` passes. ### Release note Fix SEARCH PREFIX queries on SNII indexes that use a custom keyword analyzer. |
||
|
Jack
|
74b4418d73 |
[fix](be) Validate SNII analyzer context before query cache (#66869)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: SEARCH selected the correct SNII reader but passed no analyzer context to native queries. A cold CommonGrams query therefore bypassed while a warm raw-query cache entry could be returned before the immutable segment analyzer contract was validated. This PR builds the analyzer context from the selected reader properties, passes it to analyzed native queries, and defers analyzed raw-cache lookup until segment admission. Analyzer-independent wildcard and regexp queries keep their early cache path. ### Release note Fix SNII SEARCH queries on CommonGrams indexes so cold and cached queries validate the same analyzer contract. |
||
|
Jack
|
72a587f541 |
[fix](be) Use file-backed staging for SNII ANN indexes (#66856)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: SNII staged every serialized Faiss ANN sub-file in a growing in-memory vector. A large HNSW or IVF index therefore retained a second complete copy of the serialized index and could exhaust BE memory during index construction. A 64 MiB staging reproduction increased allocated heap by 67,388,976 bytes before this change. Fault-injection reproductions also showed that final buffered append or fsync failures could escape `AnnIndexColumnWriter::finish()` as `CLuceneError`; the IVF data path additionally leaked its raw output owner. This change reuses the self-cleaning `StagedBlobFile` abstraction for ANN sub-files and keeps only the fixed CLucene output buffer resident. It also gives both `ann.faiss` and `ann.ivfdata` explicit RAII ownership, closes them in a catchable context, and converts staging failures to `Status`. The successful Faiss sub-file bytes, SNII container layout, persisted index format, and query behavior are unchanged. Existing SNII indexes remain readable after upgrade. ### Release note Reduce peak BE memory while building SNII ANN indexes by staging serialized Faiss files on temporary storage instead of retaining a complete in-memory copy. |
||
|
Jack
|
e33e04823a |
[fix](be) Stop two SNII unit tests from trampling shared state (#66961)
### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: Two cases under `be/test/storage/index/snii/` reach outside their own test and break unrelated cases. Both are test-only; no production code changes. **1. A fixed `/tmp` scratch path (`metered_file_reader_test.cpp`)** `MakeRampFile()` built its 256-byte ramp file at the constant path `/tmp/snii_metered_ramp.bin`. On a shared build machine that file belongs to whoever ran the suite first; every later user gets `EACCES` from `::open`, and all seven `SniiMeteredFileReader` cases fail at the first assertion with no hint that a foreign file is the cause: ``` metered_file_reader_test.cpp:42: Failure Value of: w.open(path).ok() Actual: false Expected: true ``` Every other scratch path in this subtree already namespaces itself with the pid and a counter, and the nearest neighbour in the same directory (`file_reader_caller_buffer_test.cpp`) also `std::remove()`s the file afterwards. This PR adopts both halves of that convention: the ramp file becomes a `RampFile` RAII object whose name carries the pid and a counter and whose destructor removes it. Removing matters as much as naming — a leftover file is what causes the collision in the first place, and pids get recycled. **2. A permanently lowered `RLIMIT_NOFILE` (`spimi_term_buffer_test.cpp`)** `SniiSpimiTermBuffer.SpillOpenIoFailureLatched` drives a spill's `::open` to `EMFILE` by lowering `RLIMIT_NOFILE` to a hardcoded 64 and then opening `/dev/null` until the table is full. Two things go wrong once the binary already holds more than 64 descriptors: 1. The cap assumes the process holds fewer than 64 descriptors, so the hog loop can still take one. Sampling `/proc/<pid>/fd` during a run shows the SNII cases alone keep the process at **14** descriptors, but adding the CLucene inverted index cases to the same binary takes it to **273** — they hold hundreds of index files open at once. Above the cap the very first `::open` already fails, `hogs` stays empty, and the case stops at `ASSERT_FALSE(hogs.empty())`. What decides this is *which* cases share the binary, not how many. 2. That assertion is fatal, so it returns from the test body — and the code that restores `RLIMIT_NOFILE` sits after it and never runs. The whole process stays capped at 64 descriptors for every later case. The result is not a descriptor leak but a cascade: the next four `SniiSpimiTermBuffer` cases fail instantly because they cannot open a spill file, and the first death test after them aborts the entire run when gtest's `pipe()` hits `EMFILE`: ``` CHECK failed: gtest-death-test.cc, line 1400: pipe(pipe_fd) != -1 Aborted ``` The fix holds the tight limit and the hog descriptors in an inner scope whose `Defer` restores `RLIMIT_NOFILE` and closes the descriptors on every exit path, fatal assertion included; and derives the cap from `::dup(STDIN_FILENO)`, the lowest free descriptor, instead of a fixed 64, so the case no longer depends on which tests ran before it. The case still drives the spill open to `EMFILE` and still asserts the error is latched. **Before / after**, with `--filter='*Snii*:*snii*:*SNII*:CollectionStatisticsTest.*:*InvertedIndex*:*inverted_index*'`: | | before | after | |---|---|---| | `SniiMeteredFileReader` | 7 failed | pass | | `SniiSpimiTermBuffer*` | 5 failed | pass | | death test | aborted the run | pass | | total | run truncated by the abort | 1693 passed, 0 failed | |
||
|
Jack
|
1db4b1d305 |
[fix](be) Validate SNII BKD splits against global bounds (#66864)
### What problem does this PR solve? Issue Number: None Related PR: #66052 Problem Summary: A CRC-valid SNII BKD index whose split catalogue was ordered but outside its persisted global bounds could open successfully and silently route queries to the wrong leaf. The issue reproduces with values 0 through 19 split into two leaves: changing the persisted split from 10 to 30 makes a point lookup for 15 inspect leaf 0 while the matching point remains in leaf 1. This PR validates the complete non-decreasing `min_value <= split_values... <= max_value` chain when opening the index and asserts the same invariant when encoding. Equal adjacent values and splits equal to either inclusive global bound remain valid. |
||
|
Jack
|
5870fb2513 |
[fix](be) Release SNII writer reservations on auxiliary failure (#66855)
### What problem does this PR solve? Issue Number: N/A Related PR: #66052 Problem Summary: SNII adopted a `LogicalIndexWriter` into `SniiCompoundWriter::indexes_` before appending its norms, null bitmap, and block-split bloom-filter sections. An injected append failure at those three boundaries reproduced retained `MemoryReporter` charges of 96, 86, and 60 bytes respectively after `add_logical_index()` returned. The ordinary build caller transfers reporter ownership only after that call succeeds, so failure teardown could leave the compound writer holding reservations that refer to an already-destroyed reporter. This change keeps the logical writer and its placement local while writing all auxiliary sections, then adopts both into the compound writer only after every append succeeds. The same ownership rule is applied to the streamed path. Poisoning behavior and the successful append order, offsets, file layout, and bytes are unchanged, so this does not change the SNII storage format and does not require rebuilding existing indexes. |
||
|
Jack
|
96fd1e9bd2 |
[feature](inverted index) Add SNII inverted index storage format (#66052)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
Add **SNII**, a Doris-native inverted index storage format designed for
object storage, as an opt-in alternative to the CLucene-based V1/V2/V3.
On cold queries served directly from S3, SNII is about an order of
magnitude faster than V3.
Cold object-storage latency is dominated by the number of round trips,
not by the bytes read. CLucene spreads a single term lookup across
several index streams that sit far apart in the file, so one lookup
becomes many small scattered reads. SNII keeps each segment's indexes in
one container, packs the sections of an index contiguously, aligns them
to file-cache block boundaries, and coalesces adjacent ranges into a
single batched read that is served in parallel — a lookup that costs
CLucene many requests costs SNII a few. Positions are decoded on demand
rather than read in full.
Enable it per table; the cluster default stays V3, so existing tables
and queries are unaffected:
```sql
PROPERTIES ("inverted_index_storage_format" = "SNII")
```
The format covers the full index lifecycle: writing and querying (exact,
analyzed, phrase and sloppy phrase), BM25 scoring, BKD and ANN indexes,
VARIANT sub-columns, the SEARCH DSL, native index compaction, `BUILD
INDEX`, and local plus cloud index changes.
Beyond the format itself, this PR also fixes shared inverted index
behaviour: reader selection no longer aborts the BE process when a
caller has no runtime column type, and match-analyzer resolution is made
consistent (custom analyzers now work with the `none` parser).
### Release note
Add SNII, a new opt-in inverted index storage format designed for object
storage, giving roughly an order of magnitude faster cold queries
against S3. Enable it per table with `"inverted_index_storage_format" =
"SNII"`; the cluster default remains V3, so existing tables are
unaffected.
**Upgrade / rollback constraint:** once a table uses the SNII format,
the cluster can no longer be rolled back to a version that does not
support SNII. A pre-SNII backend reads the new storage-format enum as
the field's default (V1) rather than rejecting it, so it would silently
interpret SNII indexes as V1. Drop or convert such tables to V3 before
rolling back. Upgrades must follow the standard order: upgrade BEs
first, then FEs.
|
||
|
Jack
|
19c32c56a2 |
[fix](be) Key the index lookup map by each surviving index in remove_index (#66316)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
`TabletSchema::remove_index` rebuilds the `(index_type, col_unique_id,
index_suffix) -> position` lookup map after dropping an entry. The loop
walks each surviving index, but builds every map key from
`_indexes.back()` instead of the entry being registered:
```cpp
for (size_t new_pos = 0; new_pos < _indexes.size(); ++new_pos) {
const auto& index = _indexes[new_pos];
...
IndexKey key = std::make_tuple(_indexes.back()->index_type(), col_uid,
_indexes.back()->get_index_suffix());
_col_id_suffix_to_index[key].push_back(new_pos);
}
```
So every survivor is filed under the LAST survivor's index type and
suffix. When the survivors are homogeneous -- e.g. several INVERTED
indexes with no suffix, which is what the existing coverage uses --
every
key is identical and the result is accidentally correct. When they
differ,
it is not: after dropping one of two INVERTED indexes on a table that
also
carries an NGRAM_BF index, the surviving INVERTED index is filed under
`(NGRAM_BF, col, "")`, and `inverted_indexs()` -- which looks up
`IndexType::INVERTED` -- no longer finds it. A surviving index then
becomes
invisible to callers that resolve indexes through this map, including
the
segment writer and index compaction, with no error reported. The same
applies to indexes that differ only in suffix (variant sub-column
indexes).
The fix keys each entry by its own `index_type()` /
`get_index_suffix()`.
This also makes `remove_index` consistent with the other three sites
that
populate the same map (`append_index`, `init_from_pb`, and the
column-append path): those iterate right after
`_indexes.emplace_back(...)`,
where `_indexes.back()` IS the entry being registered, so they are
correct
as written and are left unchanged.
The map is a runtime cache rebuilt from scratch by `init_from_pb`, so a
schema that round-trips through protobuf is unaffected; only an
in-memory
schema that keeps being used after `remove_index` sees the stale
mapping.
|
||
|
Jack
|
1cd8d296d6 |
[improvement](be) Stop exec_env.h from pulling in the inverted index writer stack (#66262)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
`runtime/exec_env.h` included
`storage/index/inverted/inverted_index_writer.h`, which nothing in that
header actually needed: the two types it names,
`InvertedIndexSearcherCache` and `InvertedIndexQueryCache`, are already
forward declared a few lines below. The include was dead, but not free —
`exec_env.h` is reached by most of the backend, so it put the whole
inverted index writer stack (and through it CLucene) in front of ~1750
translation units. Editing any index or index-writer header therefore
rebuilt essentially all of BE.
Measured on this tree from the reverse include graph, before → after:
| header | TUs rebuilt before | after |
| --- | ---: | ---: |
| `storage/index/inverted/inverted_index_writer.h` | 1752 | **142** |
| `storage/index/index_file_writer.h` | 1754 | **254** |
Deleting the include alone does not compile, which is why it had
survived. The `#ifdef BE_TEST` accessor `set_tmp_file_dir` was defined
inline in the header:
```cpp
void set_tmp_file_dir(std::unique_ptr<segment_v2::TmpFileDirs> tmp_file_dirs) {
this->_tmp_file_dirs = std::move(tmp_file_dirs);
}
```
Assigning the `unique_ptr` destroys the old pointee, so `TmpFileDirs`
had to be **complete** in every translation unit that includes
`exec_env.h` — and that completeness was being supplied, by accident,
through the dead include. Removing the include without this fix breaks
every translation unit that instantiates the setter (29 of them on the
tree where this was first tried), all with the same `invalid application
of 'sizeof' to an incomplete type 'doris::segment_v2::TmpFileDirs'`.
Worth noting: the inline body only exists under `BE_TEST`, so production
builds were always clean and only the unit-test build exposed the
requirement. That is part of why the dead include went unnoticed.
The fix keeps the header free of the dependency — the accessor is
declared in the header and defined in `exec_env.cpp`, which includes
`storage/index/index_writer.h` for the complete type. The member
`std::unique_ptr<TmpFileDirs>` needs nothing further, because
`~ExecEnv()` is already out of line.
**Guard against regression.** Explaining the rule does not hold the
line; the include compiled fine for as long as it sat there. So
`build-support/check-header-deps.py` turns it into a build error. Rules
are declarative — per hub header, a subtree that must not be reachable
through any chain of includes, plus explicit exceptions and the reason
for each:
```python
RULES = [(
"runtime/exec_env.h",
"storage/index/",
{"storage/index/inverted/inverted_index_stats.h"}, # leaf POD struct via storage/olap_common.h
"ExecEnv only names index types as pointers and already forward-declares them; ...",
)]
```
On a violation it prints the offending include chain and the fix, so the
next person can see how they got there:
```
error: runtime/exec_env.h must not reach storage/index/*
chain: runtime/exec_env.h
-> storage/index/inverted/inverted_index_writer.h
-> storage/index/index_file_writer.h
fix: forward-declare the type in the header and include the real header in the
.cpp, or route it through a *_fwd.h
```
`--report` ranks headers by how many translation units they can force a
rebuild of, for finding the next one of these. Exceptions are listed
explicitly rather than inferred, so widening the barrier stays a
deliberate, reviewable act.
|
||
|
Jack
|
973f93f51f |
[fix](build) Let ScopedLSANDisabler compile without a sanitizer (#66049)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
`doris_be_test` can only be linked as an ASAN build.
`BUILD_TYPE_UT=DEBUG` and `BUILD_TYPE_UT=RELEASE`
both fail at link:
```
ld.lld: error: undefined symbol: __lsan_disable
>>> referenced by leak_annotations.h:84
>>> cached_remote_file_reader_peer_test.cpp.o:(doris::debug::ScopedLSANDisabler::ScopedLSANDisabler())
```
`be/src/util/debug/leak_annotations.h` computes `DORIS_LSAN_ENABLED` and
uses it to guard
`ANNOTATE_LEAKING_OBJECT_PTR`, but `ScopedLSANDisabler` sits outside
every `#if`:
```cpp
class ScopedLSANDisabler {
public:
ScopedLSANDisabler() { __lsan_disable(); }
~ScopedLSANDisabler() { __lsan_enable(); }
};
```
`__lsan_disable` / `__lsan_enable` are only declared in that header;
they are defined by the
LeakSanitizer runtime. Without `-fsanitize=address` (or `=leak`) nothing
provides them, so every
translation unit that instantiates the class emits an undefined
reference. Two do —
`be/src/util/debug/leakcheck_disabler.h` and
`be/test/io/cache/cached_remote_file_reader_peer_test.cpp`.
This matters beyond tidiness: it means there is no way to produce an
optimized, un-instrumented unit
test binary, so unit tests cannot be used for any timing work — ASAN's
allocator instrumentation and
`-O0` dominate whatever is being measured.
The guard is extended to cover the class. With no LeakSanitizer there is
nothing to suppress, so it
degrades to a no-op, which is the same reasoning
`ANNOTATE_LEAKING_OBJECT_PTR` already applies.
|
||
|
Jack
|
059f56dcf2 |
[fix](fe) Reject multi-column NGRAM_BF indexes (#64343)
### What problem does this PR solve? Related PR: None Problem Summary: Creating an NGRAM_BF index with multiple columns passed FE validation and could reach BE tablet creation, where tablet metadata expects each NGRAM_BF index to bind exactly one column. This rejects invalid multi-column NGRAM_BF definitions during FE analysis for both inline table indexes and CREATE INDEX. ### Release note Reject invalid multi-column NGRAM_BF index definitions during DDL analysis. |
||
|
Jack
|
c24d454f15 |
[fix](search) reject Lucene-syntax SEARCH on columns without inverted index (#63637)
## Proposed changes Issue Number: close #N/A ### What problem does this PR solve? SEARCH (Lucene syntax) predicates against columns that have no inverted index silently fall back to an empty bitmap on BE (`vsearch.cpp` and `function_search.cpp` only log a WARNING then `return Status::OK()` with an empty result), making the query look like *no rows matched*. That is indistinguishable from a successful query that simply found nothing and misleads users. This PR adds a planning-time check in `RewriteSearchToSlots`, matching the existing "column does not exist" behavior — fail fast with a clear `AnalysisException` instead of letting BE silently return FALSE. - **Normal columns**: require `OlapTable.getInvertedIndex(column, null) != null`. - **Variant subcolumns** (`parent.path`): require any `INVERTED` index whose first column equals the parent variant column; the concrete subcolumn binding is still resolved per-segment in BE, consistent with the `is_variant_sub` branch in `function_search.cpp`. Also hardens `OlapTable.getInvertedIndex` against NPE when the table has no `TableIndexes` set (returns `null` instead of dereferencing). Error message example: ``` Field 'msg_body' has no inverted index, cannot be used in search: msg_body:error. Create an inverted index on the column first (ALTER TABLE ... ADD INDEX ... USING INVERTED). ``` |
||
|
Jack
|
05690fce06 |
[fix](compile) disambiguate Aws::Utils::DateTime ctor in kinesis_conf (#63727)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
`be/src/load/routine_load/kinesis_conf.cpp:118` constructs
`Aws::Utils::DateTime` from `std::stol(...)`. The AWS SDK declares two
constructors:
```cpp
DateTime(int64_t millisSinceEpoch);
DateTime(double epoch_millis);
```
On macOS ARM64 (Apple LP64 with libc++), `int64_t` is typedef'd to `long
long` — distinct from the `long` that `std::stol` returns. Both `long ->
long long` and `long -> double` are same-rank conversions, so overload
resolution is ambiguous and the build fails:
```
error: ambiguous conversion for functional-style cast from 'long' to 'Aws::Utils::DateTime'
118 | request.SetTimestamp(Aws::Utils::DateTime(std::stol(it->second)));
note: candidate constructor DateTime(int64_t millisSinceEpoch);
note: candidate constructor DateTime(double epoch_millis);
```
Note: a naive switch to `std::stoll` would *symmetrically* break Linux
x86_64 (glibc) builds, where `int64_t` is typedef'd to `long` — there,
`long long` is ambiguous against `(long)` vs `(double)` for the same
reason.
The portable fix is to parse with `std::stoll` (explicit 64-bit,
semantically correct for milliseconds-since-epoch) and
`static_cast<int64_t>` to bind to the integer constructor regardless of
what `int64_t` resolves to on the target platform:
```cpp
request.SetTimestamp(
Aws::Utils::DateTime(static_cast<int64_t>(std::stoll(it->second))));
```
Verified with an overload-resolution mini-repro using the same clang-20
toolchain: only the explicit-cast form is unambiguous on both `int64_t =
long long` (macOS) and `int64_t = long` (Linux x86_64) targets.
|
||
|
Jack
|
3884c1aba8 |
[fix](function) fix tokenize function incorrect result when first argument is const (#62699)
## Proposed changes
Fix a bug in the `tokenize` function where `unpack_if_const` unwraps a
`ColumnConst` to its inner data column (which has only 1 row), but
`_do_tokenize` and `_do_tokenize_none` iterate based on the source
column's row count. This causes only 1 output row to be produced instead
of `input_rows_count` rows when the first argument is a constant.
For example, `SELECT tokenize('hello world', 'parser=english') FROM
table_with_many_rows` would previously return only 1 row instead of the
expected number of rows matching the table.
The fix wraps the result in `ColumnConst` when the source column was
const, which is the standard pattern used throughout the Doris codebase
for handling const columns in function execution.
## Further comments
Related Jira: DORIS-25296
## Checklist(Required)
1. Does it affect the results of the existing test cases (Yes/No): No
2. Does it need to update the document (Yes/No): No
3. Is there a risk of compatibility changes (Yes/No): No
|
||
|
Jack
|
f7af0aa970 |
[fix](compile) guard MD5 AVX2-only symbols under __AVX2__ (#63723)
### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: The constants `MD5_A0`/`MD5_B0`/`MD5_C0`/`MD5_D0` and the helpers `md5_num_blocks()` and `md5_pad_final_blocks()` in `be/src/util/md5.cpp` are only referenced by the AVX2 multi-buffer code path (`md5_multi_buffer_compute` / `md5_binary_batch_avx2`). On non-AVX2 builds (e.g. macOS ARM64) the AVX2 block is `#ifdef`'d out, leaving these symbols orphaned and triggering compile errors under the project's strict warnings: ``` error: unused variable 'MD5_A0' [-Werror,-Wunused-const-variable] error: unused variable 'MD5_B0' [-Werror,-Wunused-const-variable] error: unused variable 'MD5_C0' [-Werror,-Wunused-const-variable] error: unused variable 'MD5_D0' [-Werror,-Wunused-const-variable] error: unused function 'md5_pad_final_blocks' [-Werror,-Wunused-function] ``` This PR moves those AVX2-only constants and helpers inside the existing `#ifdef __AVX2__` block where they belong. `MD5_DUMMY_INPUT` and `md5_to_hex` stay outside the guard because the non-AVX2 fallback in `md5_hex_batch` still uses them. No behavior change on x86_64; restores build on ARM/non-AVX2 targets. |
||
|
Jack
|
03b7afda99 |
[fix](inverted index) Split bound multi-segment readers (#63138)
### What problem does this PR solve?
Issue Number: DORIS-25499
Related PR: None
Problem Summary:
query_v2 search collection can drive segment iteration from
`readers.front()` while leaf scorers resolve CLucene readers from
`reader_bindings` or `field_reader_bindings`. If the actual leaf reader
remains a multi-segment reader, `SegmentPostings` calls
`TermDocs::readBlock()`, which CLucene does not support for
multi-segment readers and can abort BE.
This PR selects the actual segmented reader from the execution context,
rewrites all readers and reader bindings to segment-level readers for
each callback, validates segmented reader topology, and keeps
`MultiSegmentReader` / `MultiReader` doc base handling explicit. It also
adds regression coverage for `MultiReader`, segmented field bindings,
and explicit single-reader binding keys.
### Release note
None
### Check List (For Author)
- Test
- [x] Regression test
- Added `MultiSegmentCollectorTest.CollectDocSetWithMultiReader`
- Added
`MultiSegmentCollectorTest.CollectDocSetWithSegmentedFieldBinding`
- Added `MultiSegmentCollectorTest.CollectDocSetWithSingleReaderBinding`
- [x] Unit Test
- `env CCACHE_DIR=/tmp/doris25499-master-ccache
CCACHE_TEMPDIR=/tmp/doris25499-master-ccache-tmp ./run-be-ut.sh --run
--filter=MultiSegmentCollectorTest.* -j 32`
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
- Behavior changed:
- [x] No.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
|
Jack
|
53132a19a9 |
[fix](be) Fix macOS BE build (#63230)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Fix BE compilation failures on macOS libc++ while keeping the Linux path on the original `std::atomic<std::shared_ptr<...>>` implementation. `MultiVersion` now uses the existing Doris `atomic_shared_ptr` fallback only when `USE_LIBCPP` is enabled, and `MemLimiter` avoids mixed `long`/`int64_t` template deduction in `std::max`. |
||
|
Jack
|
4d643725ef |
[fix](inverted index) resolve variant sub-column indexes for score() (#62992)
### What problem does this PR solve?
Issue Number: N/A
Related PR:
Problem Summary:
Fix `score()` query failing on variant sub-columns with:
```text
Index statistics collection failed: Score query is not supported without inverted index for column=<variant.subcolumn>
```
`MatchPredicateCollector::collect` previously used
`TabletSchema::inverted_indexs(int32_t col_unique_id, const std::string&
suffix_path)`, which only consults `_col_id_suffix_to_index`. Variant
sub-column indexes can also live in:
1. `_path_set_info_map` (`subcolumn_indexes` / `typed_path_set`), when
the parent variant index is inherited by sub-columns or a typed path
index is materialized.
2. `_index_by_unique_id_with_pattern`, when the inverted index is
created on the parent variant column with
`PROPERTIES("field_pattern"="...")`.
This PR fixes both paths:
- Use the column-aware `TabletSchema::inverted_indexs(const
TabletColumn&)` lookup so inherited / typed-path variant sub-column
indexes are resolved.
- Reuse the parent variant sub-column pattern matching logic from
`variant_util` to resolve `MATCH_NAME` / `MATCH_NAME_GLOB` templates,
then look up `inverted_index_by_field_pattern(parent_uid,
matched_pattern)`.
- Clone matched field-pattern index metadata and set the actual Lucene
field suffix so score collection uses keys such as
`<parent_uid>.<parent_col>.user.name`, while
`CollectInfo::owned_index_meta` keeps the cloned metadata alive.
Behaviour after this PR:
| Scenario | Behaviour |
| --- | --- |
| Variant sub-column index is inherited or materialized in
`_path_set_info_map` | Schema lookup succeeds through
`inverted_indexs(const TabletColumn&)`; score collection uses the real
sub-column Lucene field. |
| Parent variant index uses exact `field_pattern`, e.g. `host` | Pattern
lookup resolves the index and score collection uses
`<parent_uid>.<parent_col>.host`. |
| Parent variant index uses glob `field_pattern`, e.g. `user.*`, and
slot path is `user.name` | Parent template matching returns `user.*`;
index lookup uses the matched pattern; score collection uses
`<parent_uid>.<parent_col>.user.name`. |
| No index matches the sub-column | Existing unsupported-index error is
preserved; BM25 score still requires an inverted index. |
### Release note
Fix `score()` queries on variant sub-columns whose inverted index is
inherited, typed-path based, or defined through `field_pattern`.
|
||
|
Jack
|
791c56d85d |
[feature](inverted index) support DROP INDEX ON PARTITION for inverted index (#62181)
### What problem does this PR solve? Issue Number: close #xxx Problem Summary: Currently, `DROP INDEX` only supports table-level operations — it removes both the index metadata and physical index files across all partitions. In contrast, `BUILD INDEX` already supports partition-level operations via `BUILD INDEX ON PARTITION`. This PR adds the corresponding inverse operation: `DROP INDEX ON PARTITION`, which deletes only the physical inverted index files on specified partitions while preserving the table-level index definition. This is useful for: - Cleaning up index data on specific partitions to reclaim storage space - Selectively removing indexes from cold/archive partitions while keeping them on hot partitions **Syntax:** ```sql -- Standalone form DROP INDEX [IF EXISTS] idx_name ON table_name PARTITIONS (p1, p2, ...); -- ALTER TABLE form ALTER TABLE table_name DROP INDEX [IF EXISTS] idx_name PARTITION (p1); ``` **Key design decisions:** - Only inverted indexes are supported (other index types are rejected) - Table-level index metadata is NOT modified — the index definition remains in the schema - Queries on affected partitions automatically fallback to non-index scan via `enable_fallback_on_missing_inverted_index` (default: true) - Works in both storage-compute separation (cloud) and storage-compute integrated (local) modes - No BE-side or cloud meta service changes required — reuses existing `IndexChangeJob` and `buildOrDeleteTableInvertedIndices` infrastructure |
||
|
Jack
|
361bbed335 |
[fix](profile) populate inverted index metrics for SEARCH() query path (#62121)
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: SEARCH() function uses the query_v2 engine, which bypasses `InvertedIndexReader::query()`. This causes all inverted index profile metrics to report zero, making it impossible to diagnose SEARCH() performance issues from query profiles. Four independent problems were identified and fixed: **1. Query-level timers not populated (function_search.cpp)** The MATCH path populates `inverted_index_query_timer`, `searcher_open_timer`, `searcher_search_timer`, etc. via `SCOPED_RAW_TIMER` in `inverted_index_reader.cpp`. The SEARCH path bypasses this entirely — `FieldReaderResolver::resolve()` opens the index and `evaluate_inverted_index_with_search_param()` executes the query without any timer instrumentation. Fix: Add `SCOPED_RAW_TIMER` at corresponding points in `function_search.cpp`: - `inverted_index_query_timer` around the entire evaluation (placed before DSL cache lookup so cache-hit path is also timed) - `inverted_index_searcher_open_timer` around searcher cache miss (directory open) - `inverted_index_searcher_search_timer` around query build + execution - `inverted_index_searcher_search_init_timer` around `build_query_recursive()` - `inverted_index_searcher_search_exec_timer` around `collect_multi_segment_doc_set/top_k()` - `inverted_index_searcher_cache_hit/miss` counters in `resolve()` - `inverted_index_lookup_timer` around both DSL cache and searcher cache lookups - `inverted_index_query_cache_hit/miss` counters on DSL cache lookup **2. io_ctx dangling pointer in cached IndexSearcher (function_search.cpp)** `FieldReaderResolver::resolve()` opens the index directory with the caller's `io_ctx`, builds an IndexSearcher, and inserts it into the global `InvertedIndexSearcherCache`. The cached searcher's underlying `DorisCompoundReader` main stream retains a raw pointer to the caller's `io_ctx`. When the originating scan context is destroyed, this pointer dangles — any subsequent cache hit from a different query would read through a stale `io_ctx`, causing use-after-free (caught by ASAN). Fix: After building the IndexSearcher and before inserting into cache, clear the main stream's `io_ctx` via `stream->setIoContext(nullptr)`. Subsequent query-phase reads receive the caller's `io_ctx` through CLucene API parameters (`termDocs`/`termPositions`/`terms`) — the same pattern used by the MATCH path in `InvertedIndexReader::create_index_searcher()`. **3. Profile counters pruned by level (olap_scan_operator.cpp)** All inverted index counters were registered with default `level=2` via `ADD_TIMER`/`ADD_COUNTER`. The profile display system prunes level-2 leaf counters when `profile_level < 2`, making them invisible in query profiles. The FileCache counters used `ADD_CHILD_TIMER_WITH_LEVEL(..., 1)` and were visible. Fix: Register inverted index counters with `level=1` via `ADD_TIMER_WITH_LEVEL`/`ADD_COUNTER_WITH_LEVEL`. **4. Cache control and metric accuracy issues (function_search.cpp)** Code review identified several metric and cache control issues: - DSL query-cache hit path returned early before `inverted_index_query_timer` scope, resulting in `InvertedIndexQueryTime=0` for warm queries. - Searcher cache lookup was unconditional, ignoring `enable_inverted_index_searcher_cache` session variable. A second query with cache disabled could silently hit the cache. - Resolver local `_cache` hit (same field referenced multiple times within one DSL) was incorrectly counted as `inverted_index_searcher_cache_hit`, inflating the metric. - `InvertedIndexLookupTimer` was not instrumented in the SEARCH path. Fix: - Move `inverted_index_query_timer` before DSL cache lookup so cache-hit fast path is also timed. - Gate searcher cache lookup on `enable_inverted_index_searcher_cache` (matching MATCH path in `InvertedIndexReader::handle_searcher_cache`). Insert remains unconditional for consistency. - Remove false `inverted_index_searcher_cache_hit++` from resolver local `_cache` hit. - Add `SCOPED_RAW_TIMER` for `inverted_index_lookup_timer` around both DSL and searcher cache lookups. - Record `inverted_index_query_cache_hit/miss` on DSL cache lookup result. - Add regression tests: DSL query-cache hit path, searcher cache disabled with two runs. |
||
|
Jack
|
252c8f4c52 |
[fix](fe) Fix MATCH crash on alias slots and push down as virtual column (#61584)
### What problem does this PR solve? Issue Number: close #xxx Related PR: #61635 Problem Summary: When MATCH expressions reference alias slots that have lost column metadata (e.g., `CAST(variant_col['subkey'] AS VARCHAR) AS fn`), and the MATCH is in a predicate that cannot be pushed below a join (due to OR with join-dependent conditions like EXISTS mark or LEFT JOIN null checks), ExpressionTranslator's visitMatch() throws "SlotReference in Match failed to get Column". Root cause: `Alias.toSlot()` only preserves originalColumn/originalTable when its child is a direct SlotReference. When wrapped in Cast/ElementAt, all metadata is lost. Combined with OR preventing filter pushdown, the MATCH is stuck at the join layer referencing a metadata-less slot. Reproducer: ```sql WITH contacts AS ( SELECT objectId, CAST(overflowProperties['string_8'] AS VARCHAR) AS firstName FROM objects_small WHERE portalId = 865815822 ), lists AS ( SELECT objectId FROM lists_v2 WHERE portalId = 865815822 ) SELECT o.objectId FROM contacts o LEFT JOIN lists l ON o.objectId = l.objectId WHERE firstName MATCH_ANY 'john' OR l.objectId IS NOT NULL; -- ERROR: SlotReference in Match failed to get Column ``` This PR fixes the issue with two changes: 1. **Graceful fallback in visitMatch()**: When the slot has lost column/table metadata, fall back to `invertedIndex = null` instead of throwing. The BE evaluates MATCH correctly via slow-path expression evaluation, or the virtual column mechanism (below) provides fast-path index evaluation. 2. **New rewrite rule PushDownMatchPredicateAsVirtualColumn**: Extracts MATCH from join/filter predicates, traces the alias slot back through the Project to find the original column expression, and creates a virtual column on OlapScan. The BE evaluates the virtual column via inverted index using fast_execute(), and the join layer references the boolean result. Plan transformation: ``` Before: Filter(fn MATCH_ANY 'john' OR l.objectId IS NOT NULL) ← throws error or slow path └── Join → Project[CAST(col) as fn] → OlapScan After: Filter(__match_vc OR l.objectId IS NOT NULL) ← boolean reference, no error └── Join → Project[fn, __match_vc] → OlapScan[virtualColumns=[(CAST(col) MATCH_ANY 'john')]] ↑ inverted index fast path ``` ### Before / After Examples #### 1. MATCH on CAST(variant[...]) alias + OR + JOIN — previously failed, now works with index acceleration ```sql -- ❌ Before: FE error "SlotReference in Match failed to get Column" -- ✅ After: MATCH pushed down as virtual column, inverted index fast path -- Scenario A: CTE + LEFT JOIN + OR WITH objects AS ( SELECT objectId, CAST(properties['string_8'] AS VARCHAR) AS firstName FROM objects_v3 ) SELECT o.objectId FROM objects o LEFT JOIN lists l ON o.objectId = l.objectId WHERE firstName MATCH_ANY 'john' OR l.objectId IS NOT NULL; -- Scenario B: Subquery + LEFT JOIN + OR SELECT o.objectId FROM (SELECT objectId, CAST(props['name'] AS VARCHAR) AS name FROM t1) o LEFT JOIN t2 l ON o.objectId = l.objectId WHERE name MATCH_ALL 'smith' OR l.objectId IS NOT NULL; -- Scenario C: Multiple MATCHes in different OR branches -- Both pushed down as __DORIS_VIRTUAL_COL__1 and __DORIS_VIRTUAL_COL__2 WHERE (CAST(props['first'] AS VARCHAR) MATCH_ALL 'john' AND list_condition) OR (other_list_condition AND CAST(props['last'] AS VARCHAR) MATCH_ALL 'patel'); ``` #### 2. MATCH on the right side of a JOIN — previously not supported, now works ```sql -- ❌ Before: MATCH on right side of JOIN never triggered pushdown, fell back to slow path -- ✅ After: Both left and right sides supported equally -- RIGHT JOIN: main table on right side SELECT * FROM lists l RIGHT JOIN ( SELECT objectId, CAST(props['name'] AS VARCHAR) AS fn FROM objects_v3 ) o ON l.objectId = o.objectId WHERE fn MATCH_ANY 'hello' OR l.listId IS NOT NULL; -- INNER JOIN: optimizer may place Project→OlapScan on either side SELECT * FROM t1 JOIN ( SELECT id, CAST(data['title'] AS VARCHAR) AS title FROM t2 ) sub ON t1.id = sub.id WHERE title MATCH_ALL 'keyword' OR t1.status = 1; ``` #### 3. Real-world business query — previously failed entirely, now works end-to-end ```sql -- ❌ Before: entire query fails -- ✅ After: executes correctly, total_count accurate -- Pagination + count + CTE referenced multiple times WITH results AS ( SELECT o.objectId, CAST(o.properties['first'] AS VARCHAR) AS firstName, agg.listIds FROM objects o LEFT JOIN (SELECT objectId, array_agg(listId) AS listIds FROM lists GROUP BY objectId) agg ON o.objectId = agg.objectId WHERE (CAST(o.properties['first'] AS VARCHAR) MATCH_ALL 'john' AND array_size(array_intersect(agg.listIds, array(456))) = 1) OR (array_size(array_intersect(agg.listIds, array(123))) = 1 AND CAST(o.properties['last'] AS VARCHAR) MATCH_ALL 'patel') ) SELECT * FROM ( SELECT NULL AS total_count, objectId FROM results LIMIT 10 UNION ALL SELECT count(*), NULL FROM results ) t; ``` |
||
|
Jack
|
0b5f57f421 |
[fix](inverted index) Make select_best_reader deterministic for multi-index columns (#61596)
### What problem does this PR solve? Issue Number: close #DORIS-24685 Related PR: N/A Problem Summary: When multiple inverted indexes with different analyzers exist on the same column, `select_best_reader()` returns the first matching candidate based on iteration order of `_reader_entries`. Since `_reader_entries` ordering depends on the rowset schema's index ordering, and different segments can have different orderings (e.g. after sequential `BUILD INDEX` operations), the same query selects different indexes for different segments, producing inconsistent results. Fix: Replace all order-dependent candidate selection in `select_for_text()`, `select_for_numeric()`, and `select_best_reader()` with deterministic selection by smallest `index_id` via `pick_preferred()` and `pick_smallest_index_id()` helpers. This ensures consistent index selection regardless of schema ordering across segments. |
||
|
Jack
|
574b84fa9b |
[fix](search) Fix slash character in search query_string terms (#61599)
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: The ANTLR lexer in the search() DSL parser excluded `/` from `TERM_CHAR`, causing terms like `AC/DC` to be incorrectly tokenized. The slash was silently skipped by ANTLR's default error recovery, splitting `AC/DC` into two separate terms `AC` and `DC` instead of treating it as a single term. This caused inconsistent behavior compared to Elasticsearch's query_string parsing, where `AC\/DC` (escaped slash) is handled as a single analyzed term. **Fix**: Add `/` to the `TERM_CHAR` fragment in `SearchLexer.g4`. This allows `/` to appear within terms (e.g., `AC/DC` -> single term) while regex patterns like `/[a-z]+/` still work correctly since `/` remains excluded from `TERM_START_CHAR`. |
||
|
Jack
|
fd05761931 |
[fix](build) Fix --whole-archive linker error on macOS (#61291)
### What problem does this PR solve? Issue Number: N/A Related PR: #61285 Problem Summary: macOS `ld64` does not support GNU ld's `--whole-archive` / `--no-whole-archive` linker options, causing link failure when building BE on macOS: ``` ld: unknown options: --whole-archive --no-whole-archive ``` This PR uses macOS-native `-force_load` with CMake's `$<TARGET_FILE:lib>` generator expression on Apple platforms instead. Fixes both `doris_be` (paimon factory registry libs) and `doris_be_test` (vector_search_test) link targets. |
||
|
Jack
|
afb36bb2e1 |
[fix](search) Replace ExcludeScorer with AndNotScorer for MUST_NOT to handle NULL rows (#61200)
### What problem does this PR solve?
Issue Number: close #DORIS-24681
Problem Summary:
`search('NOT msg:omega')` incorrectly includes NULL rows in the result
set, while `NOT search('msg:omega')` correctly excludes them.
**Root cause:** `OccurBooleanWeight::complex_scorer()` used
`ExcludeScorer` for MUST_NOT clause handling. `ExcludeScorer` does not
implement `has_null_bitmap()` / `get_null_bitmap()`, inheriting the
`Scorer` base class defaults that always return `false` / `nullptr`.
This caused NULL documents to be treated as TRUE (matching) rather than
NULL, violating SQL three-valued logic where `NOT(NULL) = NULL`.
**Fix:** Replace `ExcludeScorer` with `AndNotScorer` which correctly
implements three-valued logic:
- `NOT(TRUE) = FALSE` → excluded from results
- `NOT(FALSE) = TRUE` → included in results
- `NOT(NULL) = NULL` → placed in null bitmap, filtered by
`mask_out_null()`
Also plumb `binding_keys` from `function_search.cpp` through the query
builder chain (`OccurBooleanQueryBuilder` → `OccurBooleanQuery` →
`OccurBooleanWeight`) so that `per_occur_scorers()` can resolve the
correct logical field for each sub-weight, enabling proper null bitmap
fetching from the `NullBitmapResolver`.
### Release note
Fix search('NOT field:value') incorrectly including NULL rows by using
null-bitmap-aware AndNotScorer instead of ExcludeScorer.
|
||
|
Jack
|
675f5e89da |
[fix](build) Fix Boost sigtimedwait compile error on macOS (#61285)
### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: macOS does not implement `sigtimedwait` despite declaring `_POSIX_C_SOURCE >= 199309L`. Boost 1.81's `config.hpp` incorrectly enables `BOOST_POSIX_HAS_SIGTIMEDWAIT` on macOS, causing compile failures in `boost/process/detail/posix/wait_for_exit.hpp`. This issue was newly exposed by the Python UDF feature (#59543), which introduced the first usage of `boost/process.hpp` in the BE codebase via the include chain: `aggregate_function_python_udaf.cpp → python_udaf_client.h → python_client.h → python_udf_runtime.h → boost/process.hpp` This PR adds a thirdparty patch to exclude `__APPLE__` from the sigtimedwait detection macro, so Boost falls back to its fork-based `wait_for_exit` implementation. The patch is only applied on Darwin and does not affect Linux builds. |
||
|
Jack
|
807de1f692 |
[feature](search) support MATCH projection as virtual column for inverted index evaluation (#61092)
### What problem does this PR solve? Issue Number: close #xxx Problem Summary: In FULL OUTER JOIN queries, MATCH expressions in the SELECT list cannot be pushed down as filters (this would violate join semantics by incorrectly filtering rows). This means the inverted index cannot be used for MATCH evaluation, resulting in slow-path expression evaluation. This PR enables MATCH expressions used as **projections** to be pushed down as virtual columns on OlapScan, allowing the BE to evaluate them via inverted index using the existing `fast_execute()` caching mechanism. **Example:** ```sql -- Before: MATCH evaluated via slow path (no index) SELECT A.k1, A.content MATCH_ANY 'hello' as match_result FROM A FULL OUTER JOIN B ON A.k1 = B.k1; -- After: MATCH pushed as virtual column, evaluated via inverted index ``` **FE changes:** - `Match.java`: Add `PreferPushDownProject` interface so `PushDownProject` rule moves MATCH from join output into scan projections - `PushDownMatchProjectionAsVirtualColumn.java`: New rewrite rule converting MATCH projections to virtual columns on OlapScan - `RuleType.java` + `Rewriter.java`: Rule registration **BE changes (segment_iterator.cpp):** - `_construct_compound_expr_context()`: Set shared `IndexExecContext` on virtual column exprs - `_apply_index_expr()`: Evaluate inverted index for virtual column MATCH (bitmap only, no row filtering) - `_output_index_result_column_for_expr()`: Convert bitmap to UInt8 column for all index contexts (common exprs + virtual column exprs) The bitmap result is cached in `IndexExecContext`, and when `_materialization_of_virtual_column()` calls `VirtualSlotRef::execute_column()` → MATCH's `fast_execute()`, it returns the pre-computed column directly. |
||
|
Jack
|
b96cde9e00 |
[fix](inverted index) fix StringRef to std::string reinterpret_cast overflow (#61120)
## Proposed changes Fix `reinterpret_cast<std::string*>` on `StringRef*` causing buffer overflow on ARM64, where `std::string` is 24 bytes but `StringRef` is only 16 bytes. ### 1. `function_multi_match.cpp` Convert `StringRef` to `std::string` before passing as `query_value`. Downstream `FullTextIndexReader::query()` does `reinterpret_cast<const std::string*>(query_value)`, reading 8 bytes past the `StringRef` buffer. ### 2. `in_list_predicate.h` Fix 3 sites where `HybridSet` iterator's `get_value()` returns `StringRef*`, but code casts it to `std::string*`. Added `if constexpr (is_string_type(Type))` guard to safely construct `std::string` from `StringRef::data`/`StringRef::size`. |
||
|
Jack
|
6428df3f0b |
[chore](codeowners) add code owner for inverted index and search functions (#61209)
## Proposed changes
Add code ownership for inverted index and search-related files:
- `be/src/storage/index/inverted/` directory
- `be/src/exprs/function/function_search.{h,cpp}`
- `be/src/exprs/function/match.{cpp,h}`
- `be/src/exprs/vsearch.{h,cpp}`
|
||
|
Jack
|
5ba07538a7 |
[improvement](search) change default mode of search function from standard to lucene (#61055)
### What problem does this PR solve?
Issue Number: close #xxx
Problem Summary:
Change the default `mode` of the `search()` function from `"standard"`
to `"lucene"`.
Previously, when users called `search(dsl)` or `search(dsl, options)`
without explicitly specifying the `mode` parameter, it defaulted to
`"standard"` mode. This PR changes the default to `"lucene"` mode
(ES/Lucene-style boolean parsing), which is more commonly expected by
users familiar with Elasticsearch query_string syntax.
Users who need the standard mode can still explicitly specify
`'{"mode":"standard"}'` in the options parameter.
|
||
|
Jack
|
6d4e218a2e |
[fix](search) inject MATCH_ALL_DOCS for multi-MUST_NOT queries in lucene mode (#60891)
### What problem does this PR solve? Related PR: #60814 Problem Summary: In search() lucene mode, when all terms in a boolean query are MUST_NOT (e.g., `NOT a AND NOT b` or `NOT a NOT b` with default_operator=AND), the query incorrectly returns all documents instead of returning all documents EXCEPT those matching the negated terms. Root cause: Lucene's BooleanQuery with only MUST_NOT clauses matches nothing (by design). ES handles this by injecting a MatchAllDocsQuery with SHOULD occur. Doris only handled the single-term MUST_NOT case but not multi-term all-MUST_NOT queries. Fix: After `applyLuceneBooleanLogic()`, detect if ALL terms are MUST_NOT and inject `MATCH_ALL_DOCS(SHOULD)` with `minimum_should_match=1`. |
||
|
Jack
|
a4091ad5c6 |
[fix](search) fix MATCH_ALL_DOCS losing occur attribute in multi-field expansion (#60873)
### What problem does this PR solve? Issue Number: close #DORIS-24561 Problem Summary: In lucene mode with multi-field queries (e.g., `best_fields` with `fields: ["title", "content"]`), the query `"Lauren Boebert" OR *` returns results inconsistent with Elasticsearch. **Root cause:** During multi-field expansion (`expandNodeCrossFields`, `deepCopyWithField`, `setFieldOnLeaves`), `MATCH_ALL_DOCS` nodes are recreated without preserving the `occur` attribute (e.g., `SHOULD`). The BE defaults `occur=null` to `MUST`, changing the query semantics: - **Expected (ES behavior):** `SHOULD(phrase) OR SHOULD(match_all)` = all documents - **Actual (bug):** `SHOULD(phrase) AND MUST(match_all)` = only phrase-matching documents **ES explain** for `"Lauren Boebert" OR *`: ``` (title:"lauren boebert" | content:"lauren boebert") (ConstantScore(FieldExistsQuery [field=content]) | ConstantScore(FieldExistsQuery [field=title])) ``` Returns 1,000,000 docs (all). Doris was returning only phrase-matching docs. **Fix:** Preserve the `occur` attribute when creating new `MATCH_ALL_DOCS` nodes in all three multi-field expansion methods. |
||
|
Jack
|
8a076456db |
[test](search) Add regression test for wildcard query on variant subcolumns with multi-index (#60834)
### What problem does this PR solve? Related PR: #60793 Problem Summary: Wildcard queries (*, ?) on variant subcolumns with multiple inverted indexes (one without analyzer, one with standard/lowercase analyzer) previously returned empty results even when regular TERM search worked correctly. This PR adds a regression test (`test_search_variant_wildcard_custom_analyzer`) to verify the fix from #60793 with a more realistic scenario matching the HubSpot contacts table pattern: - Variant column with dual field_pattern indexes (none + standard/lowercase) - firstname/lastname stored in variant subcolumns (`string_8`, `string_17`) - 13 test cases covering: TERM baseline, leading/middle/trailing/single-char wildcards, cross-field AND, and match-all wildcard |
||
|
Jack
|
1cde086c55 |
[fix](search) Fix implicit conjunction incorrectly modifying preceding term in lucene mode (#60814)
### What problem does this PR solve? Issue Number: close #DORIS-24545 Problem Summary: In `search()` function's lucene mode, queries with mixed explicit and implicit operators produce different results from Elasticsearch. For example: - Query: `"Sumer" OR Ptolemaic\ dynasty Limonene` with `default_operator=AND` - ES result: 1 row - Doris result: 0 rows (before fix) **Root cause:** In Lucene's `QueryParserBase.addClause()`, only explicit `CONJ_AND`/`CONJ_OR` modify the preceding term's occur. Implicit conjunction (`CONJ_NONE`, i.e., space-separated terms without an explicit operator) only affects the **current** term via `default_operator`, without modifying the preceding term. The FE `SearchDslParser.hasExplicitAndBefore()` incorrectly returned `true` (based on `default_operator`) when no explicit AND token was found. This caused implicit conjunction to be treated identically to explicit AND, making it modify the preceding term's occur — diverging from Lucene/ES semantics. **Example of the bug:** For `a OR b c` with `default_operator=AND`: - Before fix: `SHOULD(a) MUST(b) MUST(c)` — wrong, implicit space before `c` incorrectly upgraded `b` from SHOULD to MUST - After fix: `SHOULD(a) SHOULD(b) MUST(c)` — correct, matches ES behavior. Only `c` gets MUST (from default_operator), `b` retains SHOULD (from the preceding OR) **Fix:** `hasExplicitAndBefore()` now returns `false` when no explicit AND token is found, regardless of `default_operator`. Only explicit AND tokens trigger the "introduced by AND" logic that modifies preceding terms. |
||
|
Jack
|
eff59f423f |
[fix](search) Use FE-provided analyzer key for multi-index columns in search() (#60798)
### What problem does this PR solve? Issue Number: close #DORIS-24542 Problem Summary: When a column has multiple inverted indexes with different analyzers (e.g., one default untokenized index and one with English parser), `search()` in Lucene/scalar mode returns empty results. **Root cause:** In `FieldReaderResolver::resolve()`, `select_best_reader()` was always called with an empty analyzer key `""`, causing it to pick the wrong (untokenized) index for tokenized queries. Additionally, the EQUAL_QUERY → MATCH_ANY_QUERY upgrade was restricted to variant subcolumns only. **Fix:** 1. Extract `analyzer_key` from FE-provided `index_properties` before calling `select_best_reader()` and pass it through 2. Remove the `is_variant_sub` restriction on the query type upgrade so regular columns with multiple indexes also get the correct FULLTEXT reader |
||
|
Jack
|
ee7bfe3894 |
[fix](search) Validate mode parameter in search() DSL options (#60785)
### What problem does this PR solve? Issue Number: close #DORIS-24352 Problem Summary: When using `search()` with an invalid `mode` value in the options JSON (e.g., `"mode":"lucenedfafa"`), the query silently falls back to standard mode instead of reporting an error. This makes it difficult for users to detect typos or misconfiguration. This PR adds validation in `SearchOptions.validate()` to reject any mode value other than `"standard"` or `"lucene"`, throwing a clear error message. ### Release note Fix search() DSL to report an error when an invalid mode value is specified instead of silently falling back to standard mode. |
||
|
Jack
|
7ee1bc437f |
[fix](search) Fix wildcard query on variant subcolumns returning empty results (#60793)
### What problem does this PR solve? Related PR: #60782 Problem Summary: When `search()` DSL uses wildcard patterns (e.g. `*ith`, `sm*th`, `sm?th`) on variant subcolumns with analyzer-based indexes (field_pattern), the queries return empty results even though regular TERM search works correctly. **Root cause:** In `FieldReaderResolver::resolve()`, only `EQUAL_QUERY` was upgraded to `MATCH_ANY_QUERY` for variant subcolumns with analyzer-based indexes. `WILDCARD_QUERY` was not upgraded, so `select_best_reader()` picked the `STRING_TYPE` reader instead of `FULLTEXT`. `WildcardWeight` then enumerated terms from the wrong (untokenized) index directory, finding no matches. **Fix:** Extend the query type upgrade condition to also cover `WILDCARD_QUERY`, so wildcard patterns correctly use the FULLTEXT index on variant subcolumns. Also fix a misleading comment in `inverted_index_iterator.cpp` where `is_equal_query()` was described as handling WILDCARD/REGEXP but actually only checks `EQUAL_QUERY`. |
||
|
Jack
|
729b9f85b6 |
[fix](search) Add searcher cache reuse and DSL result cache for search() function (#60790)
### What problem does this PR solve? Problem Summary: This PR adds searcher cache reuse and a DSL result cache for the `search()` function to improve query performance on repeated search queries against the same segments. **Key changes:** 1. **DSL result cache**: Caches the final roaring bitmap per (segment, DSL) pair so repeated identical `search()` queries skip Lucene execution entirely. Uses length-prefix key encoding to avoid hash collisions. 2. **Deep-copy bitmap semantics**: Bitmaps are deep-copied on both cache read and write to prevent `mask_out_null()` from polluting cached entries. 3. **Type-safe cache accessor**: Replaces raw `void*` return with a template `get_value<T>()` that uses `static_assert` to ensure T derives from `LRUCacheValueBase`. 4. **Session-level cache toggle**: Adds `enable_search_function_query_cache` session variable (default: true) to allow disabling the cache per query via `SET_VAR`. 5. **Const-correctness fix**: Removes unsafe `const_cast` in `build_dsl_signature` by copying TSearchParam before Thrift serialization. 6. **Defensive improvements**: Adds null check for `result_bitmap` on cache hit, logging for serialization fallback and cache bypass paths. ### Release note Add DSL result cache for search() function to skip repeated Lucene execution on identical queries. |
||
|
Jack
|
6cd22c9fdc |
[feat](search) Support field-grouped query syntax field:(term1 OR term2) (#60786)
### What problem does this PR solve?
Issue Number: close #N/A
Problem Summary:
The `search()` function did not support ES `query_string` field-grouped
syntax where all terms inside parentheses inherit the field prefix:
```sql
-- Previously failed with syntax error
SELECT * FROM t WHERE search('title:(rock OR jazz)', '{"fields":["title","content"]}');
```
ES semantics:
| Input | Expansion |
|-------|-----------|
| `title:(rock OR jazz)` | `(title:rock OR title:jazz)` |
| `title:(rock jazz)` with `default_operator:AND` | `(+title:rock
+title:jazz)` |
| `title:(rock OR jazz) AND music` with `fields:[title,content]` |
`(title:rock OR title:jazz) AND (title:music OR content:music)` |
| `title:("rock and roll" OR jazz)` | `(title:"rock and roll" OR
title:jazz)` |
### Root cause
The ANTLR grammar `SearchParser.g4` defined `fieldQuery : fieldPath
COLON searchValue` where `searchValue` only accepts leaf values (TERM,
QUOTED, etc.), not a parenthesized sub-clause. So `title:(` caused a
syntax error.
### Solution
**Grammar** (`SearchParser.g4`):
- Add `fieldGroupQuery : fieldPath COLON LPAREN clause RPAREN` rule
- Add it as alternative in `atomClause` before `fieldQuery`
**Visitor** (`SearchDslParser.java`):
- Add `markExplicitFieldRecursive()` helper — marks all leaf nodes in a
group as `explicitField=true` to prevent `MultiFieldExpander` from
re-expanding them across unintended fields
- Modify `visitBareQuery()` in both `QsAstBuilder` and
`QsLuceneModeAstBuilder` to use `currentFieldName` as field group
context when set
- Add `visitFieldGroupQuery()` to both AST builders: sets field context,
visits inner clause, marks all leaves explicit
- Update `visitAtomClause()` and `collectTermsFromNotClause()` to handle
|
||
|
Jack
|
f77e7b5f1a |
[fix](search) Fix MATCH_ALL_DOCS query failing in multi-field search mode (#60784)
### What problem does this PR solve?
Issue Number: close
Problem Summary:
When using `search('*', ...)` with multi-field options (`fields`
parameter), the query fails with:
```
only inverted index queries are supported
```
The root cause is in `SearchDslParser.java`: the multi-field parsing
methods (`parseDslMultiFieldMode` and `parseDslMultiFieldLuceneMode`)
collect field bindings by calling `collectFieldNames()` on the expanded
AST. When the query is `*` (match all), the AST node is `MATCH_ALL_DOCS`
which has no field set — by design it matches all documents regardless
of field. This caused `collectFieldNames` to return an empty set,
resulting in no field bindings. Without field bindings,
`RewriteSearchToSlots` couldn't create slot references, so the search
expression was never pushed down to the inverted index path, and BE fell
back to `execute_impl()` which returns the error.
**Fix**: After `collectFieldNames()`, if the result is empty, fall back
to using the original `fields` list as field bindings. This ensures the
push-down mechanism works for `MATCH_ALL_DOCS` queries.
**Reproducing queries** (from bug report):
```sql
select count(*) from wikipedia where search('*', '{"fields":["title", "content"], "type": "best_fields", "default_operator":"AND","mode":"lucene", "minimum_should_match": 0}');
select count(*) from wikipedia where search('*', '{"default_field": "title", "default_operator":"AND","mode":"lucene", "minimum_should_match": 0}');
```
|
||
|
Jack
|
9a9b3b2717 |
[fix](search) Upgrade query type for variant subcolumns with analyzer-based indexes (#60782)
### What problem does this PR solve? Issue Number: close #xxx Related PR: #60654 Problem Summary: Follow-up fix for #60654 (SearchDslParser refactoring). When FE resolves a variant subcolumn field pattern to a specific analyzer-based index and sends its `index_properties` via `TSearchFieldBinding`, the BE `FieldReaderResolver` was using `EQUAL_QUERY` for TERM clauses. This caused `select_best_reader` to pick the `STRING_TYPE` reader (untokenized index directory) instead of the `FULLTEXT` reader, so tokenized search terms would never match. **Root cause**: For variant subcolumns with analyzer-based indexes, `EQUAL_QUERY` opens the wrong (untokenized) index directory. The query type needs to be upgraded to `MATCH_ANY_QUERY` so `select_best_reader` picks the correct FULLTEXT reader. **Fix**: In `FieldReaderResolver::resolve()`, when the field is a variant subcolumn and the FE-provided `index_properties` indicate an analyzer-based index (`should_analyzer()` returns true), automatically upgrade `EQUAL_QUERY` to `MATCH_ANY_QUERY` before calling `select_best_reader`. Also reuse the `fb_it` iterator to avoid a redundant map lookup. |
||
|
Jack
|
b450937158 |
[refactor](search) Refactor SearchDslParser to single-phase ANTLR parsing and fix ES compatibility issues (#60654)
### What problem does this PR solve? Problem Summary: The `search()` function's DSL parser had multiple ES compatibility issues and used a two-phase parsing approach (manual pre-parse + ANTLR) that was error-prone. This PR refactors the parser and fixes several bugs: 1. **SearchDslParser refactoring**: Consolidated from two-phase (manual pre-parse + ANTLR) to single-phase ANTLR parsing. The ANTLR grammar now handles all DSL syntax directly, eliminating the fragile manual pre-parse layer. This fixes issues with operator precedence, grouping, and edge cases. 2. **ANTLR grammar improvements**: Updated `SearchLexer.g4` and `SearchParser.g4` to properly handle quoted phrases, field-qualified expressions, prefix/wildcard/regexp patterns, range queries, and boolean operators with correct precedence. 3. **minimum_should_match pipeline**: Added `default_operator` and `minimum_should_match` fields to `TSearchParam` thrift, passing them from FE `SearchPredicate` through to BE `function_search`. When `minimum_should_match > 0`, uses `OccurBooleanQuery` for proper Lucene-style boolean query semantics. 4. **Wildcard/Prefix/Regexp case-sensitivity**: Wildcard and PREFIX patterns are now lowercased when the index has `parser + lower_case=true` (matching ES query_string normalizer behavior). REGEXP patterns are NOT lowercased (matching ES regex behavior where patterns bypass analysis). 5. **MATCH_ALL_DOCS support**: Added `MATCH_ALL_DOCS` clause type for standalone `*` queries and pure NOT query rewrites. Enhanced `AllQuery` with deferred `max_doc` from `context.segment_num_rows` and nullable field support via `NullableScorer`. 6. **BE fixes**: - `regexp_weight._max_expansions`: Changed from 50 to 0 (unlimited) to prevent PREFIX queries from missing documents - `occur_boolean_weight`: Fixed swap→append bug when all SHOULD clauses must match, preserving existing MUST scorers - Variant subcolumn `index_properties` propagation for proper analyzer selection - `lower_case` default handling: inverted index `lower_case` defaults to `"true"` when a parser is configured |