[opt](lance) add prefilter option for filter execution control by zhangstar333 · Pull Request #68813 · apache/doris

zhangstar333 · 2026-10-09T05:37:35Z

What problem does this PR solve?

Problem Summary:
doc: apache/doris-website#4203
add prefilter:bool to control filter exectue;
prefilter default is false, so will do ann search firstly, then execute filter.
prefilter turn true, will execute filter firstly, then do ANN search.

Release note

None

Check List (For Author)

Test
- Regression test
- 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
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

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

hello-stephen · 2026-10-09T05:37:40Z

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

Please clearly describe your PR:

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

zhangstar333 · 2026-10-09T06:29:44Z

/review

github-actions

Review complete for PR #68813 at 471b949. I request changes for two P1 correctness/compatibility issues. The seven inline findings are two P1, four P2, and one P3. No existing inline thread or additional user focus was present. All conclusions are from static inspection; builds and tests were prohibited and were not run.

Critical checkpoints:

Goal and proof: The change exposes opt-in Lance prefilter behavior, revises search/profile metrics, and rebases the Foyer cache. The added regression outputs illustrate the new postfilter result, but they are inspected expectations rather than executed proof. The stale BE prefilter assertions are M2.
Scope: FE TVFs/planning, Thrift, BE scanners/profiles, tests/docs, and the pinned third-party patch were all reviewed. The broad dependency patch is tied to the cache and scanner update; no further distinct scope issue was substantiated.
Concurrency and locks: Multiple scanner tasks can report statistics; RuntimeProfile protects counter-map insertion with a mutex and uses atomic counter updates. I found no separate lock-order/deadlock bug or heavy critical section beyond counter registration; cache sharing creates the origin collision in M5.
Lifecycle and static initialization: Scanner statistics arrive at EOF/close, and Foyer entries survive BE restarts. Callback allocation can escape the required non-throwing C boundary (M4), and recovered cache entries retain the origin collision (M5). No cross-translation-unit static-initialization dependency was found in the changed paths.
Configuration: The new TVF prefilter property defaults false; the BE-wide Foyer cache is already default-enabled. No new dynamic configuration item needs propagation. The default interacts with mixed-version execution as M1 describes.
Compatibility: Thrift field 5 is optional but schema_version remains 1. Old BEs ignore false and prefilter while new BEs postfilter old requests, changing query rows during rolling upgrades (M1). The pinned lance-c scanner/patch interfaces were checked statically.
Parallel paths and conditions: Vector and full-text, indexed and flat splits, two-phase fetch, prepared requests, TopK/offset, and __isset.prefilter were traced. Delayed fragment scoping reaches planning before TopK; no separate fragment bug was substantiated. Optional counter trees differ between split types and FE aggregation drops later-only groups (M7).
Tests and results: Added regression suites and .out rows were checked against the changed semantics; old BE filtered cases still assert prefilter behavior without setting the flag (M2). Mixed-version, distinct-endpoint cache, warm-write, and mixed-split aggregate coverage remain absent. No test or build was executed, so runtime and CI outcomes are unverified.
Observability: New Lance groups expose scanner metrics, but M7 can hide them in the aggregate profile, and the prefilter guide misstates the new default (M3). Existing group labels still render with TUnit.NONE; no other logging/metric gap was confirmed.
Transactions, persistence, and writes: No Doris table-write, EditLog, or transaction path changed. Foyer persists cache data and metadata: origin identity is missing from recovered keys (M5), and each warm read enqueues a redundant size-key write (M6). Recovery validation rejects corrupt/truncated entries; the separate short-block suspicion remains covered by M5.
FE/BE variable passing: The search request copies and forwards the new optional flag on current paths, but lacks a rolling-upgrade capability gate (M1).
Performance and remaining issues: M6 is the confirmed write-amplification path. All other suspicious points were either resolved with code evidence or folded into the seven inline findings; no unresolved candidate remains.

