Skip to content

bug: Update credits an address-domain scalar index with the fragment it just rewrote, silently dropping rows under stable row ids #8202

Description

@wjones127

Under stable row ids, Operation::Update adds the fragment it just wrote to the fragment_bitmap of any index that does not cover a modified field — including an address-domain index (zone map, bloom filter), whose stored addresses named the fragment the update replaced. The index has no entries for the rewritten rows, but because coverage is now claimed, the scanner no longer brute-force scans them. Those rows silently disappear from index-assisted queries.

This is false negatives with no error, on a Update alone — no compaction and no optimize_indices required.

Transaction::register_pure_rewrite_rows_update_frags_in_indices (rust/lance/src/dataset/transaction.rs:2694, called from the Operation::Update arm at :2061) skips an index that covers a modified field, checks that the original fragments were covered, then inserts the new fragment ids unconditionally:

if let Some(fragment_bitmap) = index.fragment_bitmap.as_mut() {
    for fragment_id in pure_update_frag_ids.iter().map(|f| *f as u32) {
        fragment_bitmap.insert(fragment_id);
    }
}

The justification for claiming coverage is that a pure rewrite preserves row ids, so the index's entries remain valid. That holds only for a row-id-domain index. index_results_are_row_addrs (rust/lance/src/index.rs:642) already draws exactly this distinction, and its doc comment states the hazard:

Such an index cannot follow its data through a rewrite: the addresses it stores name fragments and offsets, and neither kind supports remap.

It returns true for precisely ZoneMapIndexDetails and BloomFilterIndexDetails — the two index types that fail here — and it has a single call site: the Operation::Rewrite arm at transaction.rs:2126, which correctly drops rewritten fragments from an address-domain index's coverage and lets the scanner fall back to a full scan. The Operation::Update path never consults it.

Observed

Index on a column the update does not modify. Coverage moves from [0] to [0, 1] in all three cases; only the address-domain ones lose rows.

index on domain coverage query before after
float_col, ZoneMap address [0][0,1] float_col < 100.0 100 80
int_col, BloomFilter address [0][0,1] int_col = 0 1 0
float_col, BTree row id [0][0,1] float_col < 100.0 100 100

The BTree row is the control: it receives the identical bitmap insertion and stays correct, so the insertion is not itself wrong — applying it to an address-domain index is.

A bloom filter only answers equality, so it needs a point query to be engaged at all; with a range predicate the scanner ignores the index and the fallback scan masks the bug.

Expected: all three report their pre-update counts.

Reproduction

// rust/lance/tests/repro_srid_update_addr_index.rs
use std::collections::HashMap;
use std::sync::Arc;

use arrow_array::{
    Float64Array, Int32Array, Int64Array, RecordBatch, RecordBatchIterator, StringArray,
};
use arrow_schema::{DataType, Field, Schema};
use futures::TryStreamExt;
use lance::dataset::{UpdateBuilder, WriteParams};
use lance::index::DatasetIndexExt;
use lance::Dataset;
use lance_index::scalar::{BuiltinIndexType, ScalarIndexParams};
use lance_index::IndexType;
use tempfile::tempdir;

const N: i64 = 100;

fn schema() -> Arc<Schema> {
    Arc::new(Schema::new(vec![
        Field::new("id", DataType::Int64, false),
        Field::new("int_col", DataType::Int32, false),
        Field::new("float_col", DataType::Float64, false),
        Field::new("category", DataType::Utf8, false),
    ]))
}

fn reader() -> impl arrow_array::RecordBatchReader + Send + 'static {
    let cats = ["A", "B", "C", "D", "E"];
    let batch = RecordBatch::try_new(
        schema(),
        vec![
            Arc::new(Int64Array::from_iter_values(0..N)),
            Arc::new(Int32Array::from_iter_values((0..N).map(|i| i as i32))),
            // Every row satisfies `float_col < 100.0`.
            Arc::new(Float64Array::from_iter_values((0..N).map(|i| i as f64 / 10.0))),
            Arc::new(StringArray::from_iter_values(
                (0..N).map(|i| cats[(i % 5) as usize]),
            )),
        ],
    )
    .unwrap();
    RecordBatchIterator::new(vec![Ok(batch)], schema())
}

async fn count_with_filter(ds: &Dataset, filter: &str) -> usize {
    let mut scanner = ds.scan();
    scanner.project(&["id"]).unwrap();
    scanner.filter(filter).unwrap();
    let batches: Vec<RecordBatch> =
        scanner.try_into_stream().await.unwrap().try_collect().await.unwrap();
    batches.iter().map(|b| b.num_rows()).sum()
}

async fn report(ds: &Dataset, label: &str) {
    let frags: Vec<u64> = ds.get_fragments().iter().map(|f| f.id() as u64).collect();
    let indices = ds.load_indices().await.unwrap();
    let coverage: HashMap<String, Vec<u32>> = indices
        .iter()
        .map(|i| {
            (
                i.name.clone(),
                i.fragment_bitmap.as_ref().map(|b| b.iter().collect()).unwrap_or_default(),
            )
        })
        .collect();
    println!("  [{label}] fragments={frags:?} index_coverage={coverage:?}");
}

/// Builds an index on `index_col`, updates rows matching `category = 'A'` by
/// setting `set_col` (never the indexed column), and reports `query` before/after.
async fn run(
    index_type: BuiltinIndexType,
    index_col: &str,
    set_col: &str,
    set_val: &str,
    query: &str,
    label: &str,
) -> (usize, usize) {
    let dir = tempdir().unwrap();
    let uri = dir.path().to_str().unwrap();

    let mut ds = Dataset::write(
        reader(),
        uri,
        Some(WriteParams { enable_stable_row_ids: true, ..Default::default() }),
    )
    .await
    .unwrap();

    ds.create_index(
        &[index_col],
        IndexType::Scalar,
        Some("idx".to_string()),
        &ScalarIndexParams::for_builtin(index_type),
        true,
    )
    .await
    .unwrap();

    println!("--- {label} ---");
    report(&ds, "after index build").await;
    let before = count_with_filter(&ds, query).await;

    // Rewrites the 20 rows with category='A' into a new fragment.
    let ds = UpdateBuilder::new(Arc::new(ds.clone()))
        .update_where("category = 'A'")
        .unwrap()
        .set(set_col, set_val)
        .unwrap()
        .build()
        .unwrap()
        .execute()
        .await
        .unwrap()
        .new_dataset;

    report(&ds, "after update").await;
    let after = count_with_filter(&ds, query).await;
    println!("  {query}: before={before}, after={after}");
    (before, after)
}

#[tokio::test]
async fn zonemap_loses_rows_rewritten_by_update() {
    let (before, after) = run(
        BuiltinIndexType::ZoneMap,
        "float_col",
        "int_col",
        "-1",
        "float_col < 100.0",
        "ZoneMap on float_col (address domain)",
    )
    .await;
    assert_eq!(before, N as usize);
    assert_eq!(after, N as usize, "ZoneMap dropped {} rows", N as usize - after);
}

#[tokio::test]
async fn bloomfilter_loses_rows_rewritten_by_update() {
    let (before, after) = run(
        BuiltinIndexType::BloomFilter,
        "int_col",
        "category",
        "'X'",
        "int_col = 0",
        "BloomFilter on int_col (address domain)",
    )
    .await;
    assert_eq!(before, 1);
    assert_eq!(after, 1, "BloomFilter point lookup returned {after} rows, not 1");
}

/// Control: BTree is row-id domain, so the same sequence stays correct.
#[tokio::test]
async fn btree_control_is_unaffected() {
    let (before, after) = run(
        BuiltinIndexType::BTree,
        "float_col",
        "int_col",
        "-1",
        "float_col < 100.0",
        "BTree on float_col (row id domain)",
    )
    .await;
    assert_eq!(before, N as usize);
    assert_eq!(after, N as usize, "BTree control regressed");
}

Output on main (d5050ad3):

--- ZoneMap on float_col (address domain) ---
  [after index build] fragments=[0] index_coverage={"idx": [0]}
  [after update] fragments=[0, 1] index_coverage={"idx": [0, 1]}
  float_col < 100.0: before=100, after=80        <-- 20 rows lost

--- BloomFilter on int_col (address domain) ---
  [after index build] fragments=[0] index_coverage={"idx": [0]}
  [after update] fragments=[0, 1] index_coverage={"idx": [0, 1]}
  int_col = 0: before=1, after=0                 <-- row lost

--- BTree on float_col (row id domain) ---
  [after index build] fragments=[0] index_coverage={"idx": [0]}
  [after update] fragments=[0, 1] index_coverage={"idx": [0, 1]}
  float_col < 100.0: before=100, after=100       <-- control, correct

Stable-row-id specific: without enable_stable_row_ids the registration block is not entered (it is gated on config.use_stable_row_ids), remapping runs, and all three sequences are correct.

Relationship to existing issues

Suggested fix

Mirror the Operation::Rewrite arm and skip address-domain indices in register_pure_rewrite_rows_update_frags_in_indices, alongside the existing guards:

if index_results_are_row_addrs(index) {
    continue; // its addresses named the replaced fragment; let the scanner scan it
}

The surviving original fragment can stay in coverage — its addresses are unchanged for the rows the update left alone, which is why the 80 untouched rows above are still returned correctly.


Found by lance-vibecheck (sequences srid:ws->zmf->ui, srid:ws->wo->zmf->uc, srid:ws->bfi->us), reproduced on main at d5050ad3.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingrustRust related tasks

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions