diff --git a/nodejs/src/table.rs b/nodejs/src/table.rs index 75213a5ad..b86afd21c 100644 --- a/nodejs/src/table.rs +++ b/nodejs/src/table.rs @@ -805,13 +805,11 @@ impl TryFrom for lancedb::table::LsmWriteSpec { ))); } }; - // An omitted `maintainedIndexes` keeps the spec's default of "every + // An omitted `maintainedIndexes` is the spec's default of "every // index the MemWAL can maintain". - let spec = match value.maintained_indexes { - Some(names) => spec.with_maintained_indexes(names), - None => spec, - }; - Ok(spec.with_writer_config_defaults(writer_config_defaults)) + Ok(spec + .with_maintained_indexes(value.maintained_indexes) + .with_writer_config_defaults(writer_config_defaults)) } } diff --git a/python/python/lancedb/_lancedb.pyi b/python/python/lancedb/_lancedb.pyi index 2acf0c945..acbe97f29 100644 --- a/python/python/lancedb/_lancedb.pyi +++ b/python/python/lancedb/_lancedb.pyi @@ -607,14 +607,13 @@ class LsmWriteSpec: def identity(column: str) -> "LsmWriteSpec": ... @staticmethod def unsharded() -> "LsmWriteSpec": ... - def with_maintained_indexes(self, indexes: List[str]) -> "LsmWriteSpec": - """Return a copy of this spec asking the MemWAL to keep exactly the - named indexes up to date as rows are appended, replacing the default - of every index the MemWAL supports.""" - ... - def with_no_maintained_indexes(self) -> "LsmWriteSpec": - """Return a copy of this spec asking the MemWAL to maintain no - indexes at all.""" + def with_maintained_indexes( + self, indexes: Optional[List[str]] + ) -> "LsmWriteSpec": + """Return a copy of this spec setting which indexes the MemWAL keeps + up to date. None maintains every index the MemWAL supports, resolved + when the spec is installed; a list is taken verbatim, and an empty + list maintains none.""" ... def with_writer_config_defaults(self, defaults: Dict[str, str]) -> "LsmWriteSpec": """Return a copy of this spec recording the given default diff --git a/python/python/lancedb/table.py b/python/python/lancedb/table.py index 0ca8606f8..df1853184 100644 --- a/python/python/lancedb/table.py +++ b/python/python/lancedb/table.py @@ -4637,8 +4637,8 @@ class AsyncTable: subscription: an index created afterwards is not maintained until the spec is unset and set again. Use ``with_maintained_indexes`` to pin an exact set — names must already be committed, so a still-building index - is rejected rather than quietly omitted — or - ``with_no_maintained_indexes`` to maintain none. + is rejected rather than quietly omitted — or pass an empty list to + maintain none. Parameters ---------- diff --git a/python/python/tests/test_lsm_write_spec.py b/python/python/tests/test_lsm_write_spec.py index e83ef9643..218793b89 100644 --- a/python/python/tests/test_lsm_write_spec.py +++ b/python/python/tests/test_lsm_write_spec.py @@ -85,7 +85,7 @@ def test_lsm_write_spec_repr(): assert s.num_buckets == 4 # A fresh spec defers its maintained set to install time. assert s.maintained_indexes is None - assert s.with_no_maintained_indexes().maintained_indexes == [] + assert s.with_maintained_indexes([]).maintained_indexes == [] assert "bucket" in repr(s) assert "id" in repr(s) assert "4" in repr(s) @@ -183,7 +183,7 @@ def test_get_lsm_write_spec(tmp_path): # Unsharded round-trips (no routing column). Opting out is distinct from # the inferred default. - table.set_lsm_write_spec(LsmWriteSpec.unsharded().with_no_maintained_indexes()) + table.set_lsm_write_spec(LsmWriteSpec.unsharded().with_maintained_indexes([])) spec = table.get_lsm_write_spec() assert spec.spec_type == "unsharded" assert spec.column is None diff --git a/python/src/table.rs b/python/src/table.rs index aa6f33d47..85c65a526 100644 --- a/python/src/table.rs +++ b/python/src/table.rs @@ -198,8 +198,8 @@ fn fmt_maintained(maintained: &Option>) -> String { /// `merge_insert`. /// /// Constructed via the `bucket(...)`, `identity(...)`, or `unsharded()` -/// classmethods, then optionally chain `with_maintained_indexes(...)` / -/// `with_no_maintained_indexes()` and `with_writer_config_defaults(...)`. +/// classmethods, then optionally chain `with_maintained_indexes(...)` and +/// `with_writer_config_defaults(...)`. /// A freshly constructed spec maintains every index the MemWAL supports, /// resolved when the spec is installed. #[pyclass(from_py_object)] @@ -241,24 +241,19 @@ impl LsmWriteSpec { } } - /// Maintain exactly `indexes`, replacing the default of "every index - /// the MemWAL supports". Each name must reference an index that - /// already exists on the table, of a type the MemWAL can maintain, - /// at the time `set_lsm_write_spec` is called. - pub fn with_maintained_indexes(&self, indexes: Vec) -> Self { + /// Set which indexes the MemWAL maintains. + /// + /// `None` — the default — maintains every index the MemWAL supports, + /// resolved when the spec is installed. A list is taken verbatim: + /// every name must already exist on the table and be a type the + /// MemWAL can maintain, and an empty list maintains nothing. + #[pyo3(signature = (indexes))] + pub fn with_maintained_indexes(&self, indexes: Option>) -> Self { Self { inner: self.inner.clone().with_maintained_indexes(indexes), } } - /// Maintain no indexes at all — the memtable serves scans and filters - /// without any in-memory index. - pub fn with_no_maintained_indexes(&self) -> Self { - Self { - inner: self.inner.clone().with_no_maintained_indexes(), - } - } - /// Replace the default `ShardWriter` configuration recorded in the /// MemWAL index, so every writer starts from the same defaults. pub fn with_writer_config_defaults(&self, defaults: HashMap) -> Self { diff --git a/rust/lancedb/src/index.rs b/rust/lancedb/src/index.rs index f72514a0a..c693dc056 100644 --- a/rust/lancedb/src/index.rs +++ b/rust/lancedb/src/index.rs @@ -449,22 +449,6 @@ pub struct IndexConfig { pub index_details: Option, } -impl IndexConfig { - /// Whether Lance's MemWAL can maintain this index on a memtable, and so - /// whether it may appear in an - /// [`LsmWriteSpec`](crate::table::LsmWriteSpec)'s maintained set. - /// - /// Support is defined by lance, which rejects an unmaintainable index when - /// a shard writer is opened — long after the spec commits — so callers - /// filter on this first. `false` when [`Self::type_url`] is absent, as it - /// is for remote tables, where the server resolves the set instead. - pub fn is_memwal_maintainable(&self) -> bool { - self.type_url - .as_deref() - .is_some_and(lance::dataset::mem_wal::is_maintainable_index_type) - } -} - #[skip_serializing_none] #[derive(Debug, Deserialize)] pub(crate) struct IndexMetadata { diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index d7a499d5c..47d067e2f 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -6535,7 +6535,7 @@ mod tests { .unwrap() }); let spec = crate::table::LsmWriteSpec::unsharded() - .with_maintained_indexes(["id_idx"]) + .with_maintained_indexes(vec!["id_idx".to_string()]) .with_writer_config_defaults([("max_memtable_rows", "1000")]); table.set_lsm_write_spec(spec).await.unwrap(); } @@ -6577,7 +6577,7 @@ mod tests { }); table .set_lsm_write_spec( - crate::table::LsmWriteSpec::bucket("id", 16).with_no_maintained_indexes(), + crate::table::LsmWriteSpec::bucket("id", 16).with_maintained_indexes(Vec::new()), ) .await .unwrap(); diff --git a/rust/lancedb/src/table.rs b/rust/lancedb/src/table.rs index 43c1ce082..05efa6e62 100644 --- a/rust/lancedb/src/table.rs +++ b/rust/lancedb/src/table.rs @@ -356,15 +356,15 @@ pub trait Tags: Send + Sync { } pub use self::merge::MergeResult; +pub use self::merge::lsm::resolve_maintained_indexes; /// Specification selecting Lance's MemWAL LSM-style write path for /// `merge_insert`. /// /// Construct via [`LsmWriteSpec::bucket`], [`LsmWriteSpec::identity`], or /// [`LsmWriteSpec::unsharded`], then optionally chain -/// [`LsmWriteSpec::with_maintained_indexes`] / -/// [`LsmWriteSpec::with_no_maintained_indexes`] (indexes the MemWAL keeps up -/// to date) and [`LsmWriteSpec::with_writer_config_defaults`] (default +/// [`LsmWriteSpec::with_maintained_indexes`] (indexes the MemWAL keeps up to +/// date) and [`LsmWriteSpec::with_writer_config_defaults`] (default /// `ShardWriter` configuration recorded in the MemWAL index). /// /// A freshly constructed spec maintains **every** index the MemWAL supports, @@ -467,25 +467,24 @@ impl LsmWriteSpec { } } - /// Maintain exactly `indexes`, replacing the default of "every supported - /// index". Each name must reference an index that already exists on the - /// table, of a type the MemWAL can maintain, at the time - /// `set_lsm_write_spec` is called. - pub fn with_maintained_indexes(self, indexes: I) -> Self - where - I: IntoIterator, - S: Into, - { - self.set_maintained_indexes(Some(indexes.into_iter().map(Into::into).collect())) - } - - /// Maintain no indexes at all — the memtable serves scans and filters - /// without any in-memory index. - pub fn with_no_maintained_indexes(self) -> Self { - self.set_maintained_indexes(Some(Vec::new())) - } - - fn set_maintained_indexes(mut self, v: Option>) -> Self { + /// Set which indexes the MemWAL maintains. + /// + /// `None` — the default — maintains every index the MemWAL supports, + /// resolved when the spec is installed. A list is taken verbatim: every + /// name must reference an index that already exists on the table, of a + /// type the MemWAL can maintain, and an empty list maintains nothing. + /// + /// ``` + /// # use lancedb::table::LsmWriteSpec; + /// // Whatever the table has when the spec is installed: + /// LsmWriteSpec::unsharded().with_maintained_indexes(None); + /// // Exactly these: + /// LsmWriteSpec::unsharded().with_maintained_indexes(vec!["id_idx".to_string()]); + /// // None at all: + /// LsmWriteSpec::unsharded().with_maintained_indexes(Vec::new()); + /// ``` + pub fn with_maintained_indexes(mut self, indexes: impl Into>>) -> Self { + let indexes = indexes.into(); match &mut self { Self::Bucket { maintained_indexes, .. @@ -495,7 +494,7 @@ impl LsmWriteSpec { } | Self::Unsharded { maintained_indexes, .. - } => *maintained_indexes = v, + } => *maintained_indexes = indexes, } self } @@ -1720,7 +1719,7 @@ impl Table { /// # async fn example(table: &Table) -> Result<(), Box> { /// table /// .set_lsm_write_spec( - /// LsmWriteSpec::bucket("id", 16).with_maintained_indexes(["id_idx"]), + /// LsmWriteSpec::bucket("id", 16).with_maintained_indexes(vec!["id_idx".to_string()]), /// ) /// .await?; /// # Ok(()) @@ -4993,7 +4992,7 @@ mod tests { // Bucket spec round-trips exactly, including the routing column (recovered // from its field id), maintained indexes, and writer config defaults. let spec = LsmWriteSpec::bucket("id", 4) - .with_maintained_indexes([idx_name.clone()]) + .with_maintained_indexes(vec![idx_name.clone()]) .with_writer_config_defaults([("durable_write", "false")]); table.set_lsm_write_spec(spec.clone()).await.unwrap(); assert_eq!(table.get_lsm_write_spec().await.unwrap(), Some(spec)); @@ -5009,7 +5008,7 @@ mod tests { table.set_lsm_write_spec(spec.clone()).await.unwrap(); assert_eq!( table.get_lsm_write_spec().await.unwrap(), - Some(spec.with_maintained_indexes([idx_name.clone()])) + Some(spec.with_maintained_indexes(vec![idx_name.clone()])) ); table.unset_lsm_write_spec().await.unwrap(); @@ -5018,7 +5017,7 @@ mod tests { table.set_lsm_write_spec(spec.clone()).await.unwrap(); assert_eq!( table.get_lsm_write_spec().await.unwrap(), - Some(spec.with_maintained_indexes([idx_name])) + Some(spec.with_maintained_indexes(vec![idx_name])) ); } @@ -5068,7 +5067,9 @@ mod tests { // Explicitly naming the bitmap index fails before anything commits. let err = table - .set_lsm_write_spec(LsmWriteSpec::unsharded().with_maintained_indexes(["tag_bitmap"])) + .set_lsm_write_spec( + LsmWriteSpec::unsharded().with_maintained_indexes(vec!["tag_bitmap".to_string()]), + ) .await .unwrap_err(); assert!( @@ -5095,7 +5096,7 @@ mod tests { // Opting out entirely is distinct from the default. table.unset_lsm_write_spec().await.unwrap(); table - .set_lsm_write_spec(LsmWriteSpec::unsharded().with_no_maintained_indexes()) + .set_lsm_write_spec(LsmWriteSpec::unsharded().with_maintained_indexes(Vec::new())) .await .unwrap(); assert_eq!( diff --git a/rust/lancedb/src/table/merge/lsm.rs b/rust/lancedb/src/table/merge/lsm.rs index d18deffe2..e4e1c73ae 100644 --- a/rust/lancedb/src/table/merge/lsm.rs +++ b/rust/lancedb/src/table/merge/lsm.rs @@ -29,6 +29,7 @@ use arrow_schema::{DataType, Schema as ArrowSchema, SchemaRef}; use lance::Dataset; use lance::dataset::mem_wal::{ DatasetMemWalExt, ShardWriter, ShardWriterConfig, evaluate_sharding_spec, + is_maintainable_index_type, }; use lance::index::DatasetIndexExt; use lance_core::datatypes::Schema as LanceSchema; @@ -37,6 +38,7 @@ use tokio::sync::RwLock; use uuid::Uuid; use crate::error::{Error, Result}; +use crate::index::IndexConfig; use crate::table::merge::{MergeInsertBuilder, MergeResult}; use crate::table::{BaseTable, LsmWriteSpec, NativeTable}; @@ -82,7 +84,10 @@ pub(crate) async fn set_lsm_write_spec(table: &NativeTable, spec: LsmWriteSpec) // Resolved before the builder takes its mutable borrow of the dataset // clone, and against the same table the spec is about to be committed to. - let maintained_indexes = resolve_maintained_indexes(table, spec.maintained_indexes()).await?; + // `list_indices` reports one entry per index name, merging an index's + // segments, so the resolved list needs no dedup. + let maintained_indexes = + resolve_maintained_indexes(&table.list_indices().await?, spec.maintained_indexes())?; let mut dataset = (*table.dataset.get().await?).clone(); let mut builder = dataset.initialize_mem_wal(); @@ -121,26 +126,35 @@ pub(crate) async fn set_lsm_write_spec(table: &NativeTable, spec: LsmWriteSpec) Ok(()) } -/// Resolve a spec's maintained-index selection against the table's committed -/// indexes. +/// Resolve an [`LsmWriteSpec`]'s maintained-index selection against a table's +/// committed indexes, as reported by +/// [`Table::list_indices`](crate::Table::list_indices). /// -/// `None` snapshots every index the MemWAL can maintain as of now — indexes -/// created after this call are not picked up. An explicit list is checked both -/// for names that do not exist and for types the memtable cannot build; the -/// latter would otherwise commit cleanly and then fail every memtable claim, -/// leaving the table unwritable far from the call that caused it. -async fn resolve_maintained_indexes( - table: &NativeTable, +/// `None` snapshots every index the MemWAL can maintain right now — indexes +/// created after this call are not picked up. A list is checked both for names +/// that do not exist and for types the memtable cannot build; the latter would +/// otherwise commit cleanly and then fail every memtable claim, leaving the +/// table unwritable far from the call that caused it. +/// +/// Public so a server installing specs on a caller's behalf resolves them the +/// same way `set_lsm_write_spec` does, rather than reimplementing the rules. +pub fn resolve_maintained_indexes( + indices: &[IndexConfig], requested: Option<&[String]>, ) -> Result> { - // `list_indices` reports one entry per index name, merging an index's - // segments, so the resolved list needs no dedup. - let indices = table.list_indices().await?; + let maintainable = |index: &IndexConfig| { + // Absent for remote tables, where the server resolves the set instead. + index + .type_url + .as_deref() + .is_some_and(is_maintainable_index_type) + }; + let Some(requested) = requested else { return Ok(indices - .into_iter() - .filter(|index| index.is_memwal_maintainable()) - .map(|index| index.name) + .iter() + .filter(|index| maintainable(index)) + .map(|index| index.name.clone()) .collect()); }; for name in requested { @@ -149,15 +163,16 @@ async fn resolve_maintained_indexes( .find(|index| &index.name == name) .ok_or_else(|| Error::InvalidInput { message: format!( - "set_lsm_write_spec: maintained index '{}' does not exist on this table", - name + "maintained index '{}' does not exist on this table; it has {}", + name, + index_name_list(indices), ), })?; - if !index.is_memwal_maintainable() { + if !maintainable(index) { return Err(Error::InvalidInput { message: format!( - "set_lsm_write_spec: maintained index '{}' has type {}, which the MemWAL \ - cannot maintain; supported types are BTREE, FTS, and IVF vector indexes", + "maintained index '{}' has type {}, which the MemWAL cannot maintain; \ + supported types are BTREE, FTS, and IVF vector indexes", name, index.index_type ), }); @@ -166,6 +181,16 @@ async fn resolve_maintained_indexes( Ok(requested.to_vec()) } +/// Render a table's index names for an error message. +fn index_name_list(indices: &[IndexConfig]) -> String { + if indices.is_empty() { + return "no indexes".to_string(); + } + let mut names: Vec<&str> = indices.iter().map(|index| index.name.as_str()).collect(); + names.sort_unstable(); + format!("[{}]", names.join(", ")) +} + // ============================================================================= // unset_lsm_write_spec // =============================================================================