zhangstar333 · 2026-10-09T07:54:12Z

/review

github-actions · 2026-10-09T08:10:40Z

Codex automated review failed and did not complete.

Error: Review recovery stopped: No final review submission was declared
Workflow run: https://github.com/apache/doris/actions/runs/37901705711

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

zhangstar333 · 2026-10-09T08:20:35Z

/review

github-actions

Static review complete for head 65077a5afe817b0ccd4a2ad9b0045bc1ca9547a2 after two convergence rounds and a final sweep of all 23 changed paths. This was static inspection only; no build or test was run.

Findings and prior threads

New [P1] inline: prefilter=false (including omission) filters each search split before Doris merges candidates, so multi-split vector or FTS searches can return a row that snapshot-wide postfiltering would exclude.
Existing [P1] inline #4227447649 remains applicable: old and new FE/BE combinations interpret the omitted/new prefilter field differently. The author reply does not change the wire behavior. It is carried here without reposting.
Existing [P1] inline #4227447670 remains applicable: the BE-wide Foyer cache keys data and metadata by Lance's S3 bucket prefix and path, omitting per-catalog endpoint identity. It is carried here without reposting.
Existing [P2] inline #4227447675 remains applicable: warm cached_get calls still enqueue size-key disk writes. It is already reported. Earlier profile-tree, callback, documentation, and BE test concerns have been addressed at this head.

Critical checkpoints

Goal and correctness: FE parses the boolean and sends Thrift to both vector and FTS BE paths; true and false reach Lance. The new default and explicit false fail snapshot-wide search semantics when multiple physical splits are merged. Fixed snapshot versions and fragment ownership were traced; no separate version or delete-visibility defect was substantiated.
Scope and parallel paths: The search wiring is localized, while the PR also rebases the Foyer cache patch and reorganizes profile counters. Indexed and flat vector, FTS segment, offset, prepared query, and two-phase paths were checked. The split-ordering defect spans both search kinds; phase-two row-ID fetch cannot repair candidates filtered earlier.
Concurrency, memory, and lifecycle: The BE-wide session/cache is shared across queries, dataset handles retain their cache state, per-dataset statistics use atomics, and the wrapper registry uses a short mutex section. Counters are registered before installing Lance's callback; its noexcept wrapper catches failures. No additional lock-order, static-initialization, ownership, or memory-accounting defect was substantiated. The existing origin-key P1 remains a shared-cache correctness failure, including disk recovery.
Configuration and compatibility: No new Doris config is added; the existing data cache is enabled by default and initialized once per BE process. The optional Thrift field reaches both new BE search paths, but mixed-version FE/BE semantics remain incompatible as noted in existing P1 #4227447649. No storage-format or function-symbol change was identified beyond the reviewed lance-c API/dependency rebase.
Conditions, errors, and writes: Cache eligibility and conditional/versioned-read bypasses, BE Status returns, and reader close paths were checked. This PR does not change Doris transactions, EditLog, or table write atomicity. Foyer's persistent disk writes make the already-reported origin collision durable; warm size-key writes remain a performance issue.
Tests, observability, and performance: Changed BE tests, regression suites, and expected-output files cover true, false/default, and invalid boolean values in single-split cases; they do not cover a rejected global winner across two splits or distinct storage origins. The profile metric tree and callback fixes appear complete, and profile counter names were checked. No test result was independently executed. The remaining warm-read write amplification is already tracked by P2 #4227447675.

User focus: no additional focus was supplied. All new substantiated findings are included inline; already-reported issues were deduplicated. Review execution is complete with a blocking P1 verdict.

Existing P0/P1 findings confirmed for this head: #68813 (comment), #68813 (comment)

Gabriel39 · 2026-10-09T08:49:34Z

run buildall

hello-stephen · 2026-10-09T11:27:49Z

FE UT Coverage Report

