[improvement](lance) Adopt upstream segment prefilter optimization

[Gabriel39]

Unfiltered ANN searches scoped to complete index segments can spend time materializing redundant row-ID allowlists. Update lance-c to an immutable upstream revision containing the merged Lance development-branch fix lance-format/lance#9599 and ANN diagnostics from lance-format/lance#9602, through lance-format/lance-c#89.

Keep all master changes in thirdparty/. Remove the superseded local lance-c patch chain and retain the Foyer cache integration while its interface remains outside upstream main. The upstream revision also carries the merged lance-format/lance#9537 PQ scoring change and ANN stage timing diagnostics; no Doris FE or BE code changes are included here.

The prefilter fast path requires complete visible coverage of every selected segment and no predicate. Partial coverage and predicates retain row-ID prefiltering, while deletions and unindexed fallback remain effective.

Pin lance-c 9bd730add2ac70316c1d642b8459011e2dd92022, using the merged Lance #9602 revision 68c12dfd7efe02f90ad2d7f3a239a7eb64884e57. This includes parallel-path partition-preparation timing. The upstream callback regression checks timer presence and nanosecond units while accepting valid zero durations.

The Foyer wrapper caches immutable ObjectMeta and Attributes independently of HTTP response extensions, avoiding repeated HEAD requests on warm range reads. It does not replay transport extensions from cached metadata. The HTTP regression checks HEAD/GET counts, returned bytes/ranges/metadata, fresh dataset scopes, and explicit HEAD bypass.

Generate the retained Foyer patch from source commit 24c7ca4bcb9422c113b0d3e07e4efe1173b0bc9f relative to the pinned lance-c baseline. The header records both revisions and the regeneration command. The source is submitted as zhangstar333/lance-c#3 against the branch behind lance-format/lance-c#73, on top of merged source-alignment PR #2. Doris consumes the exact reviewed source revision while that supplementary PR awaits merge.

Scope metadata, size, and block keys to a random namespace for each live underlying object-store instance. The wrapping API omits a complete stable backend identity; identical bucket/path names alone cannot distinguish S3-compatible endpoints. Weak identity records preserve sharing for the same live store without retaining it, while new stores and process restarts start cold. A fresh Dataset open that creates a new store consequently needs to warm its own cache. This deliberately reduces reuse across instances to prevent returning another origin's data; it does not claim unchanged cache-hit rates or production latency.

Avoid inserting an unchanged size entry into the WriteOnInsertion hybrid cache: look up both tiers first and populate only absent or invalid size records. Regression tests cover real HTTP endpoints sharing bucket/path/ETag, batched data and NotFound isolation, replaced origins after disk recovery, weak-reference lifetime, and actual disk-write bytes. Five warm range reads wrote 40 KB before the fix and zero bytes after it in the regression.

Fingerprint the patch in the downloader so existing source trees refresh after updates, including legacy empty markers. Identical patches reuse cached sources. Check platform definitions under nounset with simulated Darwin x86_64/arm64, and make the optional ADBC source guard safe when unset on master. Downloader lifecycle and platform handling remain downstream in Doris.

Validation: 461 regular Rust tests passed on the final source; Rust formatting and diff checks passed. GNU patch and git apply checks passed, and all 87 tracked source files match the recorded source commit. Downloader tests passed on master, branch-4.1, and the downstream hotfix branch for fresh extraction, idempotence, re-extraction, generic markers, legacy/mismatched Foyer markers, and patch failure. Shell syntax and simulated macOS initialization passed. All three opt-in native consumer tests passed: C calls, C++ calls, and static OSS HTTP transport. Full Doris builds and BE integration execution remain in PR CI.

[Gabriel39]

run buildall

[Gabriel39]

/review

[hello-stephen]

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

Please clearly describe your PR:

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

[github-actions]

Completed static review of PR #68613 at head aa0062af0dec97eebaa9b6623f035e3787565b2c after two convergence rounds. Two new findings are inline: a P2 global 4-bit PQ scoring cost and a P3 macOS test-harness failure. No existing inline comments or P0/P1 findings were present. The user supplied no additional focus points.

Critical checkpoints:

