JingsongLi commented on PR #771: URL: https://github.com/apache/paimon-rust/pull/771#issuecomment-5601283688
Thanks for restructuring the API. I took another pass focused specifically on scope and reviewability.\n\nThe PR is now 38 files and roughly +4,298/-383. My main concern is that it now includes the staged public read API that #755 explicitly left for a follow-up until there is a concrete consumer. This includes VectorRead / new_vector_read, public PkVectorIndexedSplit, TableRead to_arrow_indexed / indexed_read_type, and the C split-result handle plus count/free/read functions and supporting state/tests. Commit 048eb800 alone is about +2,153/-154.\n\nUnless this PR deliberately supersedes the scope recorded in #755, please either keep #771 on the original one-shot boundary (externally planned bucket splits in, Arrow rows out), or extract the generic staged VectorRead / TableRead API into a separate PR with its own consumer and contract discussion. That should remove roughly half of the review surface while preserving the core feature.\n\nSeveral useful but independent hardening cha nges should also be separate PRs: rejecting a deletion-vector position that crosses into the next source file; existing Lumina result-path changes for NaN ordering, sentinel-filled label buffers and tie-breaking; Lumina empty-index error precedence; and scalar batch-result arity checks in existing wrappers.\n\nFor #771 itself, I would keep the changes required by the new trust boundary: three-state FileRowSelection semantics, keeping unrestricted files off the filtered ANN path, direct planning from the supplied split, range/count validation, and the Lumina dense-filter memory budget.\n\nTests can be reduced without losing coverage. bucket_split_decoder_rejects_corrupt_bytes no longer exercises VectorRead because the Rust API accepts already-decoded splits. Projection/filter rejection is repeated across Rust integration, TableRead unit, and C integration tests; keep the semantic contract at Rust level, while C focuses on one Java-fixture happy path plus pointer/buffer/handle safety. \n\nThe earlier safety fixes look valuable. This is a request to keep this PR focused and move independent fixes into their own reviews, not to drop those fixes entirely. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
