mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-14 14:46:53 +00:00
refactor: fold the maintained-index API down to one entry point
Three cleanups to the LsmWriteSpec surface, all removing something: `with_no_maintained_indexes` is gone. `with_maintained_indexes` now takes `impl Into<Option<Vec<String>>>`, so `None` asks for every supported index, a list pins an exact set, and an empty list maintains none — one method covering what two did, and the argument reads the same as the field it sets. `IndexConfig::is_memwal_maintainable` is gone. What a memtable can build is lance's to say, and lance already says it via `is_maintainable_index_type`; mirroring that onto a lancedb type only added a place for the two to drift. `resolve_maintained_indexes` is now public and takes `&[IndexConfig]` rather than a table. It was already the whole of the resolution rules — infer from the committed set, or validate a named one — and a server installing specs on a caller's behalf needs exactly those rules. Sharing it means sophon no longer keeps a second copy that could disagree about what an absent list means, and the error now names the table's indexes so a rejected call says what was actually available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
dd197ad0ff
commit
4bb0effcb2
+4
-6
@@ -805,13 +805,11 @@ impl TryFrom<LsmWriteSpec> 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))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
----------
|
||||
|
||||
@@ -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
|
||||
|
||||
+10
-15
@@ -198,8 +198,8 @@ fn fmt_maintained(maintained: &Option<Vec<String>>) -> 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<String>) -> 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<Vec<String>>) -> 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<String, String>) -> Self {
|
||||
|
||||
@@ -449,22 +449,6 @@ pub struct IndexConfig {
|
||||
pub index_details: Option<String>,
|
||||
}
|
||||
|
||||
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 {
|
||||
|
||||
@@ -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();
|
||||
|
||||
+30
-29
@@ -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<I, S>(self, indexes: I) -> Self
|
||||
where
|
||||
I: IntoIterator<Item = S>,
|
||||
S: Into<String>,
|
||||
{
|
||||
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<Vec<String>>) -> 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<Option<Vec<String>>>) -> 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<dyn std::error::Error>> {
|
||||
/// 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!(
|
||||
|
||||
@@ -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<Vec<String>> {
|
||||
// `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
|
||||
// =============================================================================
|
||||
|
||||
Reference in New Issue
Block a user