• Goal and proof: The new raw segment-coverage guard can omit redundant row-ID prefilter materialization for unfiltered ANN scans. Pinned Lance execution still applies fragment/deletion, overlay, external, and per-segment masks; the Rust patch adds coverage and execution cases. These are code/test artifacts, not independently executed results in this review.
• Scope and parallel paths: The six changed paths form one pinned Lance backport and its source integration. Ordinary and multivector ANN use the helper; indexed batch search delegates to single-query scans. Unknown coverage, excluded segment fragments, and expression filters keep the existing loader. FTS behavior is outside this change.
• Conditions and lifecycle: The coverage guard and comments express the required raw-bitmap condition. Archive checks, source-local patch markers, patch order, relative Cargo paths, and --locked were traced through fresh, cached, named-package, clean, and failure-retry flows. Failed patches do not mark the new source ready.
• Concurrency and ownership: This change adds no threads, shared mutable state, lock acquisition, cross-translation-unit initialization, or new process lifetime management.
• Configuration and compatibility: No runtime configuration, FE/BE variable, C ABI, index storage format, or rolling-upgrade protocol changes were found; Cargo resolves the same pinned Lance version from a local patched tree.
• Transactions and writes: No FE transaction, persistence, data-write, or crash-recovery path is changed. Error handling in the patch scripts stops on patch failure before writing their completion markers.
• Tests and observability: Added Rust cases cover segment selection, deletion, stable row IDs, filters, and PQ score boundaries; the shell harness covers extraction and marker paths. Existing prefilter_loads statistics make the intended optimization observable. The macOS harness startup defect is inline. The author reports Rust/Cargo validation; this review ran no builds or test suites, so runtime and end-to-end Doris behavior remain unverified here.
• Performance and remaining concerns: The float-only 4-bit scorer affects existing bulk searches as well as the new shortcut; upstream's isolated large-partition kernel result shows a material slowdown, discussed inline. No other substantiated new correctness, lifecycle, compatibility, or coverage defect remained after the final changed-file sweep.

The final changed-file and candidate sweep found no unresolved point. Both normal full-review passes and the separate risk-focused pass returned NO_NEW_VALUABLE_FINDINGS in round 2; the review is complete. This is a nonblocking comment review with P2/P3 findings and no independently confirmed existing P0/P1 inline comment.

[github-actions]

[P2] Preserve throughput for large 4-bit PQ partitions

This replaces the 16-lane u8 bulk loop with scalar f32 lookup for every row in every 4-bit PQ bulk search, including existing full-snapshot scans. Upstream Lance #9537 measured this float-only kernel at 472.215 us versus 98.553 us for 16,384 rows and 96 subvectors (an isolated kernel result, not Doris latency), then added exact u16 screening to recover throughput. Large IVF-PQ scans can therefore regress even when they do not use the new prefilter shortcut. Please carry an exact fast path for large partitions and benchmark representative Doris ANN scans.

[github-actions]

[P3] Keep the patch harness runnable on macOS

set -u is active when this sources vars.sh, but ARROW_ADBC_FLIGHTSQL_SOURCE is initialized only on Linux and is expanded unconditionally later in vars.sh. On Darwin, the documented test command exits with an unbound-variable error before any Lance case runs. Initialize the variable or source vars.sh with nounset disabled.

[Gabriel39]

Fixed in 3e9d8fd. The optional ADBC source guard now uses ${ARROW_ADBC_FLIGHTSQL_SOURCE:-}, so sourcing vars.sh under nounset is safe when that variable is absent. The harness checks simulated Darwin x86_64 and arm64 with the variable explicitly unset. This reproduced the unbound-variable failure before the fix; both platform checks and the full downloader lifecycle pass afterward. This is shell-level platform coverage, not a native macOS build.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Complete static review of PR #68613 at c96ebcde1080359a5af9a6d29ab197916f172972. I found no new valuable inline issue. All three full-review and three risk-focused reviewers converged in one round, and the final sweep covered all 12 changed paths.

• Goal and tests: The immutable lance-c/Lance pins contain the segment-aware ANN prefilter shortcut. The pinned C and Lance sources include coverage tests for complete and partial segment selection, predicates, deleted rows, stable row IDs, and unindexed fallback. The new downloader harness covers patch application states. This review was static; I did not run builds or tests, as instructed.
• Scope, parallel paths, and conditions: The removed FTS, scanner-control, scalar-segment, OSS transport, multivector, and prefilter patch functionality is present in the pinned upstream source. The shortcut requires an empty filter plan and known coverage for every selected segment; partial or unknown coverage retains the row-ID path. Full-snapshot, fragment-scoped, FTS, scalar, multivector, and fallback paths were checked. The patch replacement and retained Foyer integration are focused on the third-party dependency.
• Data correctness and error handling: The pinned search path still applies deletion, overlay, segment-ownership, and external masks. The Foyer rebase preserves origin read behavior for uncached or unusable entries. Downloader patch failure exits before writing its success marker. No new silent-error or visibility issue was substantiated.
• Concurrency, lifecycle, and memory: Foyer's bounded metadata cache is shared across dataset scopes; its wrapper registry uses weak references and does not hold its mutex over origin I/O. Dataset restoration creates a fresh statistics scope. Counter atomics carry statistics only. No new cross-translation-unit static dependency or Doris BE allocation path was introduced.
• Configuration and compatibility: The archive checksum and extracted directory match vars.sh, and the public C/C++ entry points supplied by the removed patches remain in the pinned header. No Doris FE/BE wire variable, dynamic process configuration, EditLog, transaction, MoW, or Doris-side storage-format code changed. Mixed-version runtime behavior was not exercised.
• Performance and observability: The shortcut removes redundant row-ID materialization for eligible segment scans; prefilter metrics and data-cache statistics remain available. The existing [P2] PQ throughput thread (4129869448) already raises its separate concern, so I did not duplicate it.
• Existing context and focus: The [P3] macOS harness nounset issue (4129869462) still applies and is already inline. There was no additional user-provided focus. Neither existing thread is P0/P1, and no new comment is proposed.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Review opinion

