fix(index): drop stale rows when merging vector index segments

[wombatu-kun]

Related: #7370, #7371 (the delta-append manifestation of the same invariant, fixed on the query side).

Problem

optimize_indices(num_indices_to_merge >= 1) can leave two copies of the same row in a vector index, and a KNN query then returns that row twice, one ranked by its pre-update vector. This reproduces on main through either update path.

An in-place column update (update_columns + LanceOperation.Update in RewriteColumns mode) keeps the fragment id and the row address, and committing the Update prunes that fragment from the segment's fragment_bitmap. The ordinary update path (UpdateBuilder, RewriteRows) instead deletion-marks the old physical row and commits fields_modified: vec![], so the fragment stays covered; under stable row ids the replacement reuses the row id, so no read-time filter can tell the copies apart. Either way the segment's index file still physically holds the pre-update vector.

Root cause

take_partition_batches concatenates every physical batch of the old segment with the freshly scanned rows and applies no row filter, and merge_indices_with_unindexed_frags commits the union of both coverages. Both copies then sit in one segment whose bitmap legitimately spans their fragment.

This is the half of the invariant a read-time fix cannot reach. While the copies live in separate segments their bitmaps differ and a per-segment mask can separate them; once a merge folds them into one segment that information is gone from the manifest. Scalar indices solved the same class of bug on the write side in #7359; the vector path had no equivalent.

Fix

Pair every existing segment with a filter for the rows it may still contribute, and apply it wherever old data is read: in take_partition_batches before fresh rows are concatenated, and in partition_row_ids so a stale row is not reassigned during a split or a join. ExistingIndex in builder.rs keeps a segment and its coverage together so the two cannot drift apart positionally, and existing_index_sources in ivf.rs builds them from LogicalIvfView::segments(), the last point where the per-segment IndexMetadata is available.

The filter is #7359's OldIndexDataFilter, built by the same build_old_data_filter the scalar merge path already calls, so both paths now agree on what a segment may contribute: exact live row-id membership under stable row ids, which drops a deletion-marked posting whose id a rewrite reused; fragment coverage under address-style row ids, which suffices because a rewrite there lands at a new address and the stale posting is masked at query time. append.rs needs no production change at all.

Filtering is free in the address domain and costs one row-id sequence load per merged segment under stable row ids, the same cost the scalar merge already pays. It is deferred until a partition actually reads a segment, so the default optimize pass, which merges nothing unless a split or join fires, never builds one. A segment predating fragment bitmaps has unknown coverage and keeps every row it holds.

Tests

test_optimize_vector_index_drops_stale_rows_on_merge covers the in-place column rewrite across IVF_FLAT and IVF_PQ and both row-id schemes. All four fail without the fix with updated row returned 2 times after merge, both copies at the same address. IVF_FLAT stores the vector verbatim, so it also asserts the query distance and pins that the surviving copy is the post-update one; IVF_PQ covers the transposed-code path.

test_optimize_vector_index_drops_rewritten_rows_on_merge covers the UpdateBuilder path across both row-id schemes and both filter sites: an explicit merge(1) reading through take_partition_batches, and the default options over under-sized partitions reading through partition_row_ids to feed a join. It asserts the join actually happened so the case cannot degrade into a duplicate of the merge case. The stable-row-id cases fail with coverage-only filtering (rewritten row returned 2 times after optimize; row ids = [1000, 1000, ...]).

test_optimize_builds_merge_filters_only_when_merging drives a real optimize pass and asserts a delta append builds no filter while a merge builds one per merged segment. It was mutation-tested against num_indices_to_merge.unwrap_or(0) and fails when that default is widened.

test_optimize_join_after_delete_with_stable_row_ids asserted that partitions untouched by a join keep their deleted row ids. Live membership drops them index-wide, matching what its split sibling already asserted. The #7701 invariants the shared scenario pins, no live row lost and no fabricated id, are unchanged.

Not covered

The legacy v1 IVFIndex merge path (optimize_ivf_pq_indices / optimize_ivf_hnsw_indices into write_pq_partitions) has the same unfiltered merge. Current writers do not emit that format, so it is left untouched per the legacy-boundary rule in AGENTS.md.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The per-segment merge boundary is the right direction, but this revision still violates the stale-row invariant for stable row rewrites and adds full-history filter work to default delta appends.

