mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-12 14:52:26 +00:00
fix: read back the maintained-index selection a table was given (#4447)
`maintained_indexes` on a MemWAL table has three values: a named list, `[]` for none, and `null` for every index the table has, including ones created later. This PR makes every client read all three back as they were set. - **Rust remote client** (also used by the Python and Node SDKs) reads `null` as `None`. - **Java client** reads `null` as `null`, not as an empty list. - **Docs** for the setter, the field and the getter describe the three values the same way in Rust, Python, Node and Java. The generated JS docs are regenerated to match. ## Testing - `rust/lancedb`: 73 LSM tests pass, including one case per value. With the decoder typed as a list, the `null` case fails. - Java: two cases for `null` and `[]` on the read path, run by the Java CI job. - `cargo fmt`, `cargo clippy -D warnings` and `ruff format` are clean.
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
***
|
||||
|
||||
|
||||
+1
-1
@@ -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 |
|
||||
|
||||
|
||||
@@ -117,9 +117,13 @@ public class LanceDbTableLsm {
|
||||
/**
|
||||
* Read the {@link LsmWriteSpec} currently installed on this table.
|
||||
*
|
||||
* <p>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.
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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<LsmWriteSpec> getLsmWriteSpec() {
|
||||
JsonNode response = client.post(route("get_lsm_write_spec"), null);
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>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.
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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<String> 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.
|
||||
*
|
||||
* <p>The server always reports a concrete maintained-index list, so a null selection never
|
||||
* round-trips.
|
||||
* <p>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<String> maintainedIndexes = new ArrayList<String>();
|
||||
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<String> maintainedIndexes = null;
|
||||
if (indexesNode != null) {
|
||||
maintainedIndexes = new ArrayList<String>();
|
||||
for (JsonNode index : indexesNode) {
|
||||
if (!index.isTextual()) {
|
||||
throw new IllegalStateException(
|
||||
CONTEXT + ": maintained_indexes must hold index names, got " + index);
|
||||
}
|
||||
maintainedIndexes.add(index.asText());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<LsmWriteSpec> 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<LsmWriteSpec> 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}");
|
||||
|
||||
@@ -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<LsmWriteSpec | undefined>}
|
||||
*/
|
||||
abstract getLsmWriteSpec(): Promise<LsmWriteSpec | undefined>;
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
+4
-4
@@ -267,7 +267,7 @@ fn fmt_maintained(maintained: &Option<Vec<String>>) -> 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<Vec<String>>) -> Self {
|
||||
Self {
|
||||
|
||||
@@ -3252,8 +3252,9 @@ impl<S: HttpSend> BaseTable for RemoteTable<S> {
|
||||
#[derive(Deserialize)]
|
||||
struct LsmWriteSpecBody {
|
||||
sharding: Sharding,
|
||||
/// `null` selects every index the table has; `[]` selects none.
|
||||
#[serde(default)]
|
||||
maintained_indexes: Vec<String>,
|
||||
maintained_indexes: Option<Vec<String>>,
|
||||
#[serde(default)]
|
||||
writer_config_defaults: std::collections::HashMap<String, String>,
|
||||
}
|
||||
@@ -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<Vec<String>>,
|
||||
) {
|
||||
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| {
|
||||
|
||||
@@ -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
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user