feat(lance): Remove extra buffering in Lance writer #17916
Conversation
| root = VectorSchemaRoot.create(getArrowSchema(), allocator); | ||
| } | ||
| // Finalize the arrow writer (sets row count on VectorSchemaRoot) | ||
| arrowWriter.finish(); |
There was a problem hiding this comment.
nit: instead of finish() should the interface method be renamed to something more clear like setRowCount() or is there other functionality outside of setting rowCount as well that finish() handles?
There was a problem hiding this comment.
It will do some operation on the fields as well, I just made it match the implementation name but I think something like closeBatch or finishBatch may be more telling
There was a problem hiding this comment.
makes sense either of those names sounds good to me.
the-other-tim-brown
force-pushed
the
17808-remove-extra-buffering
branch
from
65fea30 to
c16af1e
Compare
the-other-tim-brown
force-pushed
the
17808-remove-extra-buffering
branch
from
c16af1e to
3b56819
Compare
| private final BufferAllocator allocator; | ||
| private final int batchSize; | ||
| /** | ||
| * -- GETTER -- |
There was a problem hiding this comment.
[nit] Do we need this comment here? Or is the @Getter annotation already and field name already self explanatory?
There was a problem hiding this comment.
The -- GETTER -- will add the javadoc to the getter method with lombok but I agree this field is self explanatory
4 participants
Describe the issue this Pull Request addresses
The lance writer is maintaining a list of records in a list as well as writing out to a buffer within the arrow writer. We can remove this intermediate buffer and directly write the values out to the arrow writer. This should avoid extra row copy operations flagged in #17768 (comment)
Summary and Changelog
Impact
Reduces memory overhead
Risk Level
Low, existing test coverage exists for the writer
Documentation Update
Contributor's checklist