Increment line coverage 33.33% (3/9) 🎉
Increment coverage report
Complete coverage report

zhangstar333 · 2026-10-09T14:46:56Z

run buildall

hello-stephen · 2026-10-09T15:15:33Z

FE UT Coverage Report

Increment line coverage 33.33% (3/9) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-10-09T15:55:29Z

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	78.52% (1963/2500)
Line Coverage	65.43% (35365/54048)
Region Coverage	53.07% (34022/64102)
Branch Coverage	55.82% (10476/18767)

hello-stephen · 2026-10-09T18:14:15Z

FE UT Coverage Report

Increment line coverage 33.33% (3/9) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-10-09T21:05:05Z

FE Regression Coverage Report

Increment line coverage 22.22% (2/9) 🎉
Increment coverage report
Complete coverage report

Issue Number: N/A Related PR: N/A Problem Summary: Doris pinned lance-c at 98468344bc9d56aa7ed4a4192e844afa62b34ba1 and missed upstream apache#91 vector post-filtering and scoped FTS prefiltering. Pin the dependency to 8e9e09ecc795c91ce7f97513db94c2d91eec799d, update the archive checksum, and regenerate the local Foyer patch against the new base while keeping its implementation unchanged. Include Lance-C vector post-filtering and scoped FTS prefiltering improvements. - Test: Static dependency validation: shell syntax checks, archive checksum validation, strict patch application, isolated downloader idempotence and source-tree comparison. No Doris build or unit/regression tests run. - Behavior changed: Yes; adopts upstream Lance-C apache#91 search-filter behavior. - Does this need documentation: No

### What problem does this PR solve? Issue Number: N/A Related PR: lance-format/lance-c#73 Problem Summary: Doris carried an older local Foyer patch. Refresh it from the current PR head 8a08f171b66a3e01e7ba634bcfc6168a955c7289 and rebase it onto the pinned Lance-C commit 8e9e09ecc795c91ce7f97513db94c2d91eec799d, preserving the newer upstream search and prewarm changes. The PR head includes its revert of the interim cache-origin-isolation change. ### Release note Update the bundled Lance-C Foyer cache integration to match upstream PR apache#73. ### Check List (For Author) - Test: Static validation: strict patch application, downloader refresh, isolated repeated downloader run, source-tree comparison, shell syntax, and diff checks. No build or tests run. - Behavior changed: Yes; the bundled Foyer implementation now matches the current PR head, including its cache-origin-isolation revert. - Does this need documentation: No

zhangstar333 · 2026-10-10T02:20:24Z

run buildall

Gabriel39 · 2026-10-10T03:01:34Z

/review

github-actions

Static review complete for PR #68813 at d6dcb81. No new inline findings survived the two-round review and final changed-file sweep. Three existing P1 inline findings still apply and remain blockers: 4227447649 (mixed-version FE/BE prefilter default), 4227447670 (Foyer cache keys omit storage-origin identity), and 4228261574 (postfilter runs inside each split before the global TopN). Existing P2 4227447675 still covers repeated size-key disk writes on warm reads; I am not reposting it. No additional user review focus was provided.

Critical checkpoints:

Goal, scope, and validation: I traced prefilter parsing through Thrift, BE vector/FTS scanner configuration and global TopN, plus the Foyer rebase and profile regrouping. The changed tests and expected outputs cover true, false, omitted, and invalid prefilter values on their fixtures; they do not resolve the existing multi-split postfilter blocker. The broad Cargo.lock change is tied to the pinned lance-c/Foyer rebase. This was static inspection only: no builds or tests were run.
Concurrency and lifecycle: BE scanner tasks share a lazily initialized process-wide Lance session and cache. I checked scanner close/EOF/abort, dataset wrapper ownership, cache locks and asynchronous fills, and the synchronous statistics callback. The callback updates pre-registered counters within a catch boundary, and I found no distinct new lock-order, deadlock, static-initialization, or ownership defect. Two further cache performance patterns considered during review already existed in the base patch, so they are not new PR findings.
Configuration and compatibility: Foyer cache configuration is restart-scoped; initialization failures surface as a status. Thrift field 5 is optional and does not reuse an id on the checked release branches, but the changed default still breaks mixed-version search semantics as described in existing P1 4227447649. FE sends the flag to both vector and FTS paths; the changed C API, dependency pin, checksum, and lockfile were checked for coherence.
Parallel paths and conditions: I checked vector flat/index splits, FTS segments, offset expansion, two-phase row-ID fetch, prepared search, and Doris residual filters. The per-split postfilter ordering remains the existing P1 4228261574; the offset and fetch paths did not establish a separate regression. The cache's endpoint alias remains existing P1 4227447670.
Tests and observability: The BE and regression cases, changed .out files, and profile documentation were inspected against current code. Grouped BE counters are registered before callbacks, including the phase-two timing parent; FE aggregation and printing match those names. Previously reported profile/callback issues appear addressed. No executed test result is claimed.
Visibility, persistence, writes, error and performance: Lance reads use the selected dataset version. No Doris transaction, EditLog, MoW, or product write path changes are in this diff; the cache wrapper forwards writes and persists disk entries. Origin identity remains the data-correctness risk above. Cache initialization and callback failures have explicit boundaries; I found no distinct new memory-tracking or silent-error defect. The warm size-key write amplification remains existing P2 4227447675. A short cached-block concern had no supported trigger under immutable Lance data files and Foyer recovery; other cold-fill and minimum-disk-capacity concerns predate this PR.

Existing P0/P1 findings confirmed for this head: #68813 (comment), #68813 (comment), #68813 (comment)

hello-stephen · 2026-10-10T05:05:10Z

FE UT Coverage Report

Increment line coverage 33.33% (3/9) 🎉
Increment coverage report
Complete coverage report

1

hello-stephen · 2026-10-10T08:04:33Z

FE Regression Coverage Report

Increment line coverage 10.71% (9/84) 🎉
Increment coverage report
Complete coverage report

### What problem does this PR solve? Problem Summary: doc: apache/doris-website#4203 add prefilter:bool to control filter exectue; prefilter default is false, so will do ann search firstly, then execute filter. prefilter turn true, will execute filter firstly, then do ANN search. ### Release note None ### Check List (For Author) - Test  - [x] Regression test - [ ] 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  - Behavior changed: - [ ] No. - [ ] Yes.  - Does this need documentation? - [ ] No. - [ ] Yes.  ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label