Complete static review of the 12 changed paths found no new actionable issue. The immutable lance-c archive checksum matches the pin, and its Lance dependency contains the complete-segment prefilter gate plus exact 4-bit PQ screening and rescoring. The retained Foyer patch and downloader migration have no substantiated new defect. No builds or tests were run in this review environment; author and upstream test claims are not independent runtime validation.

Critical checkpoints

• Goal and proof: The pinned Lance scanner gate skips row-ID prefilter construction only for an empty predicate plan and selected segments whose raw fragment coverage excludes no visible scan fragment. Deletion, overlay, and external masks remain in the execution path. Upstream segment tests and pinned lance-c C API tests exercise eligible and ineligible scans; runtime success in Doris was not verified here.
• Scope and reuse: Replacing eight local search patches with an immutable upstream pin reduces the local patch chain. The remaining Foyer patch is rebased to the new object-store API; no FE/BE source or protocol was changed.
• Concurrency and lifecycle: Foyer counters use atomics; wrapper identity tracking uses a mutex and weak references. The wrapper releases its lock before returning or dropping wrappers, and restore attaches a fresh dataset scope. No lock-order, cycle, or lifetime failure was established.
• Configuration and compatibility: The optional Foyer capacities are fixed at session creation and have input checks; there is no new Doris runtime configuration. Pinned C declarations cover the removed patch interfaces. No storage format, FE/BE variable, or rolling-upgrade protocol change appears in this diff.
• Parallel paths and conditions: Single and batch ANN searches call the same prefilter selector. Partial or unknown segment coverage and actual predicates retain the loader; deletions and unindexed fallback retain their masks and scan paths. The condition is documented alongside the raw-bitmap check.
• Tests and results: The added downloader harness checks fresh, repeated, re-extracted, cached-marker, and rejected-patch paths. Pinned tests cover segment scopes, physical and stable row IDs, predicates, deletions, fallback, and PQ scores. No regression .out files changed. The already reported macOS nounset failure in the harness remains; this review does not duplicate it.
• Observability and performance: Prefilter load metrics and Foyer byte counters cover the new paths. The pinned PQ implementation uses u16 candidate screening followed by exact float rescoring, superseding the earlier float-only throughput concern. No Doris end-to-end latency measurement was independently verified.
• Transactions, writes, and other risks: This diff adds no Doris transaction, persistence, or data-write path. The Foyer wrapper forwards writes and limits cached reads to immutable direct data files. The final changed-file and candidate sweep found no other substantiated new issue.

Existing inline P2 comment 4129869448 concerns a removed float-only patch and no longer applies to this head. Existing P3 comment 4129869462 about macOS test portability still applies and remains the sole open issue identified here; it is already reported. One complete review round ended with both normal reviewers and the separate risk reviewer returning NO_NEW_VALUABLE_FINDINGS.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Complete static review of PR #68613 at head d78ec5b4483c142a46db949fefb235e6e2e21a7b after two convergence rounds. One new P2 correctness issue is inline: Foyer's new metadata hit can return a data-file response from the wrong S3-compatible endpoint in a shared session. No existing P0/P1 inline finding still applies.

Critical checkpoints:

