feat(index): apply shared FRI remapping to scalar and vector queries - #9107
Open
LuQQiu wants to merge 37 commits into
Open
feat(index): apply shared FRI remapping to scalar and vector queries#9107LuQQiu wants to merge 37 commits into
LuQQiu wants to merge 37 commits into
Conversation
LuQQiu
added this pull request to stack #9108
September 9, 2026 19:16
# Conflicts: # rust/lance/src/index/frag_reuse_reader.rs
The v0 read path keeps its original signatures and remapper type end to end; asynchronous batch translation lives in sibling entry points used only by tagged histories. The bridging RowIdRemapping enum is removed, and cache scoping is owned by one helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
Which rows a segment owns under a tagged history was decided twice at two times: reader planning computed coverage, and every index open re-ran load_indices plus a whole-dataset sibling scan to reconstruct the excluded fragments. Opening N segments repeated snapshot-wide planning N times. The tagged path now builds one snapshot-level plan in a single pass over the same load_indices output: per segment UUID, the rewritten query coverage and the fragments owned by other selected siblings, with identity and missing-coverage outcomes recorded so per-segment error semantics are unchanged. The plan is cached under the manifest-path (tagged) namespace via FriQueryPlanKey and built through get_or_insert so concurrent opens coalesce; each open then only looks its segment up. The v0/no-FRI path never touches the plan machinery. Also assert in debug builds when a plugin advertises batch row-ID remapping but falls into the default load_index_with_remapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
…maps Warm tagged opens still called load_indices before the plan lookup, so every open re-ran the tagged post-processing (capability checks and segment_coverage) even on a plan cache hit. The generation decision now reads the cheap cached stored listing, and load_indices moves inside the FriQueryPlanKey loader: a warm open touches only the stored listing, the cached mapping, and the plan lookup. Unsupported FRI versions keep the exact load_indices error. Plan construction is also de-quadratized: one pass over the filtered listing groups selected segments by logical index name and unions each group's direct coverage once; each segment derives its exclusions as the group union minus its own stored bitmap. Behavior is identical. The new batch-remapping bitmap load path no longer unwraps RowAddrTreeMap deserialization; malformed null-bitmap bytes now surface a corrupt-file error naming the lookup file. The legacy V0 path is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
LuQQiu
removed this pull request from stack #9108
September 10, 2026 03:15
LuQQiu
added this pull request to stack #9117
September 10, 2026 03:15
Resolve FTS document-state sharing and scalar cache rebind API conflicts. Keep batch remappers store-bound and reload on binding changes. Formatting passes; Clippy is blocked by main requiring object_store_opendal 0.60.1, which the registry does not currently resolve.
The FRI version picks the interface and the segment's need picks the behavior. ResolvedRemapping now has three states: V0 keeps the compact synchronous handle exactly as before tagged histories existed; V1Identity carries no remapper at all, so an untouched segment on a tagged dataset loads through every plugin's original entry point, including plugins that only implement the legacy API; V1Translate feeds the additive batch entry points. The hollow identity CompactFragReuseIndexHandle is gone; cache identity is carried by the resolver, and the cache-scoping rule is uniformly "tagged history" (V1Identity | V1Translate). Tests pin the resolver dispatch: no FRI resolves to nothing, v0 to the compact handle, an untouched segment to identity, a rewritten segment to translation, and an uncommitted UUID to the existing error. An FM-index segment (legacy-API-only plugin) stays available when untouched and serves queries, while its translation-needing sibling is excluded from coverage and queries fall back to scans with correct results. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
Generalize the legacy-reader task-local guard to every batch-remapping entry point this PR adds: the free helpers in lance-index-core's remapping module, each index type's load_with_remapping body, the vector storage batch branches, the QueryRowIdRemapper, and the tagged branch of open_row_id_remapping. The LEGACY_TRAFFIC_ONLY scope lives beside the helpers so every crate in the stack checks the same guard; the check has teeth in debug builds (every test profile) and compiles away in release. The v0 lifecycle matrix arms both guards end to end: datasets without an FRI and with a legacy v0 FRI run scans, filtered queries, appends, deletes, eager and deferred compaction, index optimization, historical checkouts, index remapping, and FRI cleanup, asserting the guard never trips and every result matches its pre-operation baseline. A teeth test proves the guard rejects both the resolver's tagged branch and the shared checker inside a legacy-only scope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
…to lu/fri-index-consumers
The parent merge changed InvertedPartition.tokens to Arc<TokenSet>; the FRI loading paths added by this branch now convert at construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
…lit into lu/fri-index-consumers
fri_query_plan no longer computes sibling exclusions itself: each filtered name-group's stored provenance goes through the reader's segment_plans, whose backtrack derives coverage and exclusions in one pass, and the plan keeps only the exclusions. The sibling-union code this replaces produced the same bitmaps, so behavior is unchanged. Also pin the persistence boundary: after an Append commit on a tagged dataset, every stored segment's fragment_bitmap is byte-identical to the previous manifest's, proving snapshot-derived coverage never leaks into stored provenance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
Every paired remapper field now documents its role: the legacy synchronous remapper serves index_version 0 and is mutually exclusive with the batch sibling, which serves tagged histories; both None means no translation is needed. Each dual-dispatch site labels its legacy synchronous and tagged asynchronous branches, and each construction site that sets either field debug-asserts the exclusivity invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016RynpAxtwGB9Q9CL4JCvR4
LuQQiu
removed this pull request from stack #9117
September 10, 2026 18:17
LuQQiu
added this pull request to stack #9137
September 10, 2026 18:17
This was referenced Sep 10, 2026
LuQQiu
marked this pull request as ready for review
September 10, 2026 18:21
…to lu/fri-index-consumers
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The synchronized base and head only move the tagged FRI capability flag to its non-conflicting bit; this PR retains the same stable patch ID as the reviewed revision. The shared batched remapping boundary still keeps v0 on the legacy path, scopes tagged caches by snapshot, and falls back to scans for unsupported or incomplete coverage; focused scalar, vector, lifecycle, and prewarm coverage remains successful.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack reordered: the standalone format proposal is now #9136 (replacing #9065), followed by #9106 → #9064 → #9067 → #9068 → #9107 in native stack #9137. This PR’s diff is byte-identical to its pre-reorder diff. All branches include main
31d78d170; no history was rewritten. Formatting checks pass; Clippy and tests remain blocked by the unavailableobject_store_opendal 0.60.1dependency.Connect scalar and vector index loading to the common FRI reader from #9068. Mapping algorithms and lineage traversal remain in earlier PRs; this PR adapts index consumers and enables derived coverage only for supported consumers.
Validation:
cargo test -p lance frag_reuse_reader --lib -- --test-threads=1.cargo fmt --alland workspacecargo clippy --all --tests --benches -- -D warningspass.No format documentation changes. Draft, last layer of the six-PR reader stack.
Latest main synchronization (2026-09-10): merged
d7b031f9cthrough the stack without rewriting history.cargo fmt --alland whitespace checks pass. Workspace Clippy could not run: main now requiresobject_store_opendal 0.60.1, but the crates.io index currently resolves only up to 0.60.0. Tests were not rerun after this synchronization because dependency resolution is blocked. Earlier validation above predates this merge.