…file as in branch-4.x ### What problem does this PR solve? Issue Number: None Related PR: apache#69581, apache#68813 Problem Summary: apache#69581 ports the Lance reader from the branch-4.2 snapshot cabcf45. After that snapshot, apache#68813 changed two things in the 4.x BE, both ported here. 1. Search filter timing. apache#68813 added `TExternalSearchRequest.prefilter` (field 5): unset or false means the TVF search filter refines the top-k candidates (postfilter), true means it constrains them before top-k (prefilter). The 4.x FE sends it for `vector_search()` and `full_text_search()`. Without the field, a master BE always prefiltered vector search and never prefiltered full-text search, so a 4.x FE paired with it got different rows: on the all_types.lance fixture, top_k 1 with search filter `row_id >= 3` returns row 4 with prefilter and no row with postfilter. - `gensrc/thrift/PlanNodes.thrift`: `5: optional bool prefilter` with the 4.x comment; the stale `TSearchFilter` comment is replaced. - `_configure_vector_search()` and `_configure_full_text_search()` pass `request.__isset.prefilter && request.prefilter` to `lance_scanner_set_prefilter()`, replacing the comment that fragment-scoped nearest queries need prefiltering (master's lance-c pin 4fafcd64525 no longer does). - Compatibility (gensrc/AGENTS.md): field 5 is `optional bool prefilter` on the branch-4.2 (a7ae6f4) and branch-4.1 (3f911d0) heads; branch-4.0 and branch-3.1 have no `TExternalSearchRequest`. No id is reused. 2. Profile layout. The snapshot hangs about 45 counters directly under `LanceReader`, looks Lance's scan metrics up in two maps, and creates two scalar-index timers from Lance's statistics callback the first time they are reported, that is from Lance's execution threads while the scan runs. As in apache#68813: - The counters are grouped under `LanceTiming`, `LanceIO`, `LanceIndex`, `LanceVectorIndex`, `LanceScalarIndex`, `LanceExecutionPlan`, `LanceScan`, `LanceSearchPlan` and `LanceDataCache` (label counters, `TUnit::NONE`, like `RuntimeFilterInfo`). - One immutable table, `LANCE_SCAN_METRICS` in `lance_reader_helper.h`, drives both registration and the callback lookup. Every metric is registered in `_init_scanner_profile()` before the callback is installed, so the callback only updates existing counters and every split has the same counter tree. - The callback is `noexcept` and swallows exceptions, so nothing unwinds through the lance-c ABI. - `LanceIndexObjectLoads` (`indices_loaded`), `LanceIndexCacheHits` and `LanceIndexCacheMisses` are new; the info strings `LanceTopKPlusOffset` and `LanceSubstraitFilterBytes` are dropped. - Renames: `LanceScannerReadTime` -> `LanceScannerNextTime`, `LanceRuntimeFilterSqlTime` -> `LanceRuntimeFilterSqlTranslationTime`, `LanceExecutionIOOps` / `IORequests` / `IOBytesRead` -> `LanceIOReadOps` / `LanceIOReadRequests` / `LanceIOReadBytes`, `LanceIndexPartitionCacheMissLoads` -> `LanceIndexComponentsLoaded`, `LancePrefilter*` -> `RowIdPrefilter*` (`LancePrefilterRowIds` -> `RowIdPrefilterIds`), `LanceIVFPartitionsRanked` -> `LanceIVFPartitionsConsidered`, `LanceIVFPartitionsSearched` -> `LanceIndexPartitionsSearched`, `LanceVectorIndexSegmentsSearched` -> `LanceVectorIndexDeltasSearched`, `LanceTaskWaitTime` -> `LanceScanTaskWaitTime`, `LanceIVFPartitionRankingTime` -> `LanceIVFPartitionSelectionTime`, `LanceScalarIndexSegmentPrepareTime` -> `LanceScalarIndexCandidateEvaluationTime`. - Master differs from 4.x in one line: the callback asserts with `DCHECK_LT` that a found metric has a registered counter, where 4.x checks it in the `if`; both come from the same array. - Not ported: apache#68813's FE change that makes `AggCounter` print nothing for `TUnit.NONE`. It belongs to the FE port; until then master's merged profile prints the group labels like the existing `RuntimeFilterInfo`. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `LanceTableReaderVectorSearchTest.SearchFilterDefaultsToPostfilter` (from apache#68813) and `LanceTableReaderFullTextSearchTest.SearchFilterHonorsPrefilterFlag` (top_k 1 with filter `row_id <= 2`: no row with postfilter, row 2 with prefilter; 4.x has no BE test for the full-text path). - Changed as in apache#68813: `AppliesSearchFilterBeforeTopK` becomes `AppliesSearchFilterBeforeTopKWhenPrefilterEnabled`; `MultiVectorScoresFiltersOffsetsAndIndexedCoverage` and `MultiVectorSegmentTopKMatchesIndependentGlobalOracle` set `prefilter` for their filtered cases, and the former checks the `RowIdPrefilter*` counters and that a flat split exposes `LanceIndexPartitionLoadTime`. - All pass on macOS arm64 (ASAN) against master's pinned lance-c, with the Lance suites. With vector search always prefiltering and full-text search never prefiltering, the two new cases fail. - Behavior changed: Yes, on the Lance path added by apache#69581. A search filter without `prefilter` now postfilters vector search, and full-text search honors `prefilter = true`, as on branch-4.1/4.2. Lance profile counters are grouped and renamed. Master has no FE that plans Lance scans yet. - Does this need documentation: No Co-authored-by: zhangstar333 <[email protected]> Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