• Goal and proof: The immutable lance-c pin and its locked Lance revision contain the complete-segment ANN prefilter shortcut. Pinned C API and Lance tests cover complete/partial coverage, predicates, deletions, stable row IDs, and unindexed fallback. This is static source evidence; this review ran no build or test.
• Scope and reuse: The change replaces eight local Lance search/FTS/scalar/vector/OSS patches with the pinned upstream source and retains one rebased Foyer patch. The archive checksum, removed C/C++ API surface, static OSS transport setup, and locked dependency revision were checked. No Doris FE/BE source API caller was found; BE links lance-c.
• Data correctness and conditions: The upstream shortcut keeps the row-ID prefilter for partial/unknown coverage and predicates; deletion and fallback paths remain. Foyer bypasses its cache for conditional, versioned, and extension-bearing gets. The inline P2 identifies a new wrong-success path for unconditional data-file get_opts across endpoint options; the older block-key collision is not claimed as a new defect.
• Concurrency and lifecycle: Foyer uses a bounded shared metadata cache, atomic statistics, and short wrapper-registry mutex sections with weak references. Session open and restore attach dataset scopes; no additional race, lock-order, or lifetime issue was substantiated.
• Configuration and compatibility: Foyer capacities are fixed per session with size checks; this diff adds no dynamic Doris runtime config, FE/BE variable, wire protocol, EditLog, or storage-format change. The pinned headers retain the removed patch interfaces. Mixed-version runtime behavior was not executed here.
• Parallel paths, tests, and observability: Single/batched segment search, FTS, scalar, multivector, full-snapshot, and fallback paths were reviewed. The downloader harness covers fresh, repeated, re-extracted, old-marker, and patch-rejection states. Upstream prefilter metrics and Foyer cache statistics remain available. A two-endpoint shared-session regression is missing for the inline issue. Author/upstream test claims were not independently rerun.
• Performance, writes, and errors: The pinned Lance revision includes the merged 4-bit PQ screening and rescoring fix, superseding the older float-only throughput concern; no Doris end-to-end latency claim was verified. Foyer forwards writes, while the downloader exits before marking a failed patch. No Doris transaction, persistence, or crash-recovery path changed.

Existing inline P2 comment 4129869448 concerns the removed float-only PQ patch and no longer applies to this head. Existing P3 comment 4129869462 about the macOS nounset harness still applies and is already reported, so it is not duplicated. The user supplied no additional focus points. The final sweep matched all 12 changed paths to the authoritative diff and left no unresolved candidate. Both normal reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS in round 2. Review complete; static validation only.

[github-actions]

[P2] Keep cached metadata scoped to the physical object store

A shared session can open s3://bucket/table.lance with two S3-compatible endpoint options. Lance creates distinct underlying stores, but passes s3$bucket as the prefix for both cache keys. For a normal data file whose size is in the manifest, the scan reaches this metadata hit without an earlier HEAD: after endpoint A warms data/f.lance, endpoint B can return A's size/ETag and cached bytes even if B lacks the object. Previously cached_get issued B's HEAD and failed. Include the effective backend identity in metadata and block keys, or keep an origin check when stores cannot be distinguished; cover two endpoints sharing a bucket/path.

[Gabriel39]

Fixed in 3e9d8fd, regenerated from lance-c 24c7ca4bcb9422c113b0d3e07e4efe1173b0bc9f. Metadata, size, and block keys now include a random namespace for the exact live underlying store; the weak registry cannot keep stores alive or reuse a persisted pointer identity. The wrapper interface does not expose a complete stable endpoint identity, so new store instances and process restarts deliberately start cold, including fresh Dataset opens that create a new store. This tradeoff is documented. Regressions cover real HTTP endpoints sharing bucket/path/ETag, batched reads, NotFound, replacement after disk recovery, and retained same-store reuse. All 461 regular Rust tests passed. Upstream source update: zhangstar333/lance-c#3.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Static review complete for PR #68613 at head 97c5034eb2ed4285a848f8e68e88284a0c6cca3e (base f88735f8f62ce3375c4a6874419495622c070ca7). I found no new, non-duplicate issue to raise inline.

Goal and evidence: The change replaces eight local Lance patches with a pinned upstream lance-c archive, retains the Foyer patch, and targets redundant row-ID prefilters for complete ANN segment scopes. The pinned Lance path checks the original segment coverage and retains predicate, deletion, and unindexed fallback handling. Its regression cases cover complete and partial scopes, stable row IDs, deletions, and filters. The new shell harness covers extraction and patch-marker states. These are static test-presence checks; I did not run builds or tests.

Scope and compatibility: I covered all 12 changed paths and compared the removed patch chain with the pinned source. The reviewed C/C++ API surface, FTS, scalar segment, multivector, static OSS transport, and Foyer integration remain present. The archive digest and source root match vars.sh, the rebased Foyer patch contexts match the archive, and the locked build uses the pinned Lance revision. The change is focused on the dependency pin, retained patch, and its harness.

Concurrency and lifecycle: Foyer's cache is shared by session while dataset statistics have separate scopes. Wrapper registration uses a mutex and weak references; cache reads and origin I/O occur outside that lock. Restored datasets attach a fresh scope. I found no additional lock-order, deadlock, or release issue in the changed paths.

