feat: support taking rows by row ID - #60
Conversation
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lance-gatekeeper
Bot
added
the
K-changes
lance-gatekeeper
Bot
removed
the
K-changes
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The invalid-projection abort is fixed, and both take paths now preserve the FFI error contract through shared fallible projection validation. The direct _rowid lookup remains the right typed, order-preserving mechanism for two-phase retrieval.
lance-gatekeeper
Bot
added
the
K-approved
|
@jja725 PTAL |
| let snap = ds.snapshot(); | ||
| let projection = projection_from_columns(&snap, col_names.as_deref())?; | ||
|
|
||
| let batch = block_on(snap.take_rows(row_id_slice, projection))?; |
There was a problem hiding this comment.
Could we guard against missing or invalid row IDs here? Upstream can panic on these inputs, and since this is behind extern "C", that aborts the host process instead of returning an error. A subprocess regression test would help.
There was a problem hiding this comment.
Thanks for pointing this out @jja725
In the supported call path, the row IDs are produced by _rowid from a scanner created from the same dataset snapshot and are passed unchanged to lance_dataset_take_rows.
This is documented as a same-snapshot precondition and covered by the multi-fragment round-trip tests for both address-style and stable row IDs.
### What problem does this PR solve? Problem Summary: lance-format/lance-c#59 lance-format/lance-c#60
2 participants
Add
lance_dataset_take_rowsto read dataset rows using_rowidvalues.This is useful for two-phase vector search: first return
_rowidand_distance, perform global Top-K, and then fetch the requiredcolumns by row ID.