From b392f5f2176ec7ab0eee63eb287b6030b85360f5 Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Thu, 6 Aug 2026 16:07:04 -0500 Subject: [PATCH 1/4] feat(mem_wal): validate maintained indexes against the writer's rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `is_maintainable_index_type` only inspects an index's protobuf type url. That is not enough to decide whether the MemWAL can maintain an index: every vector index sub-type carries `VectorIndexDetails` and so maps to `MemIndexKind::Hnsw`, but the memtable's HNSW needs a `FixedSizeList` column. A caller filtering a maintained set on the type url alone can therefore commit an index that every shard-writer open then rejects, leaving the table unwritable — for example an IVF-PQ index over `FixedSizeList`. The rules that actually decide this already exist in `validate_index_configs`, but they only run at `ShardWriter::open`, far from the call that chose the set. Extract the config-building loop out of `mem_wal_writer` into `build_index_configs` and add `validate_maintained_indexes`, which builds the same configs and runs the same validation against the same shard schema (base + `_tombstone`). Both the writer and the new entry point go through that one path, so a set that validates is a set the writer can open, and the two cannot drift. The check is all-or-nothing: it reports the first index it cannot maintain rather than returning a usable subset, so a caller inferring a set surfaces the error instead of silently dropping an index the caller believes is maintained. Also correct the `is_maintainable_index_type` docs, which advised filtering a maintained set on it — the advice that makes the above unwritable table reachable. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal.rs | 2 +- rust/lance/src/dataset/mem_wal/api.rs | 322 +++++++++++++++++++----- rust/lance/src/dataset/mem_wal/index.rs | 13 +- 3 files changed, 277 insertions(+), 60 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal.rs b/rust/lance/src/dataset/mem_wal.rs index 4a9dd3e95f0..46659acba8c 100644 --- a/rust/lance/src/dataset/mem_wal.rs +++ b/rust/lance/src/dataset/mem_wal.rs @@ -92,7 +92,7 @@ pub fn schema_with_tombstone(base: &ArrowSchema) -> Arc { )) } -pub use api::{DatasetMemWalExt, InitializeMemWalBuilder}; +pub use api::{DatasetMemWalExt, InitializeMemWalBuilder, validate_maintained_indexes}; pub use index::{MemIndexKind, is_maintainable_index_type}; pub use manifest::ShardManifestStore; pub use memtable::scanner::MemTableScanner; diff --git a/rust/lance/src/dataset/mem_wal/api.rs b/rust/lance/src/dataset/mem_wal/api.rs index b927c32623a..8fa51dbf286 100644 --- a/rust/lance/src/dataset/mem_wal/api.rs +++ b/rust/lance/src/dataset/mem_wal/api.rs @@ -9,8 +9,9 @@ use std::collections::HashMap; use std::sync::Arc; -use arrow_schema::DataType; +use arrow_schema::{DataType, Schema as ArrowSchema}; use async_trait::async_trait; +use lance_core::datatypes::Schema as LanceSchema; use lance_core::{Error, Result}; use lance_index::mem_wal::{MEM_WAL_INDEX_NAME, MemWalIndexDetails, ShardingField, ShardingSpec}; use lance_index::vector::hnsw::builder::HnswBuildParams; @@ -24,9 +25,10 @@ use crate::index::DatasetIndexInternalExt; use crate::index::mem_wal::{load_mem_wal_index_details, new_mem_wal_index_meta}; use super::ShardWriterConfig; -use super::index::{MemIndexKind, unsupported_index_type}; +use super::index::{MemIndexKind, unsupported_index_type, validate_index_configs}; use super::scanner::sstable_cache::open_sstable; use super::scanner::{DatasetCache, ShardSnapshot}; +use super::schema_with_tombstone; use super::util::derived_store_params; use super::write::MemIndexConfig; use super::write::ShardWriter; @@ -633,60 +635,8 @@ impl DatasetMemWalExt for Dataset { // Get maintained_indexes from the MemWalIndex details let maintained_indexes = &mem_wal_index.details.maintained_indexes; - // Load index configs for each maintained index - let mut index_configs = Vec::new(); - for index_name in maintained_indexes { - // A maintained index can split into multiple physical segments - // (e.g. `optimize_indices(append)` deltas), which the singular - // `load_index_by_name` rejects. Every segment carries the same - // type and params, so take the first match. - let index_meta = self - .load_indices_by_name(index_name) - .await? - .into_iter() - .next() - .ok_or_else(|| { - Error::invalid_input(format!( - "Index '{}' from maintained_indexes not found on dataset", - index_name - )) - })?; - - // Detect index kind and create appropriate config - let type_url = index_meta - .index_details - .as_ref() - .map(|d| d.type_url.as_str()) - .unwrap_or(""); - - let kind = MemIndexKind::from_type_url(type_url) - .ok_or_else(|| unsupported_index_type(type_url))?; - - // Exhaustive: a new kind must be built here, or callers filtering on - // `is_maintainable_index_type` would admit an index this writer - // cannot open, failing every memtable claim. - match kind { - MemIndexKind::BTree => { - index_configs.push(MemIndexConfig::btree_from_metadata( - &index_meta, - self.schema(), - )?); - } - MemIndexKind::Fts => { - index_configs.push(MemIndexConfig::fts_from_metadata( - &index_meta, - self.schema(), - )?); - } - MemIndexKind::Hnsw => { - let hnsw_params = config.hnsw_params.get(index_name).cloned(); - let vector_config = - load_vector_index_config(self, index_name, &index_meta, hnsw_params) - .await?; - index_configs.push(vector_config); - } - }; - } + let index_configs = + build_index_configs(self, maintained_indexes, &config.hnsw_params).await?; // Set shard_id in config config.shard_id = shard_id; @@ -716,6 +666,104 @@ impl DatasetMemWalExt for Dataset { } } +/// Build the in-memory index configurations for `index_names`. +/// +/// Shared by [`DatasetMemWalExt::mem_wal_writer`], which opens a shard writer +/// with the result, and [`validate_maintained_indexes`], which only checks it. +/// Both go through here so a set that validates is a set the writer can build. +async fn build_index_configs( + dataset: &Dataset, + index_names: &[String], + hnsw_params: &HashMap, +) -> Result> { + let mut index_configs = Vec::with_capacity(index_names.len()); + for index_name in index_names { + // A maintained index can split into multiple physical segments + // (e.g. `optimize_indices(append)` deltas), which the singular + // `load_index_by_name` rejects. Every segment carries the same + // type and params, so take the first match. + let index_meta = dataset + .load_indices_by_name(index_name) + .await? + .into_iter() + .next() + .ok_or_else(|| { + Error::invalid_input(format!( + "Index '{}' from maintained_indexes not found on dataset", + index_name + )) + })?; + + // Detect index kind and create appropriate config + let type_url = index_meta + .index_details + .as_ref() + .map(|d| d.type_url.as_str()) + .unwrap_or(""); + + let kind = MemIndexKind::from_type_url(type_url) + .ok_or_else(|| unsupported_index_type(type_url))?; + + // Exhaustive: a new kind must be built here, or callers filtering on + // `is_maintainable_index_type` would admit an index this writer + // cannot open, failing every memtable claim. + index_configs.push(match kind { + MemIndexKind::BTree => { + MemIndexConfig::btree_from_metadata(&index_meta, dataset.schema())? + } + MemIndexKind::Fts => MemIndexConfig::fts_from_metadata(&index_meta, dataset.schema())?, + MemIndexKind::Hnsw => { + let hnsw_params = hnsw_params.get(index_name).cloned(); + load_vector_index_config(dataset, index_name, &index_meta, hnsw_params).await? + } + }); + } + Ok(index_configs) +} + +/// Whether the MemWAL can maintain `index_names` on `dataset`. +/// +/// [`is_maintainable_index_type`](super::is_maintainable_index_type) inspects +/// only an index's protobuf type url, which cannot answer this on its own: every +/// vector index sub-type maps to [`MemIndexKind::Hnsw`], but the memtable's HNSW +/// needs a `FixedSizeList` column. Committing an index that fails that +/// rule into `maintained_indexes` leaves a table whose every shard-writer open +/// fails — so the type url admits sets that are unwritable. +/// +/// This resolves each index against the dataset schema and applies the same +/// rules [`ShardWriter::open`] does, so a set that passes here is a set the +/// writer can open. Call it before committing a maintained set, whether that set +/// was named explicitly or taken from the dataset's own indexes. +/// +/// All-or-nothing by design: it reports the first index that cannot be +/// maintained rather than returning a usable subset. A caller that would +/// otherwise infer a set should surface that error and let the user name the +/// indexes explicitly — silently dropping one leaves an index the caller +/// believes is maintained and the writer never touches. +/// +/// Reads each vector index to inherit its distance type, so the cost scales with +/// the number of vector indexes named. +pub async fn validate_maintained_indexes(dataset: &Dataset, index_names: &[String]) -> Result<()> { + // Build params never affect validation — it reads an index's name, column, + // and field id, never its HNSW tuning — so the writer's params are not + // needed to reach the same verdict. + let index_configs = build_index_configs(dataset, index_names, &HashMap::new()).await?; + + // Validate against the shard schema, base + `_tombstone`, exactly as + // `ShardWriter::open` extends it: field ids and the primary key resolve + // against that schema, not the base one. + let base_schema: ArrowSchema = dataset.schema().into(); + let schema = schema_with_tombstone(&base_schema); + let lance_schema = LanceSchema::try_from(schema.as_ref())?; + let pk_columns: Vec = lance_schema + .unenforced_primary_key() + .iter() + .map(|field| field.name.clone()) + .collect(); + + validate_index_configs(&index_configs, schema.as_ref(), &lance_schema, &pk_columns) +} + /// Build an in-memory HNSW vector index configuration from a base-table /// vector index entry. /// @@ -787,6 +835,76 @@ mod tests { ])) } + /// A dataset of 256 rows with an IVF vector index `vector_idx` over a + /// `FixedSizeList` column. + async fn dataset_with_vector_index(uri: &str, item_type: DataType) -> Dataset { + use crate::index::vector::VectorIndexParams; + use arrow_array::ArrayRef; + use arrow_array::builder::{FixedSizeListBuilder, Float32Builder, Float64Builder}; + use lance_linalg::distance::DistanceType; + + const ROWS: i32 = 256; + const DIM: i32 = 4; + + let schema = Arc::new(ArrowSchema::new(vec![ + Field::new("id", DataType::Int32, false), + Field::new( + "vector", + DataType::FixedSizeList(Arc::new(Field::new("item", item_type.clone(), true)), DIM), + true, + ), + ])); + + let vectors: ArrayRef = match item_type { + DataType::Float32 => { + let mut builder = FixedSizeListBuilder::new(Float32Builder::new(), DIM); + for row in 0..ROWS { + for d in 0..DIM { + builder.values().append_value((row * DIM + d) as f32); + } + builder.append(true); + } + Arc::new(builder.finish()) + } + DataType::Float64 => { + let mut builder = FixedSizeListBuilder::new(Float64Builder::new(), DIM); + for row in 0..ROWS { + for d in 0..DIM { + builder.values().append_value((row * DIM + d) as f64); + } + builder.append(true); + } + Arc::new(builder.finish()) + } + other => panic!("unhandled vector item type {other:?}"), + }; + + let batch = RecordBatch::try_new( + schema.clone(), + vec![ + Arc::new(Int32Array::from((0..ROWS).collect::>())), + vectors, + ], + ) + .unwrap(); + + let reader = RecordBatchIterator::new([Ok(batch)], schema.clone()); + let mut dataset = Dataset::write(reader, uri, Some(WriteParams::default())) + .await + .unwrap(); + dataset + .create_index( + &["vector"], + IndexType::Vector, + Some("vector_idx".to_string()), + &VectorIndexParams::ivf_flat(1, DistanceType::L2), + true, + ) + .await + .unwrap(); + dataset + } + fn id_v_batch(schema: &Arc, ids: &[i32]) -> RecordBatch { let vs: Vec = ids.iter().map(|i| i * 10).collect(); RecordBatch::try_new( @@ -799,6 +917,98 @@ mod tests { .unwrap() } + #[tokio::test] + async fn test_validate_maintained_indexes_rejects_non_f32_vector_column() { + // A vector index over `FixedSizeList` is a perfectly valid + // durable index, and it carries the same `VectorIndexDetails` type url + // every vector index does — so the type url alone admits it. The memtable + // HNSW needs Float32, so committing it as maintained would leave a table + // whose every shard-writer open fails. + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let dataset = dataset_with_vector_index(&uri, DataType::Float64).await; + + let index_meta = dataset + .load_indices_by_name("vector_idx") + .await + .unwrap() + .into_iter() + .next() + .unwrap(); + assert!( + super::super::is_maintainable_index_type( + index_meta.index_details.as_ref().unwrap().type_url.as_str() + ), + "the type url cannot see the column type, so it admits this index" + ); + + let error = validate_maintained_indexes(&dataset, &["vector_idx".to_string()]) + .await + .expect_err("a Float64 vector column is not maintainable"); + assert!( + error.to_string().contains("FixedSizeList"), + "unexpected error: {error}" + ); + } + + #[tokio::test] + async fn test_validate_maintained_indexes_accepts_f32_vector_column() { + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let dataset = dataset_with_vector_index(&uri, DataType::Float32).await; + + validate_maintained_indexes(&dataset, &["vector_idx".to_string()]) + .await + .expect("a Float32 vector column is maintainable"); + } + + #[tokio::test] + async fn test_validate_maintained_indexes_accepts_btree() { + // Guards the shard-schema plumbing: validation resolves field ids against + // base + `_tombstone`, so a scalar index on an ordinary column must pass. + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let schema = id_v_schema(); + let reader = + RecordBatchIterator::new([Ok(id_v_batch(&schema, &[1, 2, 3]))], schema.clone()); + let mut dataset = Dataset::write(reader, &uri, Some(WriteParams::default())) + .await + .unwrap(); + dataset + .create_index( + &["id"], + IndexType::BTree, + Some("id_idx".to_string()), + &ScalarIndexParams::default(), + true, + ) + .await + .unwrap(); + + validate_maintained_indexes(&dataset, &["id_idx".to_string()]) + .await + .expect("a BTree index on an Int32 column is maintainable"); + } + + #[tokio::test] + async fn test_validate_maintained_indexes_rejects_unknown_name() { + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let schema = id_v_schema(); + let reader = RecordBatchIterator::new([Ok(id_v_batch(&schema, &[1]))], schema.clone()); + let dataset = Dataset::write(reader, &uri, Some(WriteParams::default())) + .await + .unwrap(); + + let error = validate_maintained_indexes(&dataset, &["nope".to_string()]) + .await + .expect_err("an index that does not exist cannot be maintained"); + assert!( + error.to_string().contains("not found"), + "unexpected: {error}" + ); + } + #[tokio::test] async fn test_prewarm_mem_wal_opens_and_warms_indexes() { // `prewarm_mem_wal` opens each SSTable (into the base diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index 3c0e2b1a3f1..74db51a4ebf 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -462,9 +462,16 @@ impl MemIndexConfig { /// Whether the MemWAL can maintain an index of this protobuf type. /// -/// Opening a shard writer rejects anything outside this set, which makes the -/// table unwritable — so filter on this before committing a maintained set, -/// not at claim time. +/// A necessary but *not* sufficient condition: the type url does not carry the +/// column, so every vector index sub-type maps to [`MemIndexKind::Hnsw`] here +/// regardless of whether its column is the `FixedSizeList` the memtable +/// HNSW needs. Filtering a maintained set on this alone can commit an index that +/// every shard-writer open then rejects, leaving the table unwritable. +/// +/// Use +/// [`validate_maintained_indexes`](super::validate_maintained_indexes) to decide +/// what to commit; it resolves each index against the dataset schema and applies +/// the writer's own rules. pub fn is_maintainable_index_type(type_url: &str) -> bool { MemIndexKind::from_type_url(type_url).is_some() } From 7884bd55e30dbe29be8f520df2e12eea22ef0b3c Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Thu, 6 Aug 2026 17:38:58 -0500 Subject: [PATCH 2/4] refactor(mem_wal)!: drop is_maintainable_index_type `validate_maintained_indexes` answers the question this was added for, and answers it correctly, leaving no caller in either lance or lancedb. Keeping it is a hazard rather than a convenience: a public predicate that looks like it decides whether an index can be maintained, sitting beside one that actually does, invites the same wrong choice that made an IVF-PQ index over `FixedSizeList` commit into a maintained set and leave the table unwritable. `MemIndexKind::from_type_url` stays and is unchanged, so callers that want the kind a type url resolves to still have it. BREAKING CHANGE: `is_maintainable_index_type` is removed. Use `validate_maintained_indexes` to decide what to commit as a maintained set, or `MemIndexKind::from_type_url` for the type-url mapping alone. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal.rs | 2 +- rust/lance/src/dataset/mem_wal/api.rs | 64 ++++++++++--------------- rust/lance/src/dataset/mem_wal/index.rs | 17 ------- 3 files changed, 27 insertions(+), 56 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal.rs b/rust/lance/src/dataset/mem_wal.rs index 46659acba8c..b179e7d56d4 100644 --- a/rust/lance/src/dataset/mem_wal.rs +++ b/rust/lance/src/dataset/mem_wal.rs @@ -93,7 +93,7 @@ pub fn schema_with_tombstone(base: &ArrowSchema) -> Arc { } pub use api::{DatasetMemWalExt, InitializeMemWalBuilder, validate_maintained_indexes}; -pub use index::{MemIndexKind, is_maintainable_index_type}; +pub use index::MemIndexKind; pub use manifest::ShardManifestStore; pub use memtable::scanner::MemTableScanner; pub use scanner::{LsmDataSource, LsmGeneration, LsmScanner, ShardSnapshot}; diff --git a/rust/lance/src/dataset/mem_wal/api.rs b/rust/lance/src/dataset/mem_wal/api.rs index 8fa51dbf286..c865227264c 100644 --- a/rust/lance/src/dataset/mem_wal/api.rs +++ b/rust/lance/src/dataset/mem_wal/api.rs @@ -668,9 +668,9 @@ impl DatasetMemWalExt for Dataset { /// Build the in-memory index configurations for `index_names`. /// -/// Shared by [`DatasetMemWalExt::mem_wal_writer`], which opens a shard writer -/// with the result, and [`validate_maintained_indexes`], which only checks it. -/// Both go through here so a set that validates is a set the writer can build. +/// Shared by [`DatasetMemWalExt::mem_wal_writer`] and +/// [`validate_maintained_indexes`], so a set that validates is one the writer +/// can build. async fn build_index_configs( dataset: &Dataset, index_names: &[String], @@ -704,9 +704,8 @@ async fn build_index_configs( let kind = MemIndexKind::from_type_url(type_url) .ok_or_else(|| unsupported_index_type(type_url))?; - // Exhaustive: a new kind must be built here, or callers filtering on - // `is_maintainable_index_type` would admit an index this writer - // cannot open, failing every memtable claim. + // Exhaustive: a new kind must be built here, or a maintained set could + // name an index this writer cannot open, failing every memtable claim. index_configs.push(match kind { MemIndexKind::BTree => { MemIndexConfig::btree_from_metadata(&index_meta, dataset.schema())? @@ -723,35 +722,25 @@ async fn build_index_configs( /// Whether the MemWAL can maintain `index_names` on `dataset`. /// -/// [`is_maintainable_index_type`](super::is_maintainable_index_type) inspects -/// only an index's protobuf type url, which cannot answer this on its own: every -/// vector index sub-type maps to [`MemIndexKind::Hnsw`], but the memtable's HNSW -/// needs a `FixedSizeList` column. Committing an index that fails that -/// rule into `maintained_indexes` leaves a table whose every shard-writer open -/// fails — so the type url admits sets that are unwritable. +/// Applies the same rules [`ShardWriter::open`] does, so a set that passes here +/// is a set the writer can open. Call it before committing a maintained set: a +/// type url alone cannot decide this — every vector sub-type maps to +/// [`MemIndexKind::Hnsw`], but the memtable's HNSW needs a +/// `FixedSizeList` column — and committing an index that fails leaves +/// a table whose every shard-writer open fails. /// -/// This resolves each index against the dataset schema and applies the same -/// rules [`ShardWriter::open`] does, so a set that passes here is a set the -/// writer can open. Call it before committing a maintained set, whether that set -/// was named explicitly or taken from the dataset's own indexes. +/// All-or-nothing: it reports the first index it cannot maintain rather than +/// returning a usable subset, so a caller inferring a set surfaces the error +/// instead of dropping an index it believes is maintained. /// -/// All-or-nothing by design: it reports the first index that cannot be -/// maintained rather than returning a usable subset. A caller that would -/// otherwise infer a set should surface that error and let the user name the -/// indexes explicitly — silently dropping one leaves an index the caller -/// believes is maintained and the writer never touches. -/// -/// Reads each vector index to inherit its distance type, so the cost scales with -/// the number of vector indexes named. +/// Opens each vector index to inherit its distance type. pub async fn validate_maintained_indexes(dataset: &Dataset, index_names: &[String]) -> Result<()> { - // Build params never affect validation — it reads an index's name, column, - // and field id, never its HNSW tuning — so the writer's params are not - // needed to reach the same verdict. + // Validation reads an index's name, column, and field id, never its HNSW + // tuning, so the writer's build params are not needed here. let index_configs = build_index_configs(dataset, index_names, &HashMap::new()).await?; - // Validate against the shard schema, base + `_tombstone`, exactly as - // `ShardWriter::open` extends it: field ids and the primary key resolve - // against that schema, not the base one. + // The shard schema is base + `_tombstone`, as `ShardWriter::open` extends + // it; field ids and the primary key resolve against that, not the base. let base_schema: ArrowSchema = dataset.schema().into(); let schema = schema_with_tombstone(&base_schema); let lance_schema = LanceSchema::try_from(schema.as_ref())?; @@ -919,11 +908,9 @@ mod tests { #[tokio::test] async fn test_validate_maintained_indexes_rejects_non_f32_vector_column() { - // A vector index over `FixedSizeList` is a perfectly valid - // durable index, and it carries the same `VectorIndexDetails` type url - // every vector index does — so the type url alone admits it. The memtable - // HNSW needs Float32, so committing it as maintained would leave a table - // whose every shard-writer open fails. + // A `FixedSizeList` vector index is a valid durable index whose + // type url resolves to `Hnsw` like any other, but the memtable HNSW needs + // Float32 — so committing it would leave the table unwritable. let tmp = tempfile::tempdir().unwrap(); let uri = format!("{}/base", tmp.path().to_str().unwrap()); let dataset = dataset_with_vector_index(&uri, DataType::Float64).await; @@ -935,11 +922,12 @@ mod tests { .into_iter() .next() .unwrap(); - assert!( - super::super::is_maintainable_index_type( + assert_eq!( + MemIndexKind::from_type_url( index_meta.index_details.as_ref().unwrap().type_url.as_str() ), - "the type url cannot see the column type, so it admits this index" + Some(MemIndexKind::Hnsw), + "the type url cannot see the column type" ); let error = validate_maintained_indexes(&dataset, &["vector_idx".to_string()]) diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index 74db51a4ebf..8e9add8bd80 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -460,22 +460,6 @@ impl MemIndexConfig { } } -/// Whether the MemWAL can maintain an index of this protobuf type. -/// -/// A necessary but *not* sufficient condition: the type url does not carry the -/// column, so every vector index sub-type maps to [`MemIndexKind::Hnsw`] here -/// regardless of whether its column is the `FixedSizeList` the memtable -/// HNSW needs. Filtering a maintained set on this alone can commit an index that -/// every shard-writer open then rejects, leaving the table unwritable. -/// -/// Use -/// [`validate_maintained_indexes`](super::validate_maintained_indexes) to decide -/// what to commit; it resolves each index against the dataset schema and applies -/// the writer's own rules. -pub fn is_maintainable_index_type(type_url: &str) -> bool { - MemIndexKind::from_type_url(type_url).is_some() -} - /// Shared by the detection and writer paths so both report the same thing. pub(crate) fn unsupported_index_type(type_url: &str) -> Error { Error::invalid_input(format!( @@ -1341,7 +1325,6 @@ mod tests { #[case] expected: Option, ) { assert_eq!(MemIndexKind::from_type_url(type_url), expected); - assert_eq!(is_maintainable_index_type(type_url), expected.is_some()); } /// `ALL` is hand-maintained, so a kind left out of it stops resolving. From f0652582625d4e8874e8d381047f57f2746c5a6e Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Thu, 6 Aug 2026 19:57:40 -0500 Subject: [PATCH 3/4] fix(mem_wal): name the index in the unmaintainable-type error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error carried only the type url, so a caller validating a maintained set learned that some index was unmaintainable but not which one — the one thing it needs in order to drop the offender. `unsupported_index_type` now takes the index name. Its one remaining caller is `build_index_configs`; the doc claiming it is shared with a detection path is stale, as that path was `is_maintainable_index_type`. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal/api.rs | 116 ++++++++++++++++++++---- rust/lance/src/dataset/mem_wal/index.rs | 9 +- 2 files changed, 102 insertions(+), 23 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/api.rs b/rust/lance/src/dataset/mem_wal/api.rs index c865227264c..a5fe235496b 100644 --- a/rust/lance/src/dataset/mem_wal/api.rs +++ b/rust/lance/src/dataset/mem_wal/api.rs @@ -146,9 +146,9 @@ impl<'a> InitializeMemWalBuilder<'a> { /// Set the base-table indexes to maintain in MemTables, replacing any /// previously set list. /// - /// Each name must reference an index that already exists on the dataset. - /// The primary key btree, when present, is maintained implicitly and must - /// not be listed. + /// Each name must reference an existing index the MemWAL can maintain; + /// [`execute`](Self::execute) enforces both. The primary key btree, when + /// present, is maintained implicitly and must not be listed. pub fn maintained_indexes(mut self, indexes: I) -> Self where I: IntoIterator, @@ -190,8 +190,9 @@ impl<'a> InitializeMemWalBuilder<'a> { /// Initialize MemWAL on the dataset, committing the MemWAL system index. /// - /// Fails if any maintained index does not exist, if the selected sharding - /// configuration is invalid, or if MemWAL is already initialized. + /// Fails if any maintained index does not exist or cannot be maintained by + /// the MemWAL, if the selected sharding configuration is invalid, or if + /// MemWAL is already initialized. pub async fn execute(self) -> Result<()> { let Self { dataset, @@ -204,20 +205,16 @@ impl<'a> InitializeMemWalBuilder<'a> { let (sharding_specs, num_shards) = resolve_sharding(dataset, sharding)?; let indices = dataset.load_indices().await?; - for index_name in &maintained_indexes { - if !indices.iter().any(|idx| &idx.name == index_name) { - return Err(Error::invalid_input(format!( - "Index '{}' not found on dataset. maintained_indexes must reference existing indexes.", - index_name - ))); - } - } if indices.iter().any(|idx| idx.name == MEM_WAL_INDEX_NAME) { return Err(Error::invalid_input( "MemWAL is already initialized on this dataset.", )); } + // Gate the commit, not just a preflight a caller may skip: a set the + // writer cannot open leaves the table unwritable. + validate_maintained_indexes(dataset, &maintained_indexes).await?; + let details = MemWalIndexDetails { num_shards, sharding_specs, @@ -702,7 +699,7 @@ async fn build_index_configs( .unwrap_or(""); let kind = MemIndexKind::from_type_url(type_url) - .ok_or_else(|| unsupported_index_type(type_url))?; + .ok_or_else(|| unsupported_index_type(index_name, type_url))?; // Exhaustive: a new kind must be built here, or a maintained set could // name an index this writer cannot open, failing every memtable claim. @@ -723,11 +720,11 @@ async fn build_index_configs( /// Whether the MemWAL can maintain `index_names` on `dataset`. /// /// Applies the same rules [`ShardWriter::open`] does, so a set that passes here -/// is a set the writer can open. Call it before committing a maintained set: a -/// type url alone cannot decide this — every vector sub-type maps to -/// [`MemIndexKind::Hnsw`], but the memtable's HNSW needs a -/// `FixedSizeList` column — and committing an index that fails leaves -/// a table whose every shard-writer open fails. +/// is a set the writer can open. [`InitializeMemWalBuilder::execute`] runs it +/// before committing; it is public so a caller inferring a set can ask the same +/// question first. A type url alone cannot decide this — every vector sub-type +/// maps to [`MemIndexKind::Hnsw`], but the memtable's HNSW needs a +/// `FixedSizeList` column. /// /// All-or-nothing: it reports the first index it cannot maintain rather than /// returning a usable subset, so a caller inferring a set surfaces the error @@ -978,6 +975,38 @@ mod tests { .expect("a BTree index on an Int32 column is maintainable"); } + #[tokio::test] + async fn test_validate_maintained_indexes_rejects_unmaintainable_kind() { + // A bitmap index is a valid durable index the memtable cannot build. + // The error names it, so a caller validating a set knows which to drop. + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let schema = id_v_schema(); + let reader = + RecordBatchIterator::new([Ok(id_v_batch(&schema, &[1, 2, 3]))], schema.clone()); + let mut dataset = Dataset::write(reader, &uri, Some(WriteParams::default())) + .await + .unwrap(); + dataset + .create_index( + &["v"], + IndexType::Bitmap, + Some("v_bitmap".to_string()), + &ScalarIndexParams::for_builtin(lance_index::scalar::BuiltinIndexType::Bitmap), + true, + ) + .await + .unwrap(); + + let error = validate_maintained_indexes(&dataset, &["v_bitmap".to_string()]) + .await + .expect_err("the memtable cannot build a bitmap index"); + assert!( + error.to_string().contains("v_bitmap"), + "the error must name the index: {error}" + ); + } + #[tokio::test] async fn test_validate_maintained_indexes_rejects_unknown_name() { let tmp = tempfile::tempdir().unwrap(); @@ -997,6 +1026,55 @@ mod tests { ); } + #[tokio::test] + async fn test_initialize_mem_wal_rejects_unmaintainable_index() { + // Initialization persists the set, so it must apply the writer's rules + // itself: a Float64 vector index committed here leaves the table unwritable. + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let mut dataset = dataset_with_vector_index(&uri, DataType::Float64).await; + + let error = dataset + .initialize_mem_wal() + .unsharded() + .maintained_indexes(["vector_idx"]) + .execute() + .await + .expect_err("a Float64 vector column is not maintainable"); + assert!( + error.to_string().contains("FixedSizeList"), + "unexpected error: {error}" + ); + assert!( + dataset.mem_wal_index_details().await.unwrap().is_none(), + "a rejected maintained set must not be committed" + ); + } + + #[tokio::test] + async fn test_initialize_mem_wal_rejects_unknown_index_name() { + let tmp = tempfile::tempdir().unwrap(); + let uri = format!("{}/base", tmp.path().to_str().unwrap()); + let schema = id_v_schema(); + let reader = RecordBatchIterator::new([Ok(id_v_batch(&schema, &[1]))], schema.clone()); + let mut dataset = Dataset::write(reader, &uri, Some(WriteParams::default())) + .await + .unwrap(); + + let error = dataset + .initialize_mem_wal() + .unsharded() + .maintained_indexes(["nope"]) + .execute() + .await + .expect_err("maintained_indexes must reference existing indexes"); + assert!( + error.to_string().contains("nope") && error.to_string().contains("not found"), + "unexpected error: {error}" + ); + assert!(dataset.mem_wal_index_details().await.unwrap().is_none()); + } + #[tokio::test] async fn test_prewarm_mem_wal_opens_and_warms_indexes() { // `prewarm_mem_wal` opens each SSTable (into the base diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index 8e9add8bd80..b9c1af446c1 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -460,11 +460,12 @@ impl MemIndexConfig { } } -/// Shared by the detection and writer paths so both report the same thing. -pub(crate) fn unsupported_index_type(type_url: &str) -> Error { +/// Names the index, not just its type: a caller validating a maintained set +/// needs to know which one to drop. +pub(crate) fn unsupported_index_type(index_name: &str, type_url: &str) -> Error { Error::invalid_input(format!( - "Unsupported index type for MemWAL: {}. Supported: BTree, Inverted, Vector", - type_url + "index '{}' has type {}, which the MemWAL cannot maintain. Supported: BTree, Inverted, Vector", + index_name, type_url )) } From 05a3463f573623a68a9b419080d2dc837565c61c Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Thu, 6 Aug 2026 20:26:19 -0500 Subject: [PATCH 4/4] docs(mem_wal): note that MemWAL does not track post-initialization changes `maintained_indexes` is validated against the dataset as it stands at initialization. Dropping or replacing a maintained index, or projecting away its column, leaves the set naming something the writer cannot build; a change that races the initialization commit lands the same way, since the commit rebases and nothing revalidates against the rebased manifest. Both surface as a failing `mem_wal_writer` rather than being prevented. Recorded as a known limitation on the API module, with pointers from `execute` and `validate_maintained_indexes`; handling dataset updates after initialization is follow-up work. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal/api.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rust/lance/src/dataset/mem_wal/api.rs b/rust/lance/src/dataset/mem_wal/api.rs index a5fe235496b..6480aff06bc 100644 --- a/rust/lance/src/dataset/mem_wal/api.rs +++ b/rust/lance/src/dataset/mem_wal/api.rs @@ -5,6 +5,14 @@ //! //! This module provides the user-facing API for initializing and using MemWAL //! on a Dataset. +//! +//! # Limitations +//! +//! MemWAL does not track dataset changes made after it is initialized: dropping +//! or replacing a maintained index, or projecting away its column, leaves +//! `maintained_indexes` naming something the writer cannot build. A change that +//! races the initialization commit lands the same way. Both surface as a failing +//! `mem_wal_writer`; handling them is follow-up work. use std::collections::HashMap; use std::sync::Arc; @@ -193,6 +201,9 @@ impl<'a> InitializeMemWalBuilder<'a> { /// Fails if any maintained index does not exist or cannot be maintained by /// the MemWAL, if the selected sharding configuration is invalid, or if /// MemWAL is already initialized. + /// + /// Validated against the dataset as it stands here; see the module-level + /// limitations for changes made afterwards. pub async fn execute(self) -> Result<()> { let Self { dataset, @@ -730,6 +741,8 @@ async fn build_index_configs( /// returning a usable subset, so a caller inferring a set surfaces the error /// instead of dropping an index it believes is maintained. /// +/// Judges `dataset` as given; see the module-level limitations. +/// /// Opens each vector index to inherit its distance type. pub async fn validate_maintained_indexes(dataset: &Dataset, index_names: &[String]) -> Result<()> { // Validation reads an index's name, column, and field id, never its HNSW