A viable revision should select the segments actually read before constructing filters, then use deletion-aware live membership for stable IDs while keeping address-mode coverage filtering.

[lance-gatekeeper]

Normal stable-row-ID rewrites still preserve the stale posting here. UpdateBuilder deletion-marks the old physical row and writes its replacement under the same stable ID, while the old fragment remains covered. Covered therefore admits the old posting; the fresh scan adds the replacement, and the merged stable-ID prefilter admits both because that ID is live again. The query returns two copies with different index distances.

I ran this addable regression against the current head:

Reproducer

#[tokio::test]
async fn stable_row_rewrite_vector_merge() {
    use crate::dataset::UpdateBuilder;
    use arrow_array::Float32Array;

    const DIMENSION: usize = 4;
    let test_dir = TempStrDir::default();
    let schema = Arc::new(Schema::new(vec![
        Field::new("id", DataType::Int32, false),
        Field::new(
            "vector",
            DataType::FixedSizeList(
                Arc::new(Field::new("item", DataType::Float32, true)),
                DIMENSION as i32,
            ),
            false,
        ),
    ]));
    let (batch, _) = clustered_vector_batch(schema.clone(), 0, 100, DIMENSION, 0.0);
    let mut dataset = Dataset::write(
        RecordBatchIterator::new(vec![Ok(batch)], schema),
        test_dir.as_str(),
        Some(WriteParams {
            enable_stable_row_ids: true,
            ..Default::default()
        }),
    )
    .await
    .unwrap();
    dataset
        .create_index(
            &["vector"],
            IndexType::Vector,
            None,
            &VectorIndexParams::ivf_flat(1, MetricType::L2),
            true,
        )
        .await
        .unwrap();

    dataset = UpdateBuilder::new(Arc::new(dataset))
        .update_where("id = 99")
        .unwrap()
        .set("vector", "array[10.0, 10.0, 10.0, 10.0]")
        .unwrap()
        .build()
        .unwrap()
        .execute()
        .await
        .unwrap()
        .new_dataset
        .as_ref()
        .clone();
    dataset
        .optimize_indices(&OptimizeOptions::merge(1))
        .await
        .unwrap();

    let query = Float32Array::from(vec![10.0; DIMENSION]);
    let results = dataset
        .scan()
        .project(&["id"])
        .unwrap()
        .nearest("vector", &query, 100)
        .unwrap()
        .with_row_id()
        .try_into_batch()
        .await
        .unwrap();
    let count = results["id"]
        .as_primitive::<arrow::datatypes::Int32Type>()
        .values()
        .iter()
        .filter(|id| **id == 99)
        .count();
    assert_eq!(count, 1, "updated stable row returned {count} times");
}

cargo test -p lance stable_row_rewrite_vector_merge -- --nocapture expected one copy and failed with updated stable row returned 2 times.

Use deletion-aware live membership for stable-ID sources, or an equivalent filter that removes tombstoned IDs before old rows are copied. Retaining ordinary deleted postings is not safe when a live replacement reuses the ID.

[wombatu-kun]

Done 1ce0e7b

[lance-gatekeeper]

Default stable-row-ID delta optimization pays for all old-row filters here even when it later merges no existing segment. With num_indices_to_merge == None, this expression is true and the loop below loads row-ID sequences for every supplied segment; on a normal append with no split or join, build_partitions later computes num_indices_to_merge.unwrap_or(0) and selects an empty merge slice. A small append can therefore read and materialize stable IDs across the entire indexed dataset only to discard them.

Select the actual merge/rebalance sources before constructing filters, or make filter construction lazy, so the default no-adjustment append has the same no-old-row-work behavior as explicit delta append.

[wombatu-kun]

Done 1ce0e7b

[lance-gatekeeper]

✅ Gate recommendation: approve.

The follow-up resolves both prior blockers: stable row rewrites now use deletion-aware live membership, and merge filters are initialized only when an old segment is actually read. The targeted rewrite, delta-append, and stable-delete regressions pass.

[Xuanwo]

Thank you!