[JaySon-Huang]

Fixes #8849 (assembled-from-pieces variant; see the issue for the whole-dataset allow-list cost analysis)

Problem

DatasetPreFilter::do_create_deletion_mask_row_id builds a whole-dataset allow list by loading every fragment's RowIdSequence and deletion vector and folding them into one RowAddrTreeMap. The cost is proportional to the dataset size, not to what changed. The whole mask is cached under (manifest version, restrict_hash), which amortizes less than it looks:

• every commit bumps the version and invalidates the entry, so the first prefiltered query after any write rebuilds every fragment's live-row set;
• distinct restrict_to sets get distinct cache entries, so a workload with varying fragment restrictions rebuilds repeatedly at the same version;
• the resident mask scales with the dataset.

Change

Assemble the allow list from per-fragment pieces. Each piece is one fragment's live stable row ids (row id sequence minus deletion vector, exactly what the old fold OR-ed in for that fragment), cached in the dataset metadata cache under its content identity:

• the row id generation, using the same content identity as RowIdSequenceKey: the inline digest, or the lineage data file's path/base_id for RowIdMeta::Column;
• the deletion identity, including the inline deletion payload when the vector is stored in the manifest.

Pieces are fetched concurrently and OR-ed on a blocking thread. The whole-mask cache under (version, restrict_hash) is kept, so repeated identical queries still hit it unchanged.

• Commits that leave a fragment's row ids and deletions untouched keep its piece warm: a post-commit rebuild only needs to build a new piece for touched fragments.
• Overlapping restrict_to sets share pieces instead of reloading them.
• Missing fragments (index bitmaps referencing removed fragments) contribute no piece, as before.

Correctness

