feat: mask data overlay files in scalar and vector index queries#7411
Closed
wjones127 wants to merge 21 commits into
Closed
feat: mask data overlay files in scalar and vector index queries#7411wjones127 wants to merge 21 commits into
wjones127 wants to merge 21 commits into
Conversation
Add a specification for data overlay files: small files attached to a fragment that supply new values for a subset of (row offset, field) cells without rewriting the base data files, for cheap cell-level updates. - protos/table.proto: rework DataOverlayFile with a dense/sparse coverage oneof (shared_offset_bitmap vs new FieldCoverage), rename read_version to committed_version (effective, commit-stamped), and document rank-based addressing with no offset column. Document reader feature flag 64. - docs: add data_overlay_file.md (full spec, worked example, guidance stub) and link it from the table format overview. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the `DataOverlay` operation (and `DataOverlayGroup`) to attach overlay files to fragments without rewriting their base data. Mirrors the `DataReplacement` batch shape, appends to each fragment's `overlays` list, and documents permissive conflict semantics: concurrent overlays, appends, deletes, and column rewrites are compatible; row-rewrites, compaction, and overlay->base folds conflict. committed_version is left 0 by the writer and stamped at commit time. Proto only — Rust/Python bindings deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The table/transaction proto changes generate new fields and an Operation variant. This wires the minimum needed to compile without implementing overlay support: - Emit empty `overlays` when converting fragments to proto. - Reject the `DataOverlay` transaction operation with NotSupported on read. Datasets that use overlays set reader feature flag 64, which already falls in the unknown-flag range rejected by `can_read_dataset`, so the library refuses them at the feature-flag layer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
The v2 file writer advanced every column from a single global row counter, so a single file could only hold columns of equal length. Sparse data overlay files need columns whose item counts differ within one file (each field covers a different set of rows). Add `FileWriter::write_columns`, which writes a set of `(field, array)` pairs and advances each field's row counter independently, leaving other fields untouched. A field never written ends up as a zero-length column. `write_batch` is unchanged: it still advances all fields together, so ordinary rectangular files round-trip exactly as before. Per-column lengths were already derivable from page metadata; expose them via `FileReader::column_num_rows`. The reader already schedules each column from its own pages, so reading a column at its own length and random access within it work without further changes. Part of the Data Overlay Files feature (OSS-1323). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review on the unequal-length-columns work: - Rename `FileWriter::write_columns(Vec<(usize, ArrayRef)>)` to the simpler `write_column(column_index, array)`; callers loop instead of batching. - Drop doc comments from internal-only helpers. - Reader: validate that a projection's top-level columns share a length and reject mismatches up front with a descriptive error, instead of panicking in the decoder. `RangeFull`/`RangeFrom` now resolve to the projected common length rather than the file's longest column, so a single short column reads back at its own length. Rectangular files are unchanged. Adds `test_read_unequal_length_projection` (V2_0 + V2_1).
Adds the in-memory + commit machinery for data overlay files (per the spec in lance-format#7381), the foundation the scanner/take/index/compaction work builds on. - `DataOverlayFile` / `OverlayCoverage` (dense `shared_offset_bitmap` and sparse per-field) with protobuf round-trip, attached to `Fragment.overlays`. - Reader feature flag 64 (`FLAG_DATA_OVERLAY_FILES`): set whenever any fragment carries overlays, so a reader that does not understand them refuses the dataset instead of returning stale base values. - `Operation::DataOverlay` transaction op: appends overlays to a fragment's list (preserving concurrently-written overlays) and stamps each overlay's `committed_version` to the new dataset version at commit time (re-stamped on retry). Conflict rules mirror DataReplacement — permissive against appends, deletes, column rewrites, index builds, and other overlays; conflicts only with row-rewriting compaction of the same fragment. Scan-side merge, take, and end-to-end write+read tests follow in the same PR branch. Part of the Data Overlay Files feature (OSS-1322). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that overlays can be committed, a scan or take over a fragment that has overlays would silently return stale base values, since the read-path merge is not implemented yet. Refuse such reads at `FileFragment::open` with a clear error instead of serving incorrect data. Lifted once the scan/take merge lands (rest of OSS-1322 / OSS-1324). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…flag OverlayCoverage now holds parsed Arc<RoaringBitmap>s, deserialized once when a fragment is loaded and shared cheaply on clone, instead of re-deserializing the serialized bytes on every coverage_for_field call. Treat the data overlay feature flag (64) as unknown in release builds unless LANCE_ENABLE_DATA_OVERLAY_FILES is set, so the unreleased feature is neither emitted by release writers nor accepted by release readers; debug builds understand it so tests exercise the path. Stable-sort a fragment's overlays by committed_version (newest last) on load so resolution can assume the ordering. Adds tests for the missing-coverage/data_file errors, the release gating, and the load-time sort. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three correctness fixes in the data-overlay write/commit path, each with tests (this path had ~0% coverage): - build_manifest dropped overlays when two DataOverlayGroups targeted the same fragment (HashMap collect kept only the last). Merge groups by fragment_id so all overlays are appended in order. - check_data_overlay_txn did not conflict when a concurrent Update/Delete removed an overlaid fragment, leaving the overlay orphaned and hard-erroring on retry. Mirror check_data_replacement_txn: retryable conflict when removed/deleted_fragment_ids intersect our fragments; fragments merely updated in place stay compatible. - impl PartialEq for Operation had the DataOverlay arm backwards: it reported DataOverlay == Rewrite as true and DataOverlay == DataOverlay as false (conflict semantics copied into the equality impl). Compare groups for the same variant; not equal to other variants. Tests: build_manifest append/stamp, duplicate-group merge, unknown-fragment error; the conflict matrix incl. remove-fragment conflict; Operation equality; OverlayCoverage serde JSON round-trip; coverage_for_field out-of-bounds; apply_feature_flags overlay arm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire overlay resolution into FragmentReader so `take` and scan reads merge data overlay cells over the base data, removing the temporary guard that refused reads of fragments carrying overlays. Reads are lazy and rank-pushed: each overlay file is opened once (no value bytes read up front), requested offsets are routed to the newest covering overlay at their coverage rank, and only the touched ranks are fetched via the existing `take` primitive. Base and overlay IO run concurrently, then the column is assembled with `interleave`. This makes overlay reads O(requested rows) rather than O(coverage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds tests for combinations the initial read-path suite missed: row-id pass-through alongside an overlay, an older overlay routed zero ranks (empty-ranks fetch branch), a newest NULL value shadowing an older non-null, per-field newest-wins across two sparse overlays, a plan present but all requested offsets uncovered (all-fall-through), and a dataset-level take spanning multiple overlaid fragments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`route_overlays` was offset-major: for every physical offset it probed
each coverage bitmap newest-first. On a full scan that is
`O(N_rows * K_overlays)` roaring `contains` probes — ~16M for a 1M-row,
16-overlay scan — of which 99% only confirm a fall-through to base. It
dominated scan CPU (~87% of the scan thread) and made scan cost grow
linearly with overlay count, badly so for scattered (stride) coverage.
A scan reads a contiguous physical range, so offset `o` is output row
`o - base`. The fast path now intersects each coverage with the batch's
offset range (a container-level op that drops a non-overlapping batch in
`O(containers)`) and routes only the in-range bits directly to rows.
Those bits carry consecutive coverage ranks starting at the count of bits
below `base`, so ranks need a single `rank` lookup rather than a probe per
cell. `take` still supplies arbitrary offsets and keeps the general path.
Microbenchmark of `route_overlays` over a 1M-row scan (ms/scan):
overlays stride old -> new contiguous old -> new
1 13.3 -> 0.67 2.8 -> 0.55
4 46.3 -> 0.86 3.5 -> 0.66
16 178.5 -> 1.48 9.4 -> 0.96
64 829.0 -> 4.07 33.3 -> 2.09
A fuzz test asserts the fast path produces byte-identical routing to the
general path across random bases, lengths, overlay counts, and densities.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A scalar or vector index built before an overlay does not reflect the overlay's values, so its entries for overlay-covered cells may be stale. Queries must stay correct while overlays remain. For each index a query relies on, compute the set of fragments carrying an overlay that was committed after the index (`committed_version > index.dataset_version`) and that touches a field the index covers. The check is field-aware (an overlay on an unrelated field excludes nothing) and version-gated (an overlay already incorporated by the index is ignored), via the new `overlay_exclusion_offsets` helper. Such fragments are dropped from the index's covered set so they fall to the flat path, which re-evaluates them against their current (overlay-merged) values: - Scalar (`FilteredReadExec`): stale fragments are removed from the `EvaluatedIndex` covered set, so the full filter is re-applied to them. - Vector: stale fragments are removed from the index segments' coverage bitmaps (so the ANN prefilter blocks their stale rows) and added to the flat-KNN fallback so their current vectors are re-scored into the top-k. This drops stale index hits and surfaces new matches the index never saw. Granularity is per fragment; row-level exclusion is a future optimization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two optimizations to reduce per-query overhead of the overlay-stale-index check introduced in OSS-1325: 1. `mask_overlay_stale_segments` now returns `Option<Vec<IndexMetadata>>` instead of always cloning the segment list. `None` means "use the original slice unchanged" — no allocation on the fast path (no overlays anywhere or no stale fragments found). Only the path that actually filters stale rows from coverage bitmaps clones the segments. 2. `collect_stale_overlay_frags` adds a cheap version pre-check before calling `overlay_exclusion_offsets`: if every overlay on a fragment predates the index (`committed_version <= segment.dataset_version`), the fragment is skipped without loading any coverage bitmaps. Also adds an `#[ignore]` benchmark (`bench_index_query_overlay_overhead`) covering BTree, FTS, and vector ANN queries at 0/4/16 overlay layers so the overhead can be measured directly. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Address understandability gaps flagged in self-review: - `overlay_stale_index_frags`: explain that `ScalarIndexExpr` has no built-in visitor (motivating the manual stack traversal), and note that `load_named_scalar_segments` hits the cached index manifest so the per-leaf cost is bounded. - `partition_frags_by_coverage`: explain why stale fragments do not need to be threaded downstream as in the vector path — the scalar flat path already handles them via `missing_frags`, making explicit re-scoring unnecessary. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Two additions from polish-pr self-review: 1. `test_overlay_stale_with_compound_index_expression`: exercises the ScalarIndexExpr tree-walk in `overlay_stale_index_frags` with an AND predicate over two independently-indexed columns (age AND id). Closes the coverage gap where no e2e test covered multi-leaf index expressions. 2. `TODO(OSS-1325)` comment in the `fts` method noting that FTS does not yet mask overlay files — a stale FTS index entry for an overlaid text field would survive until compaction. This is out of scope for OSS-1325 but documents the known gap and records what the fix would require. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Replace the 1,500-row in-memory benchmark with a realistic 1M-row disk-based harness (10 fragments × 100k rows, 32-dim vectors). Two scenarios that exercise the actual correctness overhead: Scenario A (BTree): overlay on `age` (the indexed field). Fragment 0 falls to a 100k-row flat scan instead of an O(log n) index lookup. Measures `cold_frag0_ms` (stale fragment) vs `warm_frag1_ms` (index-served) at 0/1/4/16 overlay layers to isolate flat-scan cost from index-lookup baseline. Scenario B (Vector ANN): overlay on `vec` (the indexed field). The field-aware check confirms the BTree age overlays leave the vector index clean. One vec overlay on fragment 0 forces brute-force KNN across all 100k rows of that fragment (O(100k × 32) extra distance computations). Measures `ann_ms` at 0 vs 1 vec overlay. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… stores
commit_overlay wrote overlay files via Path::from("data/{name}"). For file://
stores, to_local_path() just prepends '/', making this resolve to /data/{name}
(root filesystem, Permission Denied). For memory:// stores it happened to work
because that branch doesn't call to_local_path at all.
Fix: use dataset.base.clone().join("data").join(filename) so that for file://
stores the path includes the dataset's base directory components, which when
prepended with '/' give the correct absolute path. For memory:// stores base
is the empty path, so join("data").join(name) produces the same string as before.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Previously, when any row in a fragment had a stale vector overlay, the entire fragment (up to 100k rows) was removed from ANN index coverage and re-scored on the flat path — adding ~1.9ms overhead per stale fragment in benchmarks. This changes the vector ANN path to row-level precision: - Stale row addresses are computed per-fragment (not whole-fragment) - A RowAddrMask::BlockList for stale rows is passed to DatasetPreFilter.overlay_block, blocking them from ANN results via the existing prefilter mechanism - Only the specific stale row addresses are re-scored via a targeted TakeExec + flat KNN path, not the whole fragment - Non-stale rows in the same fragment remain in ANN coverage For a fragment with 100k rows and 1 stale row, overhead drops from O(100k) flat KNN to O(1) targeted take. New infrastructure: - DatasetPreFilter::with_overlay_block for synchronous row-level blocks - ANNIvfSubIndexExec stores and applies overlay_block at execute time - new_knn_exec accepts Option<RowAddrMask> for overlay blocking - collect_overlay_stale_rows_for_segment: per-row stale computation - Scanner::mask_overlay_stale_rows: replaces mask_overlay_stale_segments Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
wjones127
force-pushed
the
will/oss-1325-indexes-mask-data-overlay-files-correctly
branch
from
July 1, 2026 18:20
e61ea85 to
4445bbc
Compare
…SS-1325) FTS queries could return stale hits when an overlay committed after the index build covers a fragment the FTS index also covers. The inverted index entries for those rows still reflect the pre-overlay values. Fix: in `plan_match_query`, compute which FTS segments touch stale fragments (overlay committed_version > segment.dataset_version on the indexed column) via `fts_stale_frags_and_fresh_segments`. Those segments are excluded from `MatchQueryExec` (using `new_with_segments` with the fresh subset) and all fragments they covered fall to the flat `FlatMatchQueryExec` path, which reads current overlay-merged values. The fast path (no overlays) is O(n fragments) with no segment loading. Adds two tests in overlay_index_masking: - test_fts_overlay_stale_drop_and_new_match: stale term no longer returned, new term found via flat path - test_fts_overlay_unrelated_field_not_excluded: overlay on a non-FTS field does not affect FTS coverage Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Previously, any stale overlay on a fragment caused the entire fragment to fall to the flat scan path (missing_frags), scanning up to 100k rows to re-evaluate the predicate for a single stale row. Now the BTree path matches the vector ANN approach: - partition_frags_by_coverage replaces the fragment-level overlay_stale_index_frags with overlay_stale_index_rows (row-level). Stale-but-indexed fragments stay in relevant_frags; only their per-row stale offsets are returned as a HashMap<u32, RoaringBitmap>. - MaterializeIndexExec gains overlay_block: Option<RowAddrMask> + with_overlay_block builder. The block mask is ANDed into the candidate mask before row_ids_for_mask, so stale index entries never reach downstream operators. - scalar_indexed_scan builds the block list from the stale row map and applies it to MaterializeIndexExec. A new stale-Take union path (OneShotExec(stale_row_ids) -> TakeExec -> LanceFilterExec(full_filter) -> project) re-evaluates only the stale rows against their current overlay-merged values and unions with the indexed path. Non-stale rows in a fragment with a stale overlay remain on the indexed path; only the specific stale rows pay the take cost. Adds test_btree_overlay_row_level_precision: overlays one row in a fragment so that two rows in the same fragment share the queried value. Verifies the non-stale row is returned (from the index) alongside the stale row (from the Take path). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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.
Implements OSS-1325 (index masking for data overlay files). Stacked on #7409 (OSS-1324) — this branch contains the 1322/1323/1324 commits, so review the final commit (
feat: mask data overlay files in scalar and vector index queries) until the parent PRs merge.Problem
An index built before an overlay does not reflect the overlay's values, so its entries for overlay-covered cells may be stale.
WHERE age = 25after an overlay sets a row's age to 26 must not return that row from the index;WHERE age = 26must find it. Queries must stay correct while overlays remain.Approach
For each index a query relies on, compute the fragments carrying an overlay that was committed after the index (
committed_version > index.dataset_version) and that touches a field the index covers. The check is:committed_version <= index.dataset_version) is ignored.This is the new
overlay_exclusion_offsetshelper indataset/overlay.rs.Such fragments are dropped from the index's covered set so they fall to the flat path — the same path already used for fragments the index never covered — which re-evaluates them against their current (overlay-merged) values:
FilteredReadExec): stale fragments are removed from theEvaluatedIndexcovered set, so the full filter is re-applied to them instead of trusting the index result.This drops stale index hits and surfaces new matches the index never saw.
Scope note
Exclusion granularity is per fragment: a fragment with any qualifying overlay falls to the flat path in full. The spec describes row-level exclusion; per-fragment is correct and satisfies every acceptance criterion (it re-evaluates the complete predicate / re-scores the current vector for the affected rows), at the cost of flat-scanning the whole fragment rather than just its overlaid rows. Row-level granularity (preserving index benefit for non-overlaid rows within an overlaid fragment) is a natural follow-up.
Tests
dataset::scanner::overlay_index_masking(e2e) anddataset::overlay(unit):committed_version <= index.dataset_versionnot excludedoverlay_exclusion_offsetsunit tests: version gate, field-awareness, sparse per-field coverage, multi-overlay union🤖 Generated with Claude Code