### What problem does this PR solve? Issue Number: None Related PR: apache#65730 apache#66608 apache#66805 apache#66841 apache#66999 apache#67005 apache#67011 apache#67110 apache#67268 apache#67289 apache#67325 apache#67650 apache#67921 apache#68028 apache#68335 apache#68375 apache#68615 apache#68690 Problem Summary: Port the Lance BE read path of branch-4.1/4.2 (as of branch-4.2 cabcf45) to master. Master links liblance_c but has no Lance reader: file_scan_operator keeps FORMAT_LANCE off FileScannerV2 and V1 has no Lance path, so every FORMAT_LANCE scan fails today. This change is BE only. The FE connector plugin and the regression suites follow separately, so nothing on master plans a Lance scan yet and the BE unit tests carry the proof. - Reader: be/src/format_v2/lance/* and format_v2/table/lance_reader.* (plain scans, vector and full-text search, Substrait and runtime filter pushdown, scalar index segments, COUNT(*) from metadata, LIMIT, profiles), copied from branch-4.2 except one local renamed for -Wshadow. - Hooks: FileFormat::LANCE and its exhaustive switches, FileScannerV2 dispatch, and fetch_table_schema for FORMAT_LANCE. Lance queries use FileScannerV2 whatever enable_file_scanner_v2 says, as ADBC and fluss do: there is no V1 Lance reader. The second phase of TopN lazy materialization reads Lance rows through dataset take_rows. - Row locations: GlobalRowLoacationV2 gets a version 1 layout with a 64-bit Lance row id (the version 0 layout is unchanged), PRequestBlockDesc.row_id becomes uint64 (varint, wire compatible), and the materialization operator and RowIdStorageReader decode and validate by version. - Thrift: the external search request body, TLanceFileDesc 4-6 and TExternalSearchRequest 2-4 with the branch-4.1/4.2 ids. - Core: PTypeDesc keeps NULL_TYPE for a NULL literal type, and the nullable serde reads an Arrow Null array as NULLs. - Nine lance_* BE configs, a BE-wide Lance session with an optional Foyer data cache and 10 lance_session_* metrics; macOS links IOKit. - Tests: the Lance BE unit tests and their fixtures, including the docker fixture all_types.lance that one of them reads. Under master's -ffile-prefix-map the session test locates its fixture from the repository root, and the reader tests shrink the data cache to 64 MiB under the UT temp dir: the default 100 GiB disk tier is 51200 block files that stay open for the rest of the test process. Not included: the MaterializationOperator::push timer fix (apache#69579), the Lance index metadata scanner, the FE side, regression suites, and the 4.x changes after cabcf45 (apache#68707, apache#68813, apache#68815). ### Release note Add nine lance_* BE configs and lance_session_* BE metrics. They take effect once the FE can plan Lance scans. ### Check List (For Author) - Test: Unit Test - BE UT: the Lance reader, nested Null, runtime filter and session suites, plus the row-location protocol and FileScannerV2 cases - Behavior changed: Yes. PRequestBlockDesc.row_id is uint64, and the second phase of TopN lazy materialization rejects row locations of an unknown version or size. - Does this need documentation: No (documented with the FE connector) Co-authored-by: zhangstar333 <[email protected]> Co-authored-by: Gabriel <[email protected]> Co-authored-by: zhaobo wang <[email protected]> Co-authored-by: jay <[email protected]> Co-authored-by: FANNG <[email protected]> Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>