From cca7da53738562758a034922176cf29c7d71c812 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Tue, 9 Jun 2026 17:22:36 +0200 Subject: [PATCH] fix(engine): IndexCoverage reports Degraded for uncovered fragments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit key_column_index_coverage checked BTREE-exists + physical_rows but not that the index actually covers the current fragments. Since edge-index creation is skipped once a BTREE exists, fragments appended later stay unindexed while coverage still reported Indexed — so the cost chooser priced a partly-full scan as fully indexed. Compare the BTREE's fragment_bitmap (public on lance_table IndexMetadata) against the dataset's current fragment ids; report Degraded when any are uncovered. A None bitmap means Lance can't report coverage — don't over-degrade. Results are unaffected (the scan returns unindexed-fragment rows either way); this corrects the cost signal. Test: a freshly-loaded edge BTREE is Indexed; after appending an edge the new fragment is uncovered → Degraded. Surface guard pins IndexMetadata.fragment_bitmap. --- crates/omnigraph/src/table_store.rs | 29 ++++++++++++-- .../omnigraph/tests/lance_surface_guards.rs | 7 +++- crates/omnigraph/tests/traversal_indexed.rs | 38 +++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/crates/omnigraph/src/table_store.rs b/crates/omnigraph/src/table_store.rs index ca2cbd0..61f9ac1 100644 --- a/crates/omnigraph/src/table_store.rs +++ b/crates/omnigraph/src/table_store.rs @@ -659,22 +659,22 @@ impl TableStore { .load_indices() .await .map_err(|e| OmniError::Lance(e.to_string()))?; - let has_btree = indices + let btree = indices .iter() .filter(|index| !is_system_index(index)) .filter(|index| index.fields.len() == 1 && index.fields[0] == field_id) - .any(|index| { + .find(|index| { index .index_details .as_ref() .map(|details| details.type_url.ends_with("BTreeIndexDetails")) .unwrap_or(false) }); - if !has_btree { + let Some(btree) = btree else { return Ok(IndexCoverage::Degraded { reason: format!("no BTREE index on '{}'", column), }); - } + }; // Same check Lance runs: a fragment missing physical_rows disables // scalar indices for the entire scan (all-or-nothing). if ds.fragments().iter().any(|f| f.physical_rows.is_none()) { @@ -682,6 +682,27 @@ impl TableStore { reason: "a fragment is missing physical_rows".to_string(), }); } + // An index only covers the fragments it was built over; fragments + // appended afterward (edge-index creation is skipped once a BTREE exists) + // are scanned unindexed. If any CURRENT fragment is absent from the + // index's `fragment_bitmap`, the scan is partly a full scan — so the + // chooser must not price it as fully indexed. A `None` bitmap means Lance + // can't report coverage; don't over-degrade in that case. + if let Some(bitmap) = btree.fragment_bitmap.as_ref() { + let uncovered = ds + .fragments() + .iter() + .filter(|f| !bitmap.contains(f.id as u32)) + .count(); + if uncovered > 0 { + return Ok(IndexCoverage::Degraded { + reason: format!( + "{} fragment(s) not covered by the index on '{}'", + uncovered, column + ), + }); + } + } Ok(IndexCoverage::Indexed) } diff --git a/crates/omnigraph/tests/lance_surface_guards.rs b/crates/omnigraph/tests/lance_surface_guards.rs index bf8e83d..1a8b252 100644 --- a/crates/omnigraph/tests/lance_surface_guards.rs +++ b/crates/omnigraph/tests/lance_surface_guards.rs @@ -402,15 +402,20 @@ async fn _compile_scalar_index_coverage_surface() -> lance::Result<()> { // disables the scalar index for the entire scan. for frag in ds.fragments().iter() { let _physical_rows: Option = frag.physical_rows; + // `key_column_index_coverage` checks each current fragment id against the + // index `fragment_bitmap`. + let _id: u64 = frag.id; } // The index sniff: BTREE presence is detected by single-field index whose - // details type_url ends with "BTreeIndexDetails". + // details type_url ends with "BTreeIndexDetails". The fragment coverage check + // reads `fragment_bitmap` (Option) and calls `.contains(u32)`. let indices = ds.load_indices().await?; for index in indices.iter() { let _fields: &Vec = &index.fields; if let Some(details) = index.index_details.as_ref() { let _type_url: &str = details.type_url.as_str(); } + let _covered: Option = index.fragment_bitmap.as_ref().map(|b| b.contains(0u32)); } Ok(()) } diff --git a/crates/omnigraph/tests/traversal_indexed.rs b/crates/omnigraph/tests/traversal_indexed.rs index 00108e3..2ceed85 100644 --- a/crates/omnigraph/tests/traversal_indexed.rs +++ b/crates/omnigraph/tests/traversal_indexed.rs @@ -97,6 +97,44 @@ async fn key_column_index_coverage_detects_btree_presence() { ); } +// An edge appended after the BTREE was built lands in a new fragment that the +// index does not cover (edge-index creation is skipped once a BTREE exists). The +// scan is then partly a full scan, so coverage must report `Degraded` — otherwise +// the cost chooser would price an unindexed-in-part scan as fully indexed. +// (Results stay correct regardless — `indexed_finds_unindexed_appended_edge`.) +#[tokio::test] +async fn coverage_degrades_for_appended_unindexed_fragment() { + let dir = tempfile::tempdir().unwrap(); + let mut db = init_and_load(&dir).await; + + // Fresh load: the Knows BTREE covers every fragment → Indexed. + let snap = snapshot_main(&db).await.unwrap(); + let edge_ds = snap.open("edge:Knows").await.unwrap(); + assert_eq!( + TableStore::key_column_index_coverage(&edge_ds, "src").await.unwrap(), + IndexCoverage::Indexed, + "freshly-loaded edge BTREE covers all fragments" + ); + + // Append an edge → a new, unindexed fragment outside the index fragment_bitmap. + mutate_main( + &mut db, + MUTATION_QUERIES, + "add_friend", + ¶ms(&[("$from", "Alice"), ("$to", "Diana")]), + ) + .await + .unwrap(); + + let snap2 = snapshot_main(&db).await.unwrap(); + let edge_ds2 = snap2.open("edge:Knows").await.unwrap(); + let cov = TableStore::key_column_index_coverage(&edge_ds2, "src").await.unwrap(); + assert!( + matches!(cov, IndexCoverage::Degraded { .. }), + "appended unindexed fragment must degrade coverage, got {cov:?}" + ); +} + #[tokio::test] #[serial] async fn indexed_matches_csr_one_hop_same_type() {