-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add append_nulls
and append_trusted_len_iter
to PrimitiveBuilder
#728
Conversation
Codecov Report
@@ Coverage Diff @@
## master #728 +/- ##
==========================================
+ Coverage 82.50% 82.52% +0.02%
==========================================
Files 168 168
Lines 47589 47691 +102
==========================================
+ Hits 39261 39356 +95
- Misses 8328 8335 +7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Thank you @bjchambers
@nevi-me or @jorgecarleitao do you have any thoughts on adding this API. While it is unsafe
it seems like there is more than sufficient existing precedent for this type of API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good addition as it will would make builders more efficient than they currently are for appending a lot of values.
@alamb to answer you, I'm fine with the addition.
Merging this in. Thanks @bjchambers ! Sorry for the delay in merging I have been on vacation and am now catching up. |
#728) * stub out impl * mark unsafe * add tests
#728) (#759) * stub out impl * mark unsafe * add tests Co-authored-by: Ben Chambers <[email protected]>
Which issue does this PR close?
Closes #725.
Rationale for this change
Allows appending multiple values / nulls in a single call. This avoids re-checking capacity, etc.
What changes are included in this PR?
Adds
append_nulls
andappend_trusted_len_iter
methods toPrimitiveBuilder
.Are there any user-facing changes?