refactor: distributed-only scalar indexing with complete index metadata - #73
Merged
everySympathy merged 1 commit intoSep 20, 2026
Merged
Conversation
everySympathy
force-pushed
the
feat/segment-only-indexing
branch
from
September 20, 2026 07:05
8c7df3d to
223b477
Compare
everySympathy
force-pushed
the
feat/segment-only-indexing
branch
2 times, most recently
from
September 20, 2026 09:14
0e0fe96 to
a98f2b8
Compare
Make the distributed segment-index workflow (create_index_uncommitted per worker + commit_existing_index_segments at the coordinator) the only code path for create_scalar_index. - Remove the public 'segmented' parameter (it leaked an implementation mechanism into the API; passing it now fails loudly with a TypeError). - Remove the legacy partitioned-and-merged INVERTED path whose manual CreateIndex commit cannot record index_details — the source of the 'Unknown' index type metadata (references daft-engine#69). - Remove all single-node fallbacks: types without a distributed path raise ValueError with a pointer to pylance instead of silently degrading to single-node execution. - Unlock distributed builds for ZONEMAP, NGRAM, LABEL_LIST and BLOOMFILTER (pylance 11 builds all of them as segments; RTREE needs GeoArrow columns and stays excluded until testable). - replace now defaults to True (matching pylance) and is atomic: the segment commit retires the existing index's overlapped segments in the same transaction as the new ones. replace=False rejects an existing name. Stale segments that no longer overlap any live fragment cannot be retired by the swap, but normal operations never produce them and any that appear are healed by optimize_indices (verified). - Commit worker-built segments as-is: no merge step and no BITMAP shard ids. Multi-segment INVERTED/BITMAP indexes are fully functional (verified via scan statistics: split segments are loaded and pruned at query time, rows_scanned=1, with BM25 scores identical to a merged build), so compaction is left to optimize_indices. This also removes the single-node merge bottleneck from the build pipeline.
everySympathy
force-pushed
the
feat/segment-only-indexing
branch
from
September 20, 2026 10:09
a98f2b8 to
4afb71e
Compare
everySympathy
pushed a commit
to everySympathy/daft-lance
that referenced
this pull request
Sep 20, 2026
create_scalar_index(..., fragment_ids=[...]) schedules only the listed fragments: - Partial first builds commit an index that covers just those fragments. - Re-invoking with the same index name, column and type backfills: fragments already covered by committed segments are skipped, the remainder is built with create_index_uncommitted(replace=True) (the name already exists in the workers' pinned snapshot) and appended atomically via commit_existing_index_segments — non-overlapping segments are appended, so untouched committed segments are preserved. Verified query-equivalent to a one-shot full build (INVERTED, BTREE, BITMAP). - Deterministic validation: empty list and unknown fragment IDs raise ValueError (available IDs listed); duplicates are de-duplicated with a warning. - An existing index on a different column or of a different type is rejected (Lance's append-style commit accepts mixed-type segments, which would corrupt index metadata). - A fully covered request is a no-op (version not bumped), and a same-name call without fragment_ids takes the default replace=True atomic rebuild path from daft-engine#73. - A pre-commit manifest guard refuses segments whose fragments were rewritten by a concurrent compaction — Lance itself would accept the commit and the index would reference dead fragment IDs forever.
This was referenced Sep 20, 2026
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
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.
References #69. Part of #27.
What
The distributed segment-index workflow (each Daft worker builds an independent segment with
create_index_uncommitted, the coordinator commits atomically withcommit_existing_index_segments) is now the only code path increate_scalar_index.This is a breaking change, intentionally taken while the project is 0.x:
segmentedparameter is removed. It leaked an internal implementation mechanism into the API contract. Passing it now fails immediately with aTypeErrorexplaining the removal (not a confusing deep worker error via**kwargs).CreateIndexcommit cannot recordindex_details, which is exactly why indexes created through it report their type asUnknown(create_scalar_index commits indexes with empty index_details, so Lance reports their type as Unknown #69). With this change no code path in daft-lance produces empty index metadata anymore; existing broken indexes can be repaired by rebuilding withreplace=True.ValueErrorwith a pointer to pylance (lance.dataset(uri).create_scalar_index(...)) instead of silently degrading to single-node execution. Silent degradation is where semantics go to rot — the same call used to distribute or not depending on type and flags, with differentreplacedefaults and different metadata outcomes. Single-node indexing is already pylance's own job; daft-lance's value is the distributed path.replacenow defaults toTrue, matching pylance's own default, and replacement is atomic:commit_existing_index_segmentsretires the existing index's overlapped segments in the same CreateIndex transaction as the incoming ones, so a full-coverage rebuild swaps the old index in a single transaction (verified: the rebuild advances the dataset version exactly once and leaves exactly one segment — including the case where a fully-deleted fragment left stale ids inside a mixed segment's coverage).replace=Falserejects an existing name. Segments that no longer overlap any live fragment cannot be retired by the swap, but normal operations never produce them (compaction rewrites index coverage; delete retires fully-dead segments), and any that appear are healed byoptimize_indices(verified experimentally) — so no drop-based fallback is kept.Newly distributed types
pylance 11 builds every scalar index type as segments (
_is_segment_native_scalar_index_type), so this PR also unlocks distributed builds forZONEMAP,NGRAM,LABEL_LIST,BLOOMFILTER— verified end-to-end (multi-segment builds viafragment_group_size=1, full metadata, query-correct results).RTREEneeds GeoArrow extension columns and stays excluded until the test suite can create those.indices_loaded=N), prune torows_scanned=1, and BM25 scores are identical to a merged single-segment build. Multi-segment indexes are fully functional, so the forced pre-commit merge (a single-node bottleneck in the build pipeline) is dropped; segment compaction isoptimize_indices's job.Why now
The
segmentedflag was introduced when onlyBTREEhad a segment path and the pylance floor did not guarantee the segment APIs. Both reasons are gone: the dependency floor is nowpylance>=11, all scalar types are segment-native, and the flag's default anchored the legacy path that produces the broken metadata behind #69. Keeping two paths means every future parameter must be validated, tested, and documented per path (the single-node fallbacks would silently drop parameters such as the upcomingfragment_idssupport). One path, one contract.Tests
tests/io/lance/test_lance_distributed_index_types.py: parametrized coverage for the four unlocked types (metadata completeness —index_typeis neverUnknown— query correctness, replace semantics, multi-segment builds) plus a guard test keepingDISTRIBUTED_INDEX_TYPESin sync with pylance's segment-native set.Migration notes (README updated too)
segmented=from calls — it was a no-op choice between two internals; the better one is now the only one.replacesemantics: default is nowTrue; replacement is atomic via segment overlap.RTREE), call pylance directly.replace=Truerecords full metadata.