Conditions and parallel paths: The ANN shortcut requires known complete selected-segment coverage and no predicate; partial or unknown coverage still loads row-ID prefilters. The Foyer wrapper keeps conditional and versioned reads on the origin path and handles both get_opts and batched range reads. Its paginated-list hook preserves the original path-preserving lister. No new dynamic configuration or FE/BE variable propagation is involved.

Performance and observability: The pinned Lance core includes exact quantized-u16 screening with float rescoring for the common large, unfiltered 4-bit PQ top-k path, so the earlier scalar-only throughput premise no longer describes this pin. ANN stage metrics and Foyer cache statistics remain available. No Doris latency result was independently measured.

Persistence, writes, and failures: This PR does not change Doris transactions, EditLog, visible versions, storage formats, or FE/BE write paths. The Foyer object-store wrapper delegates writes and caches immutable data-file reads. Failure paths and test results were inspected statically; runtime behavior was not validated here. git diff --check on the exact PR range passed.

Existing review threads: The physical-store cache-key collision at lance-c-foyer.patch:948 still applies and is already covered by comment 4131385630 ([P2]); the macOS harness set -u failure at lance-prefilter-patch-test.sh:26 still applies and is already covered by 4129869462 ([P3]). I did not duplicate either. The older 4-bit PQ throughput comment 4129869448 predates the pinned u16 fast path and does not warrant a new inline comment. No existing P0/P1 inline finding was independently confirmed on this head.

User focus: The supplied focus file contains no additional focus points. The complete PR was reviewed.

[Gabriel39]

run buildall

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Static review complete for PR #68613 at head b3a81d98c847586d6320d92aadf293b98979f0b1 after two full-review rounds, a separate risk-focused scan in each round, candidate-by-candidate verification, and a final sweep of all 12 changed paths. No new valuable inline finding survived verification, so this review adds no inline comments. The user supplied no additional focus points.

Existing review context remains relevant: comment 4131385630 already reports the P2 Foyer metadata/block-key collision across physical stores sharing a bucket/path, and comment 4129869462 already reports the P3 macOS harness set -u failure. I independently confirmed both on this head and did not duplicate them. The older P2 float-only PQ comment 4129869448 targets a removed patch; the new pinned Lance source has bounded u16 screening with exact float scoring for eligible 4-bit PQ candidates. No existing P0/P1 inline finding applies to this head.

Critical checkpoint conclusions

• Goal and proof: The third-party pin moves Lance-C to the stated immutable upstream revision, retains the Foyer integration, and exposes the upstream complete-segment ANN prefilter path. The pinned source contains C API and ANN/FTS regression cases; this static review did not execute them or prove Doris runtime latency or results.
• Scope and clarity: Changes are confined to thirdparty/: one archive pin, removal of superseded patches, the rebased Foyer patch, and an extraction/patch harness. The Foyer patch applies with zero fuzz in a dry run against the exact archive; its checksum and source directory match vars.sh.
• Concurrency and locks: Foyer shares cache state across dataset scopes; a mutex protects the weak wrapper registry and relaxed atomics count per-scope bytes. The inspected registry operations are small, and no new lock-order or lifecycle race was substantiated. The already-reported cross-origin key collision remains a correctness concern.
• Lifecycle and initialization: The session owns the cache factory, dataset handles retain their wrapper scopes, restore attaches a fresh scope, and weak wrapper records are removed on drop. Pinned Lance applies wrappers to primary and later base stores and handles paginated listing separately. No cross-translation-unit static initialization dependency was introduced.
• Configuration: The archive revision is static, and Foyer capacities are supplied when a session is created. No new mutable Doris configuration or restart-sensitive setting was added.
• Compatibility: The removed patch chain's exported C API calls remain in the pinned header and Rust source; the manifest and lock select the same Lance revision, and the rebased Foyer dependency entries resolve. No Doris FE/BE protocol or on-disk format definition changed in this PR. Mixed-version/runtime behavior was not exercised here.
• Parallel paths and conditions: Full-snapshot prefilter skipping requires an empty predicate and complete unique fragment-ID coverage; the new segment shortcut also requires known coverage disjoint from excluded fragments. Deletion, missing-fragment, overlay, external-mask, and unindexed-fallback paths remain in the pinned source. Foyer bypasses caching for conditional, versioned, HEAD, and extension-bearing requests; paginated listing continues through its original lister.
• Tests and expected results: The new harness checks fresh patching, repeat use, re-extraction, an existing generic marker, and patch rejection. Upstream source includes positive and negative C API, ANN, FTS, PQ, and timing cases. No expected-result file was changed. No build, test, or Doris cluster was run by this reviewer, as required by the review-only prompt; author/CI claims were not treated as independent validation.
• Observability: Upstream ANN stage timings, row-ID loader metrics, FTS metrics, and the retained Foyer statistics API are present. No distinct missing-observability issue was substantiated.
• Transactions, persistence, and writes: No Doris transaction, EditLog, table-write, or FE/BE variable-passing path changed. The Foyer disk tier is a read cache for immutable Lance data files; its physical-store identity issue is covered by the existing P2 thread.
• Performance and other issues: The intended prefilter shortcut avoids redundant row-ID allowlists in eligible scans, and the new PQ path contains exact u16 screening. Three additional Foyer performance/memory candidates were checked against the merge base and dismissed because the same code was already present. No unresolved candidate remains.

Review status: complete static review; no new inline comments; existing_blocking_comment_ids is empty because the applicable existing inline findings are P2/P3.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Review complete at head 35df7a7588767d39ccd7eb2839e3307be6cd5da3. No new inline findings after the main risk scan, complete changed-file sweep, and one converged round of two full reviews plus a separate cache-risk review.

Goal and evidence. This PR replaces the local lance-c search patch chain with pinned upstream 9bd730a while retaining the Foyer integration. The pinned archive checksum and source directory match vars.sh; the Foyer patch applies with strict zero-fuzz dry-run. The deleted stack's 46 exported C API names remain in the pinned header, and the upstream tree contains the named scalar, FTS, multivector, OSS, and segment-prefilter regressions. The pinned Lance dependency includes the merged segment-prefilter optimization (#9599), ANN timing work (#9602), and 4-bit PQ scoring fix (#9537). These are static/source and ancestry checks. No build, test, Doris runtime execution, or production benchmark was run in this review, as the review instructions prohibit builds; the author's test claims were not treated as independent validation.

Critical checkpoints. Scope is confined to third-party pinning, patch removal/rebase, and a patch harness; the changes are focused for that goal. The cache's concurrent counters use relaxed atomics for cumulative statistics, and its weak wrapper registry has a short mutex scope; no new lock-order or lifecycle defect was substantiated. Dataset open and restore attach wrapper scopes and separate counters, session close leaves dataset-owned state alive, and paginated listing passes through under Lance's wrapper contract. The new cache configuration is fixed per session; no Doris dynamic configuration, FE/BE variable, transaction, EditLog, persistence, or data-write protocol changes are introduced. The public C symbols from the removed patches remain present; binary and storage-format behavior were not runtime-validated. Conditional, versioned, extension-bearing, and invalid-range reads use the origin path, while immutable data-file range and whole-object reads use Foyer. Relevant Rust and C/C++ tests are present, including negative options and restore/cache cases; the new harness checks patch application but does not execute search semantics, and no result files were changed. Data-cache statistics and upstream ANN timers cover the new observability paths. The Foyer metadata tier is bounded by its configured memory capacity; no new Doris BE allocator or MemTracker call path appears in this PR. The intended prefilter and metadata-read optimizations are present in source, but Doris latency and cold-cache throughput remain unmeasured here.

Existing review context. The cross-endpoint Foyer key collision remains covered by inline P2 comment 4131385630; the macOS harness set -u failure remains covered by P3 comment 4129869462. The former 4-bit PQ P2 thread 4129869448 was on a removed patch; the pinned Lance revision descends from the merged #9537 fix. No P0/P1 inline finding remains to list, and no duplicate comment is submitted. No additional user review focus was supplied.

[hello-stephen]

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	64.80% (30532/47114)
Line Coverage	49.53% (319916/645877)
Region Coverage	44.98% (257861/573309)
Branch Coverage	46.67% (120826/258916)

[hello-stephen]

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	76.58% (34919/45599)
Line Coverage	61.69% (393891/638471)
Region Coverage	58.02% (331273/570957)
Branch Coverage	58.98% (152236/258104)

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Review complete for head 8d2608f08845460552cce1760d4faf4d062c2c78 after two convergence rounds, a separate risk-focused pass, and a final sweep of all 12 changed paths. This is a static review; I did not build Doris or run tests.

Opinion: One new P2 performance finding is inline. The existing P2 physical-store cache-key issue remains applicable (thread); the existing P3 macOS test-harness issue also remains applicable (thread). Neither is repeated. The older 4-bit PQ throughput concern (thread) is resolved in the new pin: large 4-bit L2/Dot searches use bounded u16 screening followed by exact f32 rescoring, with Cosine mapped to L2 (source). No existing P0/P1 inline finding still applies.

Required checkpoints

