feat(arrow/array): add Resize method to RecordBuilder#805
Conversation
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
Generally looks fine, but needs unit tests and to reference the relevant issue in the description.
zeroshade
changed the title
|
It says "Fixes #796" right there in the commit message. What sort of additional reference can I add? Will see regarding the unit test tomorrow. |
Ah, I missed that. My fault there. So yea, just needs tests and to fix the linting issue |
oakad
force-pushed
the
gh-796-helper-method-for-recordbuilder
branch
from
02388ed to
985cfe9
Compare
|
I have added a unit test. I also had to alter the Also, it feels that current approach (check row counts ahead of any array move action) is generally safer. |
Presently added Resize method addresses two, relatively common needs: 1. All fields within the Builder may need to be resized to the same length. 2. As part of error recovery process, fields may need to be truncated to the length of the shortest one, effectively discarding incomplete "rows". RecordBuilder.NewRecordBatch now performs row length validation prior to any action, to ensure reusability of RecordBuilder across errors. This also prevents hard to reason about memory leaks. Fixes apache#796 Unit test for RecordBuilder.Resize
oakad
force-pushed
the
gh-796-helper-method-for-recordbuilder
branch
from
985cfe9 to
ff7fc4c
Compare
2 participants
Presently added Resize method addresses two, relatively common needs:
Fixes #796
Rationale for this change
Easier error handling when only partial data is available due to error.
What changes are included in this PR?
Additional method for RecordBuilder.
Are these changes tested?
Unit test is provided.
Are there any user-facing changes?
Additional method in RecordBuilder.