A piece's content is fully determined by its key identity: any change to a fragment's row ids or deletions changes row_id_meta or the deletion file identity, so a stale piece cannot be served. mask(F) = OR(pieces(F ∩ manifest)) is identical to the old restricted fold (#6563/#6877 semantics preserved).

Tests: the existing prefilter mask tests (including the #6877 regression test) pass unchanged. Regression coverage now includes cross-commit invalidation, multi-fragment restrictions, spilled RowIdMeta::Column lineage, inline deletion vectors, and typed cache/I/O evidence. The cache test warms the underlying row-id and deletion caches, then verifies that a restricted rebuild performs zero reads and records exactly one piece-cache hit plus one miss for the newly composed mask. A subsequent commit that leaves that fragment untouched reuses the piece without I/O.

Performance

No wall-time benchmark was rerun for the integrated head. The original PR described an author-reported local NVMe benchmark from an earlier revision, but the measured commit and harness are not identified here, so those numbers are omitted. The cache/I/O regression verifies behavior, not throughput.

Cost and scope

The final composition still ORs the selected pieces and remains O(dataset live rows); this change does not make a full allow-list build proportional only to the restriction. Pieces and composed masks may both remain resident in the existing metadata-cache budget.

The piece cache avoids rebuilding each fragment's live-row set and avoids repeated RowIdSequenceKey / DeletionFileKey lookups. When those underlying caches are already warm, the previous path could also avoid storage I/O. This change therefore does not universally reduce object-store round trips; it can avoid lower-tier loads when those caches are cold, evicted, or contended.

Notes / follow-ups

• A further step (not included) is intersecting the index-bitmap union with a filter's fragment coverage for selective scalar-index prefilters; FilterLoader currently does not expose fragment coverage.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The content-keyed per-fragment cache is a sound reuse boundary, but piece construction must preserve the existing async/CPU separation. Load row IDs and deletion vectors asynchronously, then run the clone, deletion masking, and tree construction for each miss on spawn_cpu; the final merge can remain on the CPU pool.

[wjones127]

This is a good fix, but the test needs some improvement to show what it is claiming.

[wjones127]

issue(blocking): the test claims that "while untouched fragments are served from cache", but that isn't asserted. Could you use the assert_io_eq!() macro to assert the amount of IO each of the operations does, and thus prove that caching is happening?

[Xuanwo]

Addressed in 0c0698e222980708f1a788bf5325838799680b76. The focused regression now creates a real RowIdMeta::Column fragment with an inline deletion vector, proves the cold build performs storage reads, and then asserts the restricted rebuild performs read_iops == 0 with an exact metadata-cache delta of one piece hit and one composed-mask miss while the RowIdSequenceKey and DeletionFileKey entries remain resident. A subsequent unrelated commit reuses the same piece with zero reads. Could you take another look?

[lance-gatekeeper]

✅ Gate recommendation: approve.

The added I/O and cache-counter assertions demonstrate reuse across restrictions and commits, addressing the cache-reuse test request. The prefilter and cache-identity tests pass. The merged implementation preserves restricted-mask semantics and CPU-pool separation while accounting for spilled lineage files and inline deletion payloads.

[wjones127]

Thanks for the update. The new test counts reads and cache hits/misses, so it shows the reuse directly, which is what I asked for.

I'm holding off on approving the piece cache for now. We're planning to move indices to the address domain (#10960), which removes the need for this allow list, so I'd like a benchmark showing the cache is worth it in the meantime. I also left a question about sharing the fragment row-id identity with #10800, and a note on the DeletionFileKey identity change.

[wjones127]

note: depending on which merges first, this will need to be updated to handle the new path on deletion files. FYI @geruh See: #10914

[wjones127]

question: Our plan is to move indices to the address domain (#10960). Once they return row addresses, the prefilter only needs a block list of deleted addresses, like the non-stable-row-id path, so this whole-dataset allow list goes away for index search. merge_insert reaches it through the BTREE join-key lookup (#6877), so that use goes away too.

Given that, I'd want a benchmark showing a meaningful interim win before we add another cache layer here. The row-id sequence and deletion vector are already cached under RowIdSequenceKey/DeletionFileKey, so on a warm session a piece hit only saves the clone + mask + RowAddrTreeMap::from per fragment, and the final OR is still O(live rows).

Could you share before/after numbers for the case this targets: the first prefiltered query after a small commit on a large stable-row-id table?

[wjones127]

question: #10800 also keys per-fragment row-id state by row-id source + deletion identity, but builds that identity differently. On top of the RowIdSequenceKey bytes it adds physical_rows, the resolved base path, and the row-id column index:

lance/rust/lance/src/dataset/rowids/cache.rs

Lines 192 to 206 in b5ee153

	builder.write_u64(fragment.physical_rows.map_or(u64::MAX, |rows| rows as u64));
	if let RowIdMeta::Column = row_id_meta {
	let file = file.ok_or_else(|| {
	Error::corrupt_file_named(
	&dataset.uri,
	format!("fragment {} has no spilled row id file", fragment.id),
	)
	})?;
	write_base_identity(dataset, file.base_id, &mut builder)?;
	// The same file may be projected through a different physical column.
	for (field, column) in file.fields.iter().zip(file.column_indices.iter()) {
	if *field == ROW_ID_FIELD_ID {
	builder.write_u32(*column as u32);
	}
	}

The base-path part comes from its keys living in the root namespace rather than the dataset-prefixed cache. The column index and physical_rows aren't explained by that, though. If they're needed there, are they needed here, and in RowIdSequenceKey? If not, could both PRs use this write_row_id_identity helper, so there is one definition of a fragment's row-id identity?

[wjones127]

suggestion: This changes the identity of the existing DeletionFileKey (and bumps its schema), not only the new piece key. #10800 independently adds the inline payload to its deletion identity too:

lance/rust/lance/src/dataset/rowids/cache.rs

Lines 111 to 123 in b5ee153

	DeletionFileKey {
	fragment_id: fragment.id,
	deletion_file: file,
	}
	.write_key(&mut builder);
	if let Some(inline) = &file.inline {
	builder.write_some();
	builder.write_bytes(inline);
	} else {
	builder.write_none();
	write_base_identity(dataset, file.base_id, &mut builder)?;
	}
	Ok(builder.finish())

So two inline deletion files with the same (read_version, id, type) and different bytes look like a real case. Could the description call this out as a fix to the deletion-vector cache? Once this lands, #10800's extra inline write becomes redundant.