Skip to content

refactor: distributed-only scalar indexing with complete index metadata - #73

Merged
everySympathy merged 1 commit into
daft-engine:mainfrom
everySympathy:feat/segment-only-indexing
Sep 20, 2026
Merged

everySympathy merged 1 commit into
daft-engine:mainfrom
everySympathy:feat/segment-only-indexing

Conversation

@everySympathy

@everySympathy everySympathy commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

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 with commit_existing_index_segments) is now the only code path in create_scalar_index.

This is a breaking change, intentionally taken while the project is 0.x:

  1. The public segmented parameter is removed. It leaked an internal implementation mechanism into the API contract. Passing it now fails immediately with a TypeError explaining the removal (not a confusing deep worker error via **kwargs).
  2. The legacy partitioned-and-merged INVERTED path is removed. Its manual CreateIndex commit cannot record index_details, which is exactly why indexes created through it report their type as Unknown (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 with replace=True.
  3. All single-node fallbacks are removed. Types without a distributed path raise ValueError with 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 different replace defaults and different metadata outcomes. Single-node indexing is already pylance's own job; daft-lance's value is the distributed path.
  4. replace now defaults to True, matching pylance's own default, and replacement is atomic: commit_existing_index_segments retires 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=False rejects 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 by optimize_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 for ZONEMAP, NGRAM, LABEL_LIST, BLOOMFILTER — verified end-to-end (multi-segment builds via fragment_group_size=1, full metadata, query-correct results). RTREE needs GeoArrow extension columns and stays excluded until the test suite can create those.

  1. Worker-built segments commit as-is — no merge step, no BITMAP shard ids. Verified via scan statistics: with a multi-segment (unmerged) INVERTED or BITMAP index, queries load every segment (indices_loaded=N), prune to rows_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 is optimize_indices's job.

Why now

The segmented flag was introduced when only BTREE had a segment path and the pylance floor did not guarantee the segment APIs. Both reasons are gone: the dependency floor is now pylance>=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 upcoming fragment_ids support). One path, one contract.

Tests

  • Existing scalar-index tests updated to the new contract (41 pass in the scalar-index suite); legacy-path and fallback tests rewritten to assert loud failures.
  • Replace semantics: the driver never drops (fake-level assertion) and the real-dataset assertions are "version advances exactly once, exactly one segment survives" — including the stale-coverage case produced by deleting an entire fragment.
  • New tests/io/lance/test_lance_distributed_index_types.py: parametrized coverage for the four unlocked types (metadata completeness — index_type is never Unknown — query correctness, replace semantics, multi-segment builds) plus a guard test keeping DISTRIBUTED_INDEX_TYPES in sync with pylance's segment-native set.
  • Full suite: 392 passed on pylance 11.0.0; mypy strictly below the previous baseline (the removed legacy/merge code carried pre-existing errors).

Migration notes (README updated too)

  • Remove segmented= from calls — it was a no-op choice between two internals; the better one is now the only one.
  • replace semantics: default is now True; replacement is atomic via segment overlap.
  • For single-node indexing or unsupported types (e.g. RTREE), call pylance directly.
  • Indexes created by ≤0.5.0 through the default INVERTED path carry empty index metadata; rebuilding them with replace=True records full metadata.

@everySympathy
everySympathy force-pushed the feat/segment-only-indexing branch from 8c7df3d to 223b477 Compare September 20, 2026 07:05
@everySympathy everySympathy changed the title refactor: distributed-only scalar indexing, aligned with lance-ray refactor: distributed-only scalar indexing with complete index metadata Sep 20, 2026
@everySympathy
everySympathy force-pushed the feat/segment-only-indexing branch 2 times, most recently from 0e0fe96 to a98f2b8 Compare September 20, 2026 09:14
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
everySympathy force-pushed the feat/segment-only-indexing branch from a98f2b8 to 4afb71e Compare September 20, 2026 10:09
@everySympathy
everySympathy merged commit f9eff15 into daft-engine:main Sep 20, 2026
5 checks passed
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant