feat(python): add RowIdSequence for constructing row id metadata - #8356
Conversation
There was no public way to build a stable row id sequence from Python. `RowIdMeta` could only be created from `from_json`/`from_dict`, so callers assembling a transaction externally had to hand-encode the internal `RowIdSequence` protobuf to give a new fragment pre-existing row ids. Adds `lance.fragment.RowIdSequence`, which builds a sequence from a range, an Arrow integer array, or any iterable of ints, and converts to and from the inline metadata carried on `FragmentMetadata.row_id_meta`. Duplicate row ids are rejected. `U64Segment` assumes uniqueness: a repeated value makes `SegmentStats::n_holes` underflow, which panics in debug builds and in release silently encodes a shorter sequence with a spurious hole. The new `RowIdSequence::try_from_iter` validates before encoding, leaving the existing infallible `From` conversions unchanged for internal callers. Closes #8353 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Detect `range` inputs with pyo3's `PyRange` instead of importing `builtins` and calling `is_instance`. Bounds past `isize` fall through to the element-wise path, which still covers the whole uint64 row id domain. Iterating a sequence no longer materializes every row id. A `#[pyclass]` cannot hold the borrowing iterator `RowIdSequence::iter` returns, and stepping through `get` is quadratic because segment lengths are recomputed per call, so the iterator buffers fixed-size slices instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The typed wrapper addresses the API gap, but this revision still leaves two independent acceptance contracts unresolved: caller-supplied metadata must remain inside the dataset live-ID/allocator invariant, and sequence traversal must stay linear for all supported encodings.
A viable revision can keep typed construction while validating preserved IDs against the candidate live dataset on every commit/rebase (or carry them through an Update-scoped API with that context), and make iteration retain forward traversal state with gapped and multi-segment coverage.
| } | ||
|
|
||
| /// Encode the sequence as row id metadata stored inline in the manifest. | ||
| fn to_inline_metadata(&self) -> PyRowIdMeta { |
There was a problem hiding this comment.
This emits caller-chosen IDs into fragment metadata without any commit-time check that they are existing IDs being relocated. assign_row_ids accepts a complete supplied sequence and leaves next_row_id unchanged, so a fresh supplied ID is reused by the next ordinary append; retained IDs or two replacement fragments can similarly create duplicate live IDs. The result violates the durable global-uniqueness contract and breaks row-ID lookup. Construction-time validation cannot establish dataset liveness: validate the candidate live state during commit and every rebase, or carry preserved IDs through an Update-specific field so allocation and validation are atomic.
Reproducer run on this head
import tempfile
import lance
import pyarrow as pa
from lance import LanceDataset, LanceOperation
from lance.fragment import RowIdSequence, write_fragments
uri = tempfile.mkdtemp() + "/collision.lance"
ds = lance.write_dataset(
pa.table({"id": [1, 2]}), uri, enable_stable_row_ids=True
)
(new_fragment,) = write_fragments(pa.table({"id": [3]}), uri)
new_fragment.row_id_meta = RowIdSequence([2]).to_inline_metadata()
ds = LanceDataset.commit(
uri,
LanceOperation.Update(
removed_fragment_ids=[], updated_fragments=[],
new_fragments=[new_fragment], fields_modified=[],
),
read_version=ds.version,
)
ds = lance.write_dataset(pa.table({"id": [4]}), uri, mode="append")
print(ds.to_table(columns=["id"], with_row_id=True)["_rowid"].to_pylist())
ds._take_rows([2])The IDs printed as [0, 1, 2, 2]; _take_rows then failed with row id index corrupt: stable row id 2 is live in multiple fragments.
There was a problem hiding this comment.
It is up to the caller to guarantee uniqueness. For performance reasons, we don't re-check uniqueness of the row ids on every commit.
Buffering slices of the sequence made iteration quadratic for the gapped `RangeWithHoles` and `RangeWithBitmap` encodings: `slice` takes an absolute offset and skips its prefix one element at a time, so each refill walked a longer prefix. Materialize the row ids once instead, and record why the buffered form does not work so it is not reintroduced. Iterating 400k gapped row ids goes from 4x-per-2x growth to linear, and back in line with a contiguous sequence of the same length. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The iterator is linear again on this revision, and the focused row-ID suite passes. One acceptance contract remains: the public API says dataset-wide duplicates are rejected, while construction validates only the supplied sequence and commit trusts the caller without advancing the stable-ID allocator.
A caller-owned contract is viable for this advanced commit surface, but it must be explicit where users construct the metadata. Document the provenance, transaction, global-uniqueness, and allocator obligations before exposing it as safe typed metadata.
The docstring said row ids must be unique within a dataset and that duplicates are rejected, which read as though construction enforced global uniqueness. It only rejects duplicates within the sequence. Spell out what the caller owns: supply only existing row ids being relocated by the same transaction, and not unused ones, since a sequence covering all of a fragment's rows leaves `next_row_id` untouched and a later append will reissue the same id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The public API now states the caller-owned uniqueness and allocator contract, while the implementation keeps sequence-local validation and linear iteration. This resolves the remaining durable row-ID risk without adding a commit-time scan.
There was no public way to build a stable row id sequence from Python.
RowIdMetacould only be constructed viafrom_json/from_dict, so a caller assembling a transaction externally — a distributed engine computing updates on workers and committing from a driver — had to hand-encode the internalRowIdSequenceprotobuf to tell Lance that rows in a new fragment already have row ids. Without that, the commit path allocates fresh ids and the rewritten rows silently lose their identity.This PR adds
lance.fragment.RowIdSequence. It builds a sequence from arange, an Arrow integer array, aChunkedArray, or any iterable of ints, and converts to and from the inline metadata carried onFragmentMetadata.row_id_meta. The commit path already honors a sequence supplied this way, so no changes were needed there.Duplicate row ids are now rejected rather than silently mis-encoded.
U64Segmentassumes uniqueness: a repeated value makesSegmentStats::n_holescomputetotal_slots - count, which underflows. That panics in debug builds, and in release it wraps, steers encoding selection towardRangeWithBitmap, and yields a shorter sequence with a spurious hole —[1, 1, 2]encodes to[1]. The newRowIdSequence::try_from_itervalidates before encoding; the existing infallibleFromconversions are unchanged, so internal callers on hot paths do not pay for the check.Closes #8353. Addresses the narrowed request in #8317.
Example
Preserving a row's identity in an update assembled outside of Lance — delete the old row, write a replacement fragment, and attach the original row id to it:
Reading a fragment's existing sequence back:
Accepted inputs are any of the eight Arrow integer types, a
ChunkedArray, arange(a step-of-one range is stored compactly without materializing its values), or any iterable of ints. Nulls, non-integer arrays, negative values, and duplicates each raise a specific error.Not included
Validation is within-sequence only. Checking that supplied ids are unique across fragments and currently live in the dataset — also requested in #8317 — can only happen at commit time and is left out of this PR.
Documentation for the distributed-write guide will follow in a stacked PR based on this branch.