• Goal and proof: The pinned lance-c revision contains the complete-segment ANN prefilter selection and targeted tests. The pinned Lance implementation checks selected segments' raw fragment coverage against excluded visible fragments; predicate, partial-coverage, deletion, overlay, and unindexed fallback paths remain. This is code and test-presence evidence, not an independently run result.
• Scope and reuse: Changes stay in thirdparty/: the immutable upstream pin replaces the local FTS, scanner, scalar-segment, OSS, multivector, and prefilter patch chain, while one rebased Foyer patch remains. The jump in transitive Lance revision is broad, so I checked the old C API surfaces and their tests in the pin.
• Concurrency and lifecycle: Shared Foyer cache state uses Arc, a short Mutex section for weak wrapper identity records, and relaxed atomics only for counters. Dataset open and restore attach separate statistic scopes; weak records are removed when wrappers drop. No heavy cache I/O occurs while that registry lock is held, and no new cross-translation-unit static initializer dependency was found.
• Configuration and compatibility: No Doris dynamic configuration or FE/BE variable is added. Cache capacities are fixed at session creation. Removed patch API symbols remain in pinned lance-c; no Doris protocol or storage-format declaration changes here. Mixed-version file behavior was not exercised in this static review.
• Parallel paths and conditions: Ordinary ANN and multivector scans use the selected-segment prefilter check; FTS and scalar segment scans retain their scoped coverage and fallback logic. Conditional, versioned, and HEAD object reads bypass Foyer caching. The existing cross-endpoint cache-key defect is already threaded.
• Tests and results: Pinned upstream tests cover full/partial segment scope, filters, deletions, unindexed tails, metrics, Foyer range/EOF/recovery/statistics, and C/C++ APIs. The new downloader harness covers fresh extraction, reuse, re-extraction, stale markers, and patch failure, though it is not wired into CI and has the already-threaded macOS nounset failure. No local build, test, or generated result was used as proof.
• Observability, error handling, and memory: ANN prefilter and stage timing metrics and dataset cache statistics are exposed. Downloader patch failure does not write a success marker; Foyer read failures fall back to the origin where supported. Cache memory/disk capacities are explicit; no Doris MemTracker path changed. The new inline finding concerns avoidable disk-cache work on warm reads.
• Transactions, persistence, and writes: No Doris transaction, EditLog, visible-version, delete-bitmap, or FE/BE data-write path changes. Foyer persists only optional local cache entries; Lance data-file immutability remains the read-cache assumption.

There was no additional user-provided review focus. All main risk items and subagent candidates have a resolved disposition; the three round-two reviewers returned NO_NEW_VALUABLE_FINDINGS on the final comment set.

[github-actions]

[P2] Avoid writing the size entry on warm range reads. When this new metadata-cache hit avoids the origin HEAD, cached_get still reaches the unconditional HybridCache::insert(size_key, ...) below. The cache uses WriteOnInsertion; Foyer enqueues that same key for disk storage on every get_opts call, and its block writer allocates a 4096-byte aligned entry. Lance single-range reads use get_opts, so repeated scans of fully cached data can keep writing size metadata and churn the disk tier. Populate the size key only on a true size miss (or keep it memory-only), and verify a warm range read causes no further storage enqueue.

[Gabriel39]

Fixed in 3e9d8fd (lance-c source 24c7ca4). cached_get now checks the size entry in both hybrid tiers and inserts only when absent or invalid. The regression reopens the disk cache with a cold metadata cache, performs five warm range reads, drains the actual disk writer, and checks disk_write_bytes. It failed before the fix with 40960 bytes and passes after the fix with zero. All 461 regular Rust tests passed. The source change is included in zhangstar333/lance-c#3.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Complete static review of PR #68613 at 2056cc2e77ab9027b87887a95c2af186cdb2a7a0 (base f88735f8f62ce3375c4a6874419495622c070ca7). I found no additional valuable inline issue. The existing Foyer cache-key and repeated size-write P2 threads (4131385630, 4141000574) and the macOS harness P3 thread (4129869462) still cover their respective concerns; I have not duplicated them. The older PQ4 comment (4129869448) targets a superseded patch; the pinned Lance revision contains the bounded u16 screening path. No existing P0/P1 inline finding remains applicable to this head.

Critical checkpoints

