feat: add blob take and BlobFile read APIs - #82
Conversation
182df3b to
96656bc
Compare
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The opaque LanceBlobFile wrapper closes the C-access gap documented in #76 while preserving upstream random and range reads instead of forcing large payloads into scan batches. Input validation happens before caller output is touched, handle ownership is independent of the dataset, and the multi-fragment coverage exercises inline, packed, dedicated, empty, null, bounds, and cursor behavior through Rust, C, and C++.
This is stacked on #81, whose current head exactly matches this branch’s first two commits; land #81 first, then this PR.
lance-gatekeeper
Bot
added
the
K-approved
jja725
left a comment
jja725
left a comment
There was a problem hiding this comment.
can you resolve the conflict
96656bc to
6d9bb30
Compare
lance-gatekeeper
Bot
removed
the
K-approved
6d9bb30 to
fc13965
Compare
I’ve updated it |
LuciferYang
left a comment
LuciferYang
left a comment
There was a problem hiding this comment.
Two small non-blocking notes inline, both LOW. The blob API looks solid: validation is thorough, the C++ RAII and handle lifetimes are correct, and the tests cover the tricky cases well (sentinel-checked out-untouched on every error path, u64 overflow, out-of-bounds, handles outliving the dataset).
|
@MisterRaindrop seems like it conflict with #81 again, could you rebase again |
lance_dataset_take_blobs / _by_indices fill a caller array of LanceBlobFile* (NULL for null values, untouched on error). The handle has size, read, read_up_to, read_range, seek, tell and close, and wraps upstream BlobFile only, so it outlives the dataset.
Reuses the fixture from the scanner blob handling PR, with a stable row id switch.
`lance_blob_file_read` and `read_up_to` built the destination slice at the caller-declared length, so the safety precondition covered bytes the read never touches. Size both to `bytes.len()` instead. Also note in `adopt_blobs` that C++ leak-freedom depends on the C side filling `out` all-or-nothing.
b0c21e5 to
78596e7
Compare
I’ve updated it |
3 participants
Builds on #81; the first two commits are that PR.
Adds the row-addressed blob API from #76:
lance_dataset_take_blobs(by_rowid) andlance_dataset_take_blobs_by_indices(by row offset) fill a caller array ofLanceBlobFile*, NULL for a null value; the handle hassize,read,read_up_to,read_range,seek,tellandclose. Reads copy into caller buffers and return 0 / -1 with the thread-local error set.The handle wraps upstream
BlobFileonly, so it outlives the dataset. Take is all-or-nothing (outuntouched on error),readrejects a buffer smaller than the remaining bytes instead of truncating, and an unknown column is an invalid argument rather thanLANCE_ERR_INTERNAL.lance.hppgets an RAIIBlobFileandDataset::take_blobs*()returningstd::vector<std::optional<BlobFile>>.Tests: sixteen
test_blob_*cases (both entry points over every storage layout, with and without stable row ids, cursor semantics, bounds and overflow, handles outliving the dataset, NULL and invalid arguments withoutuntouched) plus the C and C++ smoke programs.Open:
readgoes from the cursor to the end like the Rust and Javaread(), andseekis not bounds-checked, like upstream.take_blobs_by_addressesandkind/uriaccessors are not included.