From b8bec5078161d4c8cacc83ca1365b695d1724d8d Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Thu, 30 Jul 2026 04:57:58 -0500 Subject: [PATCH 1/6] feat(mem_wal): expose whether an index type can be maintained MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MemIndexConfig::detect_index_type` is the authority on what a MemWAL memtable can build, but it is private, so a caller assembling a `maintained_indexes` set has no way to check one before committing a spec. An unmaintainable index is only rejected when a shard writer opens, which fails every memtable claim and leaves the table unwritable — far from the call that caused it. Expose the predicate, defined in terms of `detect_index_type` so the two cannot drift apart. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal.rs | 1 + rust/lance/src/dataset/mem_wal/index.rs | 32 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/rust/lance/src/dataset/mem_wal.rs b/rust/lance/src/dataset/mem_wal.rs index 784ee1fa76c..9ba791a354f 100644 --- a/rust/lance/src/dataset/mem_wal.rs +++ b/rust/lance/src/dataset/mem_wal.rs @@ -93,6 +93,7 @@ pub fn schema_with_tombstone(base: &ArrowSchema) -> Arc { } pub use api::{DatasetMemWalExt, InitializeMemWalBuilder}; +pub use index::is_maintainable_index_type; 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/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index b3da5b5b67e..93f700c430c 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -415,6 +415,16 @@ impl MemIndexConfig { } } +/// Whether the MemWAL can maintain an index of this protobuf type on a +/// memtable. +/// +/// Opening a shard writer rejects any maintained index outside this set, which +/// makes the table unwritable, so callers choosing what to maintain filter on +/// this first rather than discovering the problem at claim time. +pub fn is_maintainable_index_type(type_url: &str) -> bool { + MemIndexConfig::detect_index_type(type_url).is_ok() +} + /// Registry managing all in-memory indexes for a MemTable. /// /// Indexes are keyed by index name. Each index stores its field_id for @@ -1177,6 +1187,28 @@ mod tests { use std::sync::Arc; use uuid::Uuid; + /// The maintainable-type predicate must agree with the check a shard + /// writer makes when it opens, since callers use it to keep an + /// unmaintainable index out of `maintained_indexes` — where it would + /// instead fail every memtable claim and leave the table unwritable. + #[rstest] + #[case::btree("/lance.index.pb.BTreeIndexDetails", true)] + #[case::fts("/lance.index.pb.InvertedIndexDetails", true)] + #[case::vector("/lance.index.pb.VectorIndexDetails", true)] + #[case::bitmap("/lance.index.pb.BitmapIndexDetails", false)] + #[case::label_list("/lance.index.pb.LabelListIndexDetails", false)] + #[case::absent("", false)] + fn maintainable_index_types_match_writer_support( + #[case] type_url: &str, + #[case] maintainable: bool, + ) { + assert_eq!(is_maintainable_index_type(type_url), maintainable); + assert_eq!( + MemIndexConfig::detect_index_type(type_url).is_ok(), + maintainable + ); + } + /// Check if an index type is supported and log warning if not. fn check_index_type_supported(index_type: &str) -> bool { match index_type.to_lowercase().as_str() { From f81b3428241e2ffd177b7e3d0df71072dc814821 Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Fri, 31 Jul 2026 10:13:17 -0500 Subject: [PATCH 2/6] refactor(mem_wal): make MemIndexKind the single registry of supported types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detection and construction were two lists joined by a string: a type url was mapped to `"btree"`/`"fts"`/`"vector"`, and the shard writer matched that back with a catch-all arm. Adding a branch to the first without the second would make `is_maintainable_index_type` admit an index the writer cannot open, failing every memtable claim and leaving the table unwritable — the exact failure the predicate exists to prevent. Replace the string tag with `MemIndexKind`, and drop the catch-all. Adding a variant is now a compile error in `details_suffix` (declare the type url) and in `Dataset::mem_wal_writer` (build it). `MemIndexConfig::kind` closes the loop from the other side: a new config variant fails to compile until it declares its kind. Verified both directions by adding a variant and reading the errors back. Matching stays on the details-message suffix, not the whole type url: the same message ships under `/lance.table.`, the legacy `/lance.index.pb.`, and `type.googleapis.com/` prefixes, and all must resolve. The tests now cover each supported kind under every prefix, and every registered index type that is not supported. `detect_index_type` is kept as a deprecated wrapper; it had one caller. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal.rs | 2 +- rust/lance/src/dataset/mem_wal/api.rs | 24 ++-- rust/lance/src/dataset/mem_wal/index.rs | 173 ++++++++++++++++++++---- rust/lance/src/dataset/mem_wal/write.rs | 2 +- 4 files changed, 159 insertions(+), 42 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal.rs b/rust/lance/src/dataset/mem_wal.rs index 9ba791a354f..2a09106ed27 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}; -pub use index::is_maintainable_index_type; +pub use index::{MemIndexKind, is_maintainable_index_type}; 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 005b47d2b53..7ef2354d158 100644 --- a/rust/lance/src/dataset/mem_wal/api.rs +++ b/rust/lance/src/dataset/mem_wal/api.rs @@ -24,6 +24,7 @@ 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::scanner::sstable_cache::open_sstable; use super::scanner::{DatasetCache, ShardSnapshot}; use super::util::derived_store_params; @@ -651,41 +652,40 @@ impl DatasetMemWalExt for Dataset { )) })?; - // Detect index type and create appropriate config + // 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 index_type = MemIndexConfig::detect_index_type(type_url)?; + let kind = MemIndexKind::from_type_url(type_url) + .ok_or_else(|| unsupported_index_type(type_url))?; - match index_type { - "btree" => { + // Exhaustive on purpose: a new `MemIndexKind` has to be built here, + // or callers filtering on `is_maintainable_index_type` would admit + // an index this writer cannot open — failing every memtable claim + // and leaving the table unwritable. + match kind { + MemIndexKind::BTree => { index_configs.push(MemIndexConfig::btree_from_metadata( &index_meta, self.schema(), )?); } - "fts" => { + MemIndexKind::Fts => { index_configs.push(MemIndexConfig::fts_from_metadata( &index_meta, self.schema(), )?); } - "vector" => { + 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); } - _ => { - return Err(Error::invalid_input(format!( - "Unknown index type: {}", - index_type - ))); - } }; } diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index 93f700c430c..7beb94a5f6d 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -259,9 +259,64 @@ fn is_encodable_pk_type(data_type: &DataType) -> bool { ) } +/// The index kinds a MemTable can maintain. +/// +/// This is the registry of MemWAL index support, and it is deliberately +/// data-free: base-table indexes are identified by protobuf type url before +/// any [`MemIndexConfig`] exists, so the lookup key cannot be the config enum +/// itself. +/// +/// Adding memtable support for an index type means adding a variant here. The +/// compiler then requires the rest: [`details_suffix`](Self::details_suffix) +/// must name the type url that identifies it, `MemIndexConfig::kind` must map +/// its config, and `Dataset::mem_wal_writer` must build it. Each of those +/// matches exhaustively, so none can be skipped. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum MemIndexKind { + /// BTree index for scalar fields (point lookups, range queries). + BTree, + /// HNSW vector index built incrementally, queryable while building. + Hnsw, + /// Full-text search index. + Fts, +} + +impl MemIndexKind { + /// Every maintainable kind. Only used to drive + /// [`from_type_url`](Self::from_type_url); a kind missing from this list is + /// simply never detected, so it goes unmaintained rather than reaching a + /// memtable that cannot build it. + pub const ALL: &'static [Self] = &[Self::BTree, Self::Hnsw, Self::Fts]; + + /// Suffix of the protobuf details message identifying this kind on a base + /// table index. + /// + /// A suffix rather than a whole type url on purpose: the same details + /// message appears under several prefixes across dataset versions + /// (`/lance.table.`, the legacy `/lance.index.pb.`, and the + /// `type.googleapis.com/` form), and all of them must resolve. + pub const fn details_suffix(self) -> &'static str { + match self { + Self::BTree => "BTreeIndexDetails", + Self::Hnsw => "VectorIndexDetails", + Self::Fts => "InvertedIndexDetails", + } + } + + /// The kind a base-table index of this protobuf type maps to, or `None` + /// when a memtable cannot maintain it. + pub fn from_type_url(type_url: &str) -> Option { + Self::ALL + .iter() + .copied() + .find(|kind| type_url.ends_with(kind.details_suffix())) + } +} + /// Configuration for an index in MemWAL. /// -/// Each variant contains all the configuration needed for that index type. +/// Each variant contains all the configuration needed for that index type, +/// and pairs 1:1 with a [`MemIndexKind`] via [`kind`](Self::kind). /// `Hnsw` is boxed because `HnswBuildParams` is small but the variant may /// grow with future config (e.g. shard-specific tuning). #[derive(Debug, Clone)] @@ -275,6 +330,18 @@ pub enum MemIndexConfig { } impl MemIndexConfig { + /// The kind of index this config builds. + /// + /// The link that makes [`MemIndexKind`] the single registry: a new config + /// variant fails to compile here until it declares its kind. + pub const fn kind(&self) -> MemIndexKind { + match self { + Self::BTree(_) => MemIndexKind::BTree, + Self::Hnsw(_) => MemIndexKind::Hnsw, + Self::Fts(_) => MemIndexKind::Fts, + } + } + /// Get the index name. pub fn name(&self) -> &str { match self { @@ -364,18 +431,17 @@ impl MemIndexConfig { } /// Detect index type from protobuf type_url. + #[deprecated( + since = "10.0.1", + note = "use `MemIndexKind::from_type_url`, which returns a kind the \ + memtable is known to be able to build" + )] pub fn detect_index_type(type_url: &str) -> Result<&'static str> { - if type_url.ends_with("BTreeIndexDetails") { - Ok("btree") - } else if type_url.ends_with("InvertedIndexDetails") { - Ok("fts") - } else if type_url.ends_with("VectorIndexDetails") { - Ok("vector") - } else { - Err(Error::invalid_input(format!( - "Unsupported index type for MemWAL: {}. Supported: BTree, Inverted, Vector", - type_url - ))) + match MemIndexKind::from_type_url(type_url) { + Some(MemIndexKind::BTree) => Ok("btree"), + Some(MemIndexKind::Fts) => Ok("fts"), + Some(MemIndexKind::Hnsw) => Ok("vector"), + None => Err(unsupported_index_type(type_url)), } } @@ -422,7 +488,16 @@ impl MemIndexConfig { /// makes the table unwritable, so callers choosing what to maintain filter on /// this first rather than discovering the problem at claim time. pub fn is_maintainable_index_type(type_url: &str) -> bool { - MemIndexConfig::detect_index_type(type_url).is_ok() + MemIndexKind::from_type_url(type_url).is_some() +} + +/// The error for a base-table index no memtable can maintain, shared by the +/// detection and writer paths so they report the same thing. +pub(crate) fn unsupported_index_type(type_url: &str) -> Error { + Error::invalid_input(format!( + "Unsupported index type for MemWAL: {}. Supported: BTree, Inverted, Vector", + type_url + )) } /// Registry managing all in-memory indexes for a MemTable. @@ -1187,25 +1262,67 @@ mod tests { use std::sync::Arc; use uuid::Uuid; - /// The maintainable-type predicate must agree with the check a shard - /// writer makes when it opens, since callers use it to keep an - /// unmaintainable index out of `maintained_indexes` — where it would - /// instead fail every memtable claim and leave the table unwritable. + /// Type urls resolve to the kind a memtable will actually build, and + /// anything else resolves to nothing — callers use this to keep an + /// unmaintainable index out of `maintained_indexes`, where it would + /// otherwise fail every memtable claim and leave the table unwritable. + /// + /// The same details message ships under several prefixes depending on when + /// the index was written, so each supported kind is checked under all of + /// them: matching is on the suffix precisely so old datasets still resolve. #[rstest] - #[case::btree("/lance.index.pb.BTreeIndexDetails", true)] - #[case::fts("/lance.index.pb.InvertedIndexDetails", true)] - #[case::vector("/lance.index.pb.VectorIndexDetails", true)] - #[case::bitmap("/lance.index.pb.BitmapIndexDetails", false)] - #[case::label_list("/lance.index.pb.LabelListIndexDetails", false)] - #[case::absent("", false)] - fn maintainable_index_types_match_writer_support( + #[case::btree("/lance.table.BTreeIndexDetails", Some(MemIndexKind::BTree))] + #[case::btree_legacy("/lance.index.pb.BTreeIndexDetails", Some(MemIndexKind::BTree))] + #[case::btree_any( + "type.googleapis.com/lance.table.BTreeIndexDetails", + Some(MemIndexKind::BTree) + )] + #[case::fts("/lance.table.InvertedIndexDetails", Some(MemIndexKind::Fts))] + #[case::fts_legacy("/lance.index.pb.InvertedIndexDetails", Some(MemIndexKind::Fts))] + #[case::fts_any( + "type.googleapis.com/lance.index.InvertedIndexDetails", + Some(MemIndexKind::Fts) + )] + #[case::vector("/lance.index.pb.VectorIndexDetails", Some(MemIndexKind::Hnsw))] + #[case::vector_any( + "type.googleapis.com/lance.index.VectorIndexDetails", + Some(MemIndexKind::Hnsw) + )] + #[case::bitmap("/lance.table.BitmapIndexDetails", None)] + #[case::label_list("/lance.table.LabelListIndexDetails", None)] + #[case::ngram("/lance.table.NGramIndexDetails", None)] + #[case::zone_map("/lance.table.ZoneMapIndexDetails", None)] + #[case::bloom_filter("/lance.index.pb.BloomFilterIndexDetails", None)] + #[case::json("/lance.index.pb.JsonIndexDetails", None)] + #[case::fm("/lance.index.pb.FMIndexDetails", None)] + #[case::absent("", None)] + fn type_urls_resolve_to_the_kind_the_writer_builds( #[case] type_url: &str, - #[case] maintainable: bool, + #[case] expected: Option, ) { - assert_eq!(is_maintainable_index_type(type_url), maintainable); + assert_eq!(MemIndexKind::from_type_url(type_url), expected); + assert_eq!(is_maintainable_index_type(type_url), expected.is_some()); + } + + /// `MemIndexKind::ALL` is hand-maintained, so a kind left out of it would + /// silently stop resolving. Every kind reachable from a config must appear. + #[test] + fn every_kind_is_registered_and_uniquely_identified() { + for kind in MemIndexKind::ALL { + assert_eq!( + MemIndexKind::from_type_url(&format!("/lance.table.{}", kind.details_suffix())), + Some(*kind), + "{kind:?} does not resolve from its own suffix", + ); + } + let suffixes: std::collections::HashSet<_> = MemIndexKind::ALL + .iter() + .map(|k| k.details_suffix()) + .collect(); assert_eq!( - MemIndexConfig::detect_index_type(type_url).is_ok(), - maintainable + suffixes.len(), + MemIndexKind::ALL.len(), + "two kinds share a details suffix, so one can never be resolved", ); } diff --git a/rust/lance/src/dataset/mem_wal/write.rs b/rust/lance/src/dataset/mem_wal/write.rs index 7f05cbfff3e..3e588ca35ce 100644 --- a/rust/lance/src/dataset/mem_wal/write.rs +++ b/rust/lance/src/dataset/mem_wal/write.rs @@ -37,7 +37,7 @@ use uuid::Uuid; pub use super::index::{ BTreeIndexConfig, BTreeMemIndex, FtsIndexConfig, HnswIndexConfig, IndexStore, MemIndexConfig, - validate_index_configs, + MemIndexKind, validate_index_configs, }; pub use super::memtable::CacheConfig; pub use super::memtable::MemTable; From 6525661e871981d82c12be0cdc424fe8356dc9fd Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Fri, 31 Jul 2026 10:24:51 -0500 Subject: [PATCH 3/6] fix(mem_wal): pack flushed vector index details like every other writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MemWAL flush hand-built the `Any` for a flushed generation's vector index, producing `type.googleapis.com/lance.index.VectorIndexDetails`. No other writer in lance emits that prefix — index creation goes through `Any::from_msg`, which stamps the package (`/lance.index.pb.`). The hand-written form was introduced with the MemWAL regional writer and is the only production source of it; every other occurrence is a test fixture. Readers all match on the message-name suffix (`IndexDetails::is_vector`, `type_name_from_uri`, `MemIndexKind::from_type_url`), so nothing was broken by it — it just made MemWAL-flushed indexes the odd ones out, and it is the sort of difference that trips the next person who reaches for an exact url comparison. Use the existing `vector_index_details_default()` helper. A default `VectorIndexDetails` carries no set fields, so the encoded payload is unchanged and only the type url moves. Datasets flushed by the old code keep the old url, so suffix matching stays required; the kind-resolution test pins that form explicitly. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal/index.rs | 29 ++++++++----------- .../src/dataset/mem_wal/memtable/flush.rs | 10 ++++--- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index 7beb94a5f6d..c5aa4c4d9a2 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -291,10 +291,10 @@ impl MemIndexKind { /// Suffix of the protobuf details message identifying this kind on a base /// table index. /// - /// A suffix rather than a whole type url on purpose: the same details - /// message appears under several prefixes across dataset versions - /// (`/lance.table.`, the legacy `/lance.index.pb.`, and the - /// `type.googleapis.com/` form), and all of them must resolve. + /// A suffix rather than a whole type url on purpose: the prefix varies + /// across dataset versions (`/lance.table.`, `/lance.index.pb.`, and the + /// `type.googleapis.com/` form MemWAL flush once wrote), and every form + /// has to keep resolving. pub const fn details_suffix(self) -> &'static str { match self { Self::BTree => "BTreeIndexDetails", @@ -1267,24 +1267,19 @@ mod tests { /// unmaintainable index out of `maintained_indexes`, where it would /// otherwise fail every memtable claim and leave the table unwritable. /// - /// The same details message ships under several prefixes depending on when - /// the index was written, so each supported kind is checked under all of - /// them: matching is on the suffix precisely so old datasets still resolve. + /// Matching is on the message-name suffix, not the whole url, because the + /// prefix varies with how and when the index was written: `Any::from_msg` + /// emits the package (`/lance.table.`, `/lance.index.pb.`), while MemWAL + /// flush used to hand-write a `type.googleapis.com/` url, so existing + /// datasets still carry that form. #[rstest] #[case::btree("/lance.table.BTreeIndexDetails", Some(MemIndexKind::BTree))] - #[case::btree_legacy("/lance.index.pb.BTreeIndexDetails", Some(MemIndexKind::BTree))] - #[case::btree_any( - "type.googleapis.com/lance.table.BTreeIndexDetails", - Some(MemIndexKind::BTree) - )] #[case::fts("/lance.table.InvertedIndexDetails", Some(MemIndexKind::Fts))] #[case::fts_legacy("/lance.index.pb.InvertedIndexDetails", Some(MemIndexKind::Fts))] - #[case::fts_any( - "type.googleapis.com/lance.index.InvertedIndexDetails", - Some(MemIndexKind::Fts) - )] #[case::vector("/lance.index.pb.VectorIndexDetails", Some(MemIndexKind::Hnsw))] - #[case::vector_any( + // Written by MemWAL flush before it switched to `Any::from_msg`; flushed + // generations in existing datasets still have it. + #[case::vector_legacy_flush( "type.googleapis.com/lance.index.VectorIndexDetails", Some(MemIndexKind::Hnsw) )] diff --git a/rust/lance/src/dataset/mem_wal/memtable/flush.rs b/rust/lance/src/dataset/mem_wal/memtable/flush.rs index 0a5c97ee6c7..6068c991e76 100644 --- a/rust/lance/src/dataset/mem_wal/memtable/flush.rs +++ b/rust/lance/src/dataset/mem_wal/memtable/flush.rs @@ -33,6 +33,7 @@ use crate::dataset::mem_wal::manifest::ShardManifestStore; use crate::dataset::mem_wal::scanner::SsTableWarmer; use crate::dataset::mem_wal::scanner::exec::{compute_pk_hash, validate_pk_types}; use crate::dataset::mem_wal::util::{derived_store_params, generate_random_hash, sstable_path}; +use crate::index::vector::details::vector_index_details_default; use crate::session::Session; #[derive(Debug, Clone)] @@ -1105,10 +1106,11 @@ impl MemTableFlusher { ); index_writer.finish().await?; - let index_details = Some(Arc::new(prost_types::Any { - type_url: "type.googleapis.com/lance.index.VectorIndexDetails".to_string(), - value: vec![], - })); + // Pack the details the same way index creation does. Hand-building the + // `Any` here produced a `type.googleapis.com/` type url, which no other + // writer in lance emits; readers match on the message-name suffix, so + // it worked, but it made memwal-flushed indexes the odd ones out. + let index_details = Some(Arc::new(vector_index_details_default())); let index_meta = IndexMetadata { uuid: index_uuid, name: config.name.clone(), From f6835cc0b5b30985f8ed952bdd056e121566e09d Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Fri, 31 Jul 2026 11:00:45 -0500 Subject: [PATCH 4/6] docs(mem_wal): trim the comments added with MemIndexKind Same content, fewer words. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal/api.rs | 7 +- rust/lance/src/dataset/mem_wal/index.rs | 75 +++++++------------ .../src/dataset/mem_wal/memtable/flush.rs | 6 +- 3 files changed, 31 insertions(+), 57 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/api.rs b/rust/lance/src/dataset/mem_wal/api.rs index 7ef2354d158..b927c32623a 100644 --- a/rust/lance/src/dataset/mem_wal/api.rs +++ b/rust/lance/src/dataset/mem_wal/api.rs @@ -662,10 +662,9 @@ impl DatasetMemWalExt for Dataset { let kind = MemIndexKind::from_type_url(type_url) .ok_or_else(|| unsupported_index_type(type_url))?; - // Exhaustive on purpose: a new `MemIndexKind` has to be built here, - // or callers filtering on `is_maintainable_index_type` would admit - // an index this writer cannot open — failing every memtable claim - // and leaving the table unwritable. + // 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( diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index c5aa4c4d9a2..c29d63d70f5 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -259,18 +259,12 @@ fn is_encodable_pk_type(data_type: &DataType) -> bool { ) } -/// The index kinds a MemTable can maintain. +/// The index kinds a MemTable can maintain — the registry of MemWAL index +/// support. Data-free because indexes are identified by type url before any +/// [`MemIndexConfig`] exists. /// -/// This is the registry of MemWAL index support, and it is deliberately -/// data-free: base-table indexes are identified by protobuf type url before -/// any [`MemIndexConfig`] exists, so the lookup key cannot be the config enum -/// itself. -/// -/// Adding memtable support for an index type means adding a variant here. The -/// compiler then requires the rest: [`details_suffix`](Self::details_suffix) -/// must name the type url that identifies it, `MemIndexConfig::kind` must map -/// its config, and `Dataset::mem_wal_writer` must build it. Each of those -/// matches exhaustively, so none can be skipped. +/// Adding a variant is a compile error in [`details_suffix`](Self::details_suffix), +/// `MemIndexConfig::kind`, and `Dataset::mem_wal_writer` until each handles it. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum MemIndexKind { /// BTree index for scalar fields (point lookups, range queries). @@ -282,19 +276,15 @@ pub enum MemIndexKind { } impl MemIndexKind { - /// Every maintainable kind. Only used to drive - /// [`from_type_url`](Self::from_type_url); a kind missing from this list is - /// simply never detected, so it goes unmaintained rather than reaching a - /// memtable that cannot build it. + /// Every maintainable kind. A kind missing here is never detected, so it + /// goes unmaintained rather than reaching a memtable that cannot build it. pub const ALL: &'static [Self] = &[Self::BTree, Self::Hnsw, Self::Fts]; - /// Suffix of the protobuf details message identifying this kind on a base - /// table index. + /// Suffix of the protobuf details message identifying this kind. /// - /// A suffix rather than a whole type url on purpose: the prefix varies - /// across dataset versions (`/lance.table.`, `/lance.index.pb.`, and the - /// `type.googleapis.com/` form MemWAL flush once wrote), and every form - /// has to keep resolving. + /// Only the suffix: the prefix varies by dataset version + /// (`/lance.table.`, `/lance.index.pb.`, and the `type.googleapis.com/` + /// form MemWAL flush once wrote), and all must resolve. pub const fn details_suffix(self) -> &'static str { match self { Self::BTree => "BTreeIndexDetails", @@ -313,10 +303,9 @@ impl MemIndexKind { } } -/// Configuration for an index in MemWAL. +/// Configuration for an index in MemWAL. Pairs 1:1 with [`MemIndexKind`] via +/// [`kind`](Self::kind). /// -/// Each variant contains all the configuration needed for that index type, -/// and pairs 1:1 with a [`MemIndexKind`] via [`kind`](Self::kind). /// `Hnsw` is boxed because `HnswBuildParams` is small but the variant may /// grow with future config (e.g. shard-specific tuning). #[derive(Debug, Clone)] @@ -330,10 +319,8 @@ pub enum MemIndexConfig { } impl MemIndexConfig { - /// The kind of index this config builds. - /// - /// The link that makes [`MemIndexKind`] the single registry: a new config - /// variant fails to compile here until it declares its kind. + /// The kind this config builds. Links the config enum to the registry, so + /// a new variant must declare its kind. pub const fn kind(&self) -> MemIndexKind { match self { Self::BTree(_) => MemIndexKind::BTree, @@ -481,18 +468,16 @@ impl MemIndexConfig { } } -/// Whether the MemWAL can maintain an index of this protobuf type on a -/// memtable. +/// Whether the MemWAL can maintain an index of this protobuf type. /// -/// Opening a shard writer rejects any maintained index outside this set, which -/// makes the table unwritable, so callers choosing what to maintain filter on -/// this first rather than discovering the problem at claim time. +/// 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. pub fn is_maintainable_index_type(type_url: &str) -> bool { MemIndexKind::from_type_url(type_url).is_some() } -/// The error for a base-table index no memtable can maintain, shared by the -/// detection and writer paths so they report the same thing. +/// 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!( "Unsupported index type for MemWAL: {}. Supported: BTree, Inverted, Vector", @@ -1262,23 +1247,16 @@ mod tests { use std::sync::Arc; use uuid::Uuid; - /// Type urls resolve to the kind a memtable will actually build, and - /// anything else resolves to nothing — callers use this to keep an - /// unmaintainable index out of `maintained_indexes`, where it would - /// otherwise fail every memtable claim and leave the table unwritable. - /// - /// Matching is on the message-name suffix, not the whole url, because the - /// prefix varies with how and when the index was written: `Any::from_msg` - /// emits the package (`/lance.table.`, `/lance.index.pb.`), while MemWAL - /// flush used to hand-write a `type.googleapis.com/` url, so existing - /// datasets still carry that form. + /// Matching is on the message-name suffix, not the whole url: `Any::from_msg` + /// emits the package (`/lance.table.`, `/lance.index.pb.`), while MemWAL flush + /// used to hand-write a `type.googleapis.com/` url that existing datasets + /// still carry. #[rstest] #[case::btree("/lance.table.BTreeIndexDetails", Some(MemIndexKind::BTree))] #[case::fts("/lance.table.InvertedIndexDetails", Some(MemIndexKind::Fts))] #[case::fts_legacy("/lance.index.pb.InvertedIndexDetails", Some(MemIndexKind::Fts))] #[case::vector("/lance.index.pb.VectorIndexDetails", Some(MemIndexKind::Hnsw))] - // Written by MemWAL flush before it switched to `Any::from_msg`; flushed - // generations in existing datasets still have it. + // What MemWAL flush wrote before it switched to `Any::from_msg`. #[case::vector_legacy_flush( "type.googleapis.com/lance.index.VectorIndexDetails", Some(MemIndexKind::Hnsw) @@ -1299,8 +1277,7 @@ mod tests { assert_eq!(is_maintainable_index_type(type_url), expected.is_some()); } - /// `MemIndexKind::ALL` is hand-maintained, so a kind left out of it would - /// silently stop resolving. Every kind reachable from a config must appear. + /// `ALL` is hand-maintained, so a kind left out of it stops resolving. #[test] fn every_kind_is_registered_and_uniquely_identified() { for kind in MemIndexKind::ALL { diff --git a/rust/lance/src/dataset/mem_wal/memtable/flush.rs b/rust/lance/src/dataset/mem_wal/memtable/flush.rs index 6068c991e76..694d7f282ab 100644 --- a/rust/lance/src/dataset/mem_wal/memtable/flush.rs +++ b/rust/lance/src/dataset/mem_wal/memtable/flush.rs @@ -1106,10 +1106,8 @@ impl MemTableFlusher { ); index_writer.finish().await?; - // Pack the details the same way index creation does. Hand-building the - // `Any` here produced a `type.googleapis.com/` type url, which no other - // writer in lance emits; readers match on the message-name suffix, so - // it worked, but it made memwal-flushed indexes the odd ones out. + // Packed the same way index creation does; hand-building the `Any` here + // produced a `type.googleapis.com/` url no other writer in lance emits. let index_details = Some(Arc::new(vector_index_details_default())); let index_meta = IndexMetadata { uuid: index_uuid, From f13af084e7dca54001eaaddb69f16ee3ce8f8ba2 Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Fri, 31 Jul 2026 11:07:47 -0500 Subject: [PATCH 5/6] refactor(mem_wal): drop detect_index_type instead of deprecating it `MemIndexKind::from_type_url` replaced its only caller, and nothing else in lance, lancedb, or sophon ever used it. Deprecating a function with no consumers just leaves a second way to ask the same question. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal/index.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index c29d63d70f5..f7b39faf1a6 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -417,21 +417,6 @@ impl MemIndexConfig { )) } - /// Detect index type from protobuf type_url. - #[deprecated( - since = "10.0.1", - note = "use `MemIndexKind::from_type_url`, which returns a kind the \ - memtable is known to be able to build" - )] - pub fn detect_index_type(type_url: &str) -> Result<&'static str> { - match MemIndexKind::from_type_url(type_url) { - Some(MemIndexKind::BTree) => Ok("btree"), - Some(MemIndexKind::Fts) => Ok("fts"), - Some(MemIndexKind::Hnsw) => Ok("vector"), - None => Err(unsupported_index_type(type_url)), - } - } - fn fts_format_version_from_metadata( index_meta: &IndexMetadata, ) -> Result { From d836ee04390fd2acc2ed64e24cd791c16ef7a1da Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Fri, 31 Jul 2026 11:10:27 -0500 Subject: [PATCH 6/6] test(mem_wal): drop the dead index-type support check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check_index_type_supported` and its test were never reachable from production code, and asserted on a string vocabulary — `scalar`, `fulltext`, and the `btree`/`hnsw`/`fts` tags — that nothing emits now that `detect_index_type` is gone. Takes the `log::warn` import with it. Co-Authored-By: Claude Opus 5 (1M context) --- rust/lance/src/dataset/mem_wal/index.rs | 30 ------------------------- 1 file changed, 30 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/index.rs b/rust/lance/src/dataset/mem_wal/index.rs index f7b39faf1a6..7525b573d54 100644 --- a/rust/lance/src/dataset/mem_wal/index.rs +++ b/rust/lance/src/dataset/mem_wal/index.rs @@ -1227,7 +1227,6 @@ mod tests { use super::*; use arrow_array::{Int32Array, StringArray}; use arrow_schema::{DataType, Field, Schema as ArrowSchema}; - use log::warn; use rstest::rstest; use std::sync::Arc; use uuid::Uuid; @@ -1283,23 +1282,6 @@ mod tests { ); } - /// Check if an index type is supported and log warning if not. - fn check_index_type_supported(index_type: &str) -> bool { - match index_type.to_lowercase().as_str() { - "btree" | "scalar" => true, - "hnsw" | "vector" => true, - "fts" | "inverted" | "fulltext" => true, - _ => { - warn!( - "Index type '{}' is not supported for MemWAL. \ - Supported types: btree, hnsw, fts. Skipping.", - index_type - ); - false - } - } - } - fn create_test_schema() -> Arc { Arc::new(ArrowSchema::new(vec![ Field::new("id", DataType::Int32, false), @@ -1652,18 +1634,6 @@ mod tests { assert_eq!(fts.doc_count(), 3); } - #[test] - fn test_check_index_type_supported() { - assert!(check_index_type_supported("btree")); - assert!(check_index_type_supported("BTree")); - assert!(check_index_type_supported("hnsw")); - assert!(check_index_type_supported("vector")); - assert!(check_index_type_supported("fts")); - assert!(check_index_type_supported("inverted")); - - assert!(!check_index_type_supported("unknown")); - } - #[test] fn fts_from_metadata_preserves_format_version() { let arrow_schema = create_test_schema();