Add --with-row-id / --with-row-addr output columns (Lance) - #44
Merged
Merged
Conversation
Surface Lance row identity on the row-producing commands. `--with-row-id` appends the stable per-row `_rowid`; `--with-row-addr` appends the physical `_rowaddr` (`fragment_id << 32 | offset`). Both are UInt64, may be combined, and are appended after the projected columns (`_rowid` before `_rowaddr`), matching Lance's scanner convention. - Extend `ScanOptions` with a `row_ids: RowIds` field and thread a `RowIds` argument through `Dataset::take`; add a `Dataset::supports_row_id` capability hook (default false, Lance true) so the flags fail cleanly with "not supported by this format" on other backends. - Scan path uses `Scanner::with_row_id()` / `with_row_address()`. Take path requests the pseudo-columns via `ProjectionRequest::from_columns` (which preserves system columns) and reassembles the output in canonical `[projected…, _rowid, _rowaddr]` order so `take`/`tail`/`sample` match the streaming scan and the writer header exactly. - New `row_id` module owns the constants, projection cleaning (strip redundant system columns from `--columns`), the exclude-while-flagged error, and the output-schema extension. The pseudo-columns are always emitted when flagged regardless of `--columns`/`--exclude-columns`; explicitly excluding one errors with a hint to drop the flag. - Flags on `cat`, `head`, `tail`, `take`, `sample`. - Tests: cross-command consistency, non-contiguous `_rowid` after deletion, projection/exclude combos, exclusion error, both flags together, and the tail/sample/take paths. README + CHANGELOG document both identifiers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer follow-ups on the approved branch (no re-review required): 1. Correct the `_rowid` stability claims. Lance 4.0's `enable_stable_row_ids` defaults to FALSE, so for default-written datasets `_rowid` is address-based (equals `_rowaddr`) and is rewritten by compaction. Reword the README table, surrounding prose, the `--with-row-id` clap help, and the CHANGELOG to say "stable across deletions; stable across compaction only for datasets written with Lance's stable row ids enabled". 2. `cat` now checks row-id capability for every opened dataset in the open loop, not just the first, so a future mixed-format concatenation is covered. 3. New tests: nested projection + row ids through the `take` path (exercising `assemble_take_output`), `_rowid` stability across `--version`, and a CSV rendering assertion for the appended UInt64 columns. 4. Comment at the `ProjectionRequest::from_columns` take call site noting Lance internally `.unwrap()`s `project_preserve_system_columns`, and that arrs's prior projection validation (plus `RowIds`-only system names) is what keeps that infallible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Adds
--with-row-id/--with-row-addr(Lance only) tocat,head,tail,take, andsample: appends_rowid(stable across deletions; across compaction only with Lance stable row ids) and/or_rowaddr(fragment_id << 32 | offset) as trailing UInt64 columns, consistent across scan and take paths and correct across deletions. Excluding a requested pseudo-column errors with a hint; aDataset::supports_row_idcapability hook rejects unsupported formats cleanly. Includes the review commit (stability-doc corrections, added tests, per-input capability check incat).Rebased onto current main over #19/#11/#13/#22/#14: threaded
row_idsalongside #14's progress params in the mandated render→progress→rowids order across the five commands; stitched the interleaved diff/row-id test sections in tests/cli.rs; fixup gives #22's blob the newDataset::takeRowIds argument (defaulting off) and adapts cat's #14 glob-expanded inputs.Verified live:
--with-row-id --with-row-addroutput parses throughjq; progress stays TTY-gated. Gates: fmt/clippy clean, 329 tests / 0 failures.Closes #21
🤖 Generated with Claude Code