diff --git a/docs/src/js/classes/Table.md b/docs/src/js/classes/Table.md index ac16b8a81..3ac1dd031 100644 --- a/docs/src/js/classes/Table.md +++ b/docs/src/js/classes/Table.md @@ -662,10 +662,9 @@ Read the [LsmWriteSpec](../interfaces/LsmWriteSpec.md) currently installed on th Resolves to `undefined` when the MemWAL LSM write path is not enabled (no spec has been set, or it was removed with [Table#unsetLsmWriteSpec](Table.md#unsetlsmwritespec)). -The returned spec mirrors what was passed to -[Table#setLsmWriteSpec](Table.md#setlsmwritespec), except that `maintainedIndexes` always -reports the concrete list resolved when the spec was set — `undefined` -never round-trips. +The spec is the one installed, including its maintained-index selection: +an absent `maintainedIndexes` for every index the table has, an empty +array for none. #### Returns @@ -1107,8 +1106,8 @@ All variants require the table to have an unenforced primary key ([Table#setUnenforcedPrimaryKey](Table.md#setunenforcedprimarykey)); bucket sharding additionally requires it to be the single column being bucketed. -Omitting `maintainedIndexes` maintains every index on the table, resolved -here, failing if one cannot be maintained — name them to install anyway. +Omitting `maintainedIndexes` maintains every index the table has, +including ones created later, and skips a kind the MemWAL cannot maintain. Naming them pins an exact set, and a still-building index is rejected rather than quietly omitted. diff --git a/docs/src/js/interfaces/LsmWriteSpec.md b/docs/src/js/interfaces/LsmWriteSpec.md index f2ae91186..693542dbd 100644 --- a/docs/src/js/interfaces/LsmWriteSpec.md +++ b/docs/src/js/interfaces/LsmWriteSpec.md @@ -34,9 +34,8 @@ Bucket and identity variants: the sharding column. optional maintainedIndexes: string[]; ``` -Indexes the MemWAL keeps up to date. Omit to maintain every supported -index, resolved on install — a snapshot, so indexes created later are not -maintained. Pass `[]` for none. +Indexes the MemWAL keeps up to date. Omit for every index the table has, +including ones created later. Pass `[]` for none. *** diff --git a/java/README.md b/java/README.md index c46c8174b..bfee16268 100644 --- a/java/README.md +++ b/java/README.md @@ -67,7 +67,7 @@ reader usually expects: | Value | Meaning | | --- | --- | -| unset (null) | Maintain **every** index the MemWAL can, resolved on install | +| unset (null) | Maintain **every** index the table has, including ones created later | | `Collections.emptyList()` | Maintain **none** | | `Arrays.asList("id_idx")` | Maintain exactly those | diff --git a/java/lancedb-core/src/main/java/com/lancedb/LanceDbTableLsm.java b/java/lancedb-core/src/main/java/com/lancedb/LanceDbTableLsm.java index 23b18199e..bb1467ecd 100644 --- a/java/lancedb-core/src/main/java/com/lancedb/LanceDbTableLsm.java +++ b/java/lancedb-core/src/main/java/com/lancedb/LanceDbTableLsm.java @@ -117,9 +117,13 @@ public class LanceDbTableLsm { /** * Read the {@link LsmWriteSpec} currently installed on this table. * - *

Empty when the LSM write path is not enabled. The returned spec mirrors what was installed, - * except that {@link LsmWriteSpec#maintainedIndexes()} always reports the concrete list resolved - * when the spec was set — a null selection never round-trips. + *

Empty when the LSM write path is not enabled at all — no spec has been set, or one was + * removed with {@link #unsetLsmWriteSpec()}. That is a different answer from a present spec + * whose {@link LsmWriteSpec#maintainedIndexes()} is null, which selects indexes automatically. + * + *

The spec read back is the one that was installed, selection included: null maintains every + * supported index the table has now or gains later, an empty list maintains none, and a + * non-empty list maintains exactly those. All three round-trip. */ public Optional getLsmWriteSpec() { JsonNode response = client.post(route("get_lsm_write_spec"), null); diff --git a/java/lancedb-core/src/main/java/com/lancedb/LsmWriteSpec.java b/java/lancedb-core/src/main/java/com/lancedb/LsmWriteSpec.java index da0966910..c02298c45 100644 --- a/java/lancedb-core/src/main/java/com/lancedb/LsmWriteSpec.java +++ b/java/lancedb-core/src/main/java/com/lancedb/LsmWriteSpec.java @@ -36,6 +36,8 @@ import java.util.Map; */ public class LsmWriteSpec { + private static final String CONTEXT = "get_lsm_write_spec response"; + /** How writes are routed to MemWAL shards. */ public enum Sharding { /** Hash-bucket writes by a scalar column. */ @@ -124,9 +126,8 @@ public class LsmWriteSpec { /** * Set the indexes the MemWAL keeps up to date as rows are appended. * - *

Pass {@code null} — the default for a fresh spec — to maintain every index the MemWAL can, - * resolved when the spec is installed. That is a snapshot: indexes created later are not - * maintained until the spec is unset and set again. Pass an empty list to maintain none. + *

Pass {@code null} — the default for a fresh spec — to maintain every index the table has, + * including ones created later. Pass an empty list to maintain none. * *

Note that {@code null} and the empty list mean opposite things here. */ @@ -176,8 +177,10 @@ public class LsmWriteSpec { } /** - * The indexes the MemWAL maintains, or null to have the server resolve every maintainable index - * on install. An empty list means none. + * Which indexes the MemWAL maintains. + * + *

Null selects automatically: every supported index the table has now or gains later. An empty + * list maintains none. A non-empty list maintains exactly those. */ public List maintainedIndexes() { return maintainedIndexes == null ? null : Collections.unmodifiableList(maintainedIndexes); @@ -210,8 +213,8 @@ public class LsmWriteSpec { /** * Rebuild a spec from a {@code get_lsm_write_spec} response body. * - *

The server always reports a concrete maintained-index list, so a null selection never - * round-trips. + *

A null {@code maintained_indexes} selects every index the table has; an empty list selects + * none. */ static LsmWriteSpec fromJson(JsonNode node) { JsonNode shardingNode = node.get("sharding"); @@ -224,10 +227,19 @@ public class LsmWriteSpec { Integer numBuckets = shardingNode.hasNonNull("num_buckets") ? shardingNode.get("num_buckets").asInt() : null; - List maintainedIndexes = new ArrayList(); - JsonNode indexesNode = node.get("maintained_indexes"); - if (indexesNode != null && indexesNode.isArray()) { + // Absent or null is the automatic selection; anything else has to be an + // array of names. Reading a string or an object as automatic would turn a + // malformed response into "maintain every index", which is a policy, not a + // parse failure. + JsonNode indexesNode = JsonFields.optionalArray(node, "maintained_indexes", CONTEXT); + List maintainedIndexes = null; + if (indexesNode != null) { + maintainedIndexes = new ArrayList(); for (JsonNode index : indexesNode) { + if (!index.isTextual()) { + throw new IllegalStateException( + CONTEXT + ": maintained_indexes must hold index names, got " + index); + } maintainedIndexes.add(index.asText()); } } diff --git a/java/lancedb-core/src/test/java/com/lancedb/LanceDbTableLsmTest.java b/java/lancedb-core/src/test/java/com/lancedb/LanceDbTableLsmTest.java index e84fa5421..66ce140b6 100644 --- a/java/lancedb-core/src/test/java/com/lancedb/LanceDbTableLsmTest.java +++ b/java/lancedb-core/src/test/java/com/lancedb/LanceDbTableLsmTest.java @@ -215,6 +215,61 @@ public class LanceDbTableLsmTest { assertEquals("true", spec.get().writerConfigDefaults().get("durable_write")); } + /** A null selection, every index the table has, reads back as null rather than an empty list. */ + @Test + public void testGetLsmWriteSpecRoundTripsEveryIndexSelection() { + enqueue( + "get_lsm_write_spec", + 200, + "{\"lsm_write_spec\":{\"sharding\":{\"mode\":\"unsharded\"}," + + "\"maintained_indexes\":null,\"writer_config_defaults\":{}}}"); + + Optional spec = lsm.getLsmWriteSpec(); + + assertTrue(spec.isPresent()); + assertNull(spec.get().maintainedIndexes()); + } + + @Test + public void testGetLsmWriteSpecRoundTripsNoIndexSelection() { + enqueue( + "get_lsm_write_spec", + 200, + "{\"lsm_write_spec\":{\"sharding\":{\"mode\":\"unsharded\"}," + + "\"maintained_indexes\":[],\"writer_config_defaults\":{}}}"); + + Optional spec = lsm.getLsmWriteSpec(); + + assertTrue(spec.isPresent()); + assertEquals(Collections.emptyList(), spec.get().maintainedIndexes()); + } + + /** + * Null is the automatic selection, so anything that is neither null nor an array has to be + * rejected: reading a string or an object as null would turn a malformed response into a policy. + */ + @Test + public void testGetLsmWriteSpecRejectsANonArrayMaintainedIndexes() { + enqueue( + "get_lsm_write_spec", + 200, + "{\"lsm_write_spec\":{\"sharding\":{\"mode\":\"unsharded\"}," + + "\"maintained_indexes\":\"id_idx\",\"writer_config_defaults\":{}}}"); + + assertThrows(IllegalStateException.class, () -> lsm.getLsmWriteSpec()); + } + + @Test + public void testGetLsmWriteSpecRejectsNonStringIndexNames() { + enqueue( + "get_lsm_write_spec", + 200, + "{\"lsm_write_spec\":{\"sharding\":{\"mode\":\"unsharded\"}," + + "\"maintained_indexes\":[7],\"writer_config_defaults\":{}}}"); + + assertThrows(IllegalStateException.class, () -> lsm.getLsmWriteSpec()); + } + @Test public void testGetLsmWriteSpecAbsent() { enqueue("get_lsm_write_spec", 200, "{\"lsm_write_spec\":null}"); diff --git a/nodejs/lancedb/table.ts b/nodejs/lancedb/table.ts index 6bdc69418..eae718168 100644 --- a/nodejs/lancedb/table.ts +++ b/nodejs/lancedb/table.ts @@ -225,9 +225,8 @@ export interface LsmWriteSpec { /** Bucket variant: the number of buckets, in `[1, 1024]`. */ numBuckets?: number; /** - * Indexes the MemWAL keeps up to date. Omit to maintain every supported - * index, resolved on install — a snapshot, so indexes created later are not - * maintained. Pass `[]` for none. + * Indexes the MemWAL keeps up to date. Omit for every index the table has, + * including ones created later. Pass `[]` for none. */ maintainedIndexes?: string[]; /** Default `ShardWriter` configuration recorded in the MemWAL index. */ @@ -766,8 +765,8 @@ export abstract class Table { * ({@link Table#setUnenforcedPrimaryKey}); bucket sharding additionally * requires it to be the single column being bucketed. * - * Omitting `maintainedIndexes` maintains every index on the table, resolved - * here, failing if one cannot be maintained — name them to install anyway. + * Omitting `maintainedIndexes` maintains every index the table has, + * including ones created later, and skips a kind the MemWAL cannot maintain. * Naming them pins an exact set, and a still-building index is rejected * rather than quietly omitted. * @param {LsmWriteSpec} spec The sharding spec to install. @@ -797,10 +796,9 @@ export abstract class Table { * * Resolves to `undefined` when the MemWAL LSM write path is not enabled (no * spec has been set, or it was removed with {@link Table#unsetLsmWriteSpec}). - * The returned spec mirrors what was passed to - * {@link Table#setLsmWriteSpec}, except that `maintainedIndexes` always - * reports the concrete list resolved when the spec was set — `undefined` - * never round-trips. + * The spec is the one installed, including its maintained-index selection: + * an absent `maintainedIndexes` for every index the table has, an empty + * array for none. * @returns {Promise} */ abstract getLsmWriteSpec(): Promise; diff --git a/python/python/lancedb/table.py b/python/python/lancedb/table.py index 11ca7de84..8824834c2 100644 --- a/python/python/lancedb/table.py +++ b/python/python/lancedb/table.py @@ -5445,11 +5445,16 @@ class AsyncTable: async def get_lsm_write_spec(self) -> Optional["LsmWriteSpec"]: """Read the LsmWriteSpec currently installed on this table. - Returns ``None`` when the MemWAL LSM write path is not enabled (no - spec has been set, or it was removed with `unset_lsm_write_spec`). - The returned spec mirrors what was passed to `set_lsm_write_spec`, - except that ``maintained_indexes`` always reports the concrete list - resolved when the spec was set — ``None`` never round-trips. + Returns ``None`` when the LSM write path is not enabled at all — no + spec has been set, or one was removed with `unset_lsm_write_spec`. + That is a different answer from a spec whose ``maintained_indexes`` + is ``None``, which is an installed spec selecting indexes + automatically. + + The spec read back is the one that was installed, selection + included: ``None`` maintains every supported index the table has now + or gains later, ``[]`` maintains none, and a non-empty list maintains + exactly those. All three round-trip. """ return await self._inner.get_lsm_write_spec() diff --git a/python/src/table.rs b/python/src/table.rs index 16f62bf8d..bb3d0c3c5 100644 --- a/python/src/table.rs +++ b/python/src/table.rs @@ -267,7 +267,7 @@ fn fmt_maintained(maintained: &Option>) -> String { /// Constructed via the `bucket(...)`, `identity(...)`, or `unsharded()` /// classmethods, then optionally chain `with_maintained_indexes(...)` and /// `with_writer_config_defaults(...)`. A fresh spec maintains every index the -/// MemWAL supports, resolved on install. +/// table has, including ones created later. #[pyclass(module = "lancedb._lancedb", from_py_object)] #[derive(Clone, Debug)] pub struct LsmWriteSpec { @@ -307,9 +307,9 @@ impl LsmWriteSpec { } } - /// Set which indexes the MemWAL maintains. `None` (the default) - /// resolves every supported index on install; a list is verbatim, - /// and an empty list maintains nothing. + /// Set which indexes the MemWAL maintains. `None` (the default) is + /// every index the table has, including ones created later; a list is + /// verbatim, and an empty list maintains nothing. #[pyo3(signature = (indexes))] pub fn with_maintained_indexes(&self, indexes: Option>) -> Self { Self { diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index cd9e9ab64..35114cded 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -3252,8 +3252,9 @@ impl BaseTable for RemoteTable { #[derive(Deserialize)] struct LsmWriteSpecBody { sharding: Sharding, + /// `null` selects every index the table has; `[]` selects none. #[serde(default)] - maintained_indexes: Vec, + maintained_indexes: Option>, #[serde(default)] writer_config_defaults: std::collections::HashMap, } @@ -9584,6 +9585,43 @@ mod tests { } } + /// Every selection reads back as the server reported it: every index + /// (`null`), none (`[]`), and a named list. + #[rstest::rstest] + #[case::every_index(serde_json::Value::Null, None)] + #[case::no_index(serde_json::json!([]), Some(vec![]))] + #[case::named(serde_json::json!(["id_idx"]), Some(vec!["id_idx".to_string()]))] + #[tokio::test] + async fn test_get_lsm_write_spec_round_trips_the_selection( + #[case] reported: serde_json::Value, + #[case] expected: Option>, + ) { + let table = Table::new_with_handler("my_table", move |_| { + let response = serde_json::json!({ + "lsm_write_spec": { + "sharding": { "mode": "unsharded" }, + "maintained_indexes": reported, + "writer_config_defaults": {}, + } + }); + http::Response::builder() + .status(200) + .body(response.to_string()) + .unwrap() + }); + + let spec = table + .get_lsm_write_spec() + .await + .unwrap() + .expect("a spec should be reported"); + assert_eq!( + spec.maintained_indexes().map(<[String]>::to_vec), + expected, + "the selection the server reported must survive the read" + ); + } + #[tokio::test] async fn test_get_lsm_write_spec_absent() { let table = Table::new_with_handler("my_table", |request| { diff --git a/rust/lancedb/src/table.rs b/rust/lancedb/src/table.rs index cf7fcc145..5179440df 100644 --- a/rust/lancedb/src/table.rs +++ b/rust/lancedb/src/table.rs @@ -2034,12 +2034,16 @@ impl Table { /// Read the [`LsmWriteSpec`] currently installed on this table. /// - /// Returns `Ok(None)` when the MemWAL LSM write path is not enabled (no - /// spec has been set, or it was removed with [`Table::unset_lsm_write_spec`]). - /// The returned spec mirrors what was passed to - /// [`Table::set_lsm_write_spec`], except that - /// [`LsmWriteSpec::maintained_indexes`] always reports the concrete list - /// resolved when the spec was set — `None` never round-trips. + /// `Ok(None)` means the LSM write path is not enabled at all — no spec has + /// been set, or one was removed with [`Table::unset_lsm_write_spec`]. That + /// is a different answer from a spec whose + /// [`LsmWriteSpec::maintained_indexes`] is `None`, which is an installed + /// spec selecting indexes automatically. + /// + /// The spec read back is the one that was installed, selection included: + /// `None` maintains every supported index the table has now or gains + /// later, `[]` maintains none, and a non-empty list maintains exactly + /// those. All three round-trip. /// /// # Example ///