feat(mem_wal): expose whether an index type can be maintained - #8095
Open
hamersaw wants to merge 6 commits into
Open
feat(mem_wal): expose whether an index type can be maintained#8095hamersaw wants to merge 6 commits into
hamersaw wants to merge 6 commits into
Conversation
`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) <noreply@anthropic.com>
… types 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Same content, fewer words. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
hamersaw
marked this pull request as ready for review
July 31, 2026 16:14
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The registry/predicate direction is sound, and the focused URL-classification and HNSW flush/reopen tests pass. I did not find a correctness or wire-compatibility blocker.
For the durable configuration contract, the safer follow-up is to apply the same registry inside InitializeMemWalBuilder::execute; external filtering should remain a convenience rather than the only guard against unsupported persisted specs.
| /// 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 { |
Contributor
There was a problem hiding this comment.
MemIndexConfig::detect_index_type is already public in v9.0.0 and the live base; deleting it in this patch makes downstream callers stop compiling. Keep it as a deprecated wrapper around MemIndexKind::from_type_url while steering new code to this predicate, so the registry cleanup does not require a flag-day source migration.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Exposes
is_maintainable_index_type(type_url) -> boolfromlance::dataset::mem_wal.Why
MemIndexConfig::detect_index_typeis the authority on what a MemWAL memtable can build (BTree, Inverted, Vector), but it is private. A caller assembling amaintained_indexesset therefore has no way to check an index before committing a spec.That matters because an unmaintainable index is not skipped —
mem_wal_writerpropagates the error when it opens a shard writer (api.rs), so every memtable claim on the table fails and the table goes unwritable. The failure surfaces far from theinitialize_mem_walcall that caused it.With the predicate public, callers can filter an inferred maintained set and reject an explicit one up front. LanceDB is the first consumer (it resolves "maintain everything the MemWAL supports" when a spec omits the list).
Notes
is_maintainable_index_typeis defined asdetect_index_type(..).is_ok()rather than a second list of type URLs, so the two cannot drift apart.Testing
An rstest over the three supported type URLs plus bitmap, label-list, and the absent-details case, asserting the predicate agrees with
detect_index_type.🤖 Generated with Claude Code