• Goal and proof: The PR replaces the local Lance search patch chain with pinned lance-c 9bd730a and Lance 68c12df, retaining a Foyer patch. Static source checks confirm the public C/C++ entry points from the removed patches and the complete-segment ANN gate. Upstream tests and the new downloader harness cover major cases, but this review did not execute them; runtime success remains unverified here.
• Scope and clarity: All 12 changed paths are in thirdparty/. The source pin, Foyer patch, downloader fingerprint, and harness form a focused dependency update.
• Concurrency and lifecycle: Downloading and patching are sequential. Foyer shares a hybrid cache across dataset scopes, uses atomic counters and a mutex-protected weak wrapper registry, and attaches fresh counters on restore. I found no distinct lock-order, static-initialization, ownership-cycle, or teardown issue. The patch marker handles fresh and mismatched cached trees; previously installed artifacts follow the repository's existing separate refresh convention.
• Configuration and compatibility: No Doris dynamic configuration, FE/BE variable, wire-format, or storage-format change is in the diff. The new dependency pin preserves sampled C/C++ ABI declarations and the old patch-chain API set. No rolling-upgrade incompatibility was established from the static evidence.
• Parallel paths and conditions: The ANN shortcut checks raw segment coverage and absence of predicates; partial/unknown coverage retains prefiltering. Batch ANN, unindexed fallback, deletion and segment-ownership masks, FTS/scalar/multivector interfaces, and cache get_opts/get_ranges paths were reviewed. Conditional, versioned, and extension-bearing gets bypass the Foyer cache. The two known Foyer cache concerns remain in existing threads.
• Tests and observability: The pinned sources include C API and Lance regressions for segment scopes, deletions, stable row IDs, metrics, and timing; the local harness covers downloader reuse and invalidation. No .out result changes appear. ANN timings and per-dataset cache counters provide observability. Author and CI test claims are not independent validation in this static-only review.
• Persistence, writes, and performance: No Doris transaction, EditLog, or data-write path changes. Foyer forwards object-store writes and only caches direct immutable data-file reads. The intended global row-ID prefilter work is skipped for eligible ANN scans; per-segment ownership masks remain necessary. The existing warm size-write comment covers the material additional cache-performance concern.

User focus: no additional focus points were supplied. This review is complete at the stated head, with no new inline comments.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Complete static review of PR #68613 at head 3e9d8fd2163a306a5c96f34a8faf1c089e28872d. I found two new issues: a P2 retry failure in the new Foyer patch refresh and a P3 gap in the new lifecycle harness's CI coverage. Both are attached inline. No existing P0/P1 inline finding remains applicable, so existing_blocking_comment_ids is empty.

Goal, scope, and tests. The PR pins an upstream lance-c revision containing the ANN segment-prefilter work and retains a source-generated Foyer patch. I checked the archive checksum, patch preimages, removed-patch API/test parity, and the locked Lance revision. The change is confined to thirdparty and is focused on replacing the old patch chain. Rust, C/C++, HTTP cache, and downloader regressions are present in the change; the new downloader lifecycle harness is not invoked by the PR gate (P3 inline). This review was static only: I ran no build or test, and author or CI test claims were not treated as independent runtime validation.

Concurrency and lifecycle. The origin namespace and wrapper registries use short mutex-protected operations and weak references; per-dataset counters use relaxed atomics for statistics. Session closure, restored dataset scopes, wrapper drop, and disk-cache reopening have corresponding code paths and regressions. I found no distinct lock-order, reference-cycle, or cross-origin reuse issue on this head.

Configuration and compatibility. The optional data-cache C API validates directory and capacities when creating a session. No Doris dynamic configuration, FE/BE transmitted variable, protocol field, storage format, or EditLog path changes. The removed local C APIs remain available in the pinned upstream source; no mixed-version protocol change was identified.

Parallel paths, errors, and writes. I traced whole-object, single-range, and batched reads, plus HEAD/conditional bypass, direct data-file selection, origin identity, EOF and NotFound behavior. The new fingerprint refresh has an incomplete-extraction retry path (P2 inline). Doris transaction and data-write semantics are untouched; the Foyer disk tier is a cache. No additional status/error propagation or parallel-path defect was substantiated.

Performance and observability. The pin contains the exact 4-bit PQ screening path behind the earlier throughput concern. The current Foyer patch checks both tiers before writing an unchanged size record, and its origin-scoped keys prevent cross-endpoint cache hits. Dataset cache statistics and upstream ANN timing diagnostics cover the new paths; no further observability issue was found.

Existing review context and focus. The four earlier P2/P3 threads are covered by this head: PQ scoring is in the pinned Lance source, Foyer keys include live-origin namespaces, unchanged size entries are not reinserted on warm reads, and the macOS nounset guard is present. I did not duplicate them. The focus file listed no additional user-specific points.

Three review rounds completed. The final changed-file and candidate sweep found no unresolved suspicious point beyond the two inline findings.

[hello-stephen]

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	64.91% (30689/47276)
Line Coverage	49.66% (321775/647922)
Region Coverage	45.19% (260348/576057)
Branch Coverage	46.84% (122037/260538)

[hello-stephen]

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	76.68% (35085/45754)
Line Coverage	61.82% (395999/640537)
Region Coverage	58.21% (333978/573735)
Branch Coverage	59.15% (153642/259745)