Provide into_builder for bytearray#3326
Conversation
viirya
force-pushed
the
byte_array_into_builder
branch
from
29d0cc3 to
bfe278e
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
This is a pretty compelling feature @viirya 👍 I don't have enough of the low level buffer code loaded into my head to feel confident approving this without some more study. If no on else does so I'll try and find some time later this week
What do you think @avantgardnerio ?
viirya
force-pushed
the
byte_array_into_builder
branch
from
63e8424 to
453bd35
Compare
| fn test_into_builder_err() { | ||
| let array: StringArray = vec!["hello", "arrow"].into(); | ||
|
|
||
| // Clone it, so we cannot get a mutable builder back |
| /// # Safety | ||
| /// This doesn't verify buffer contents as it assumes the buffers are from existing and | ||
| /// valid [`GenericByteArray`]. | ||
| pub unsafe fn new_from_buffer( |
tustvold
left a comment
tustvold
left a comment
There was a problem hiding this comment.
I think we need to drop the mutable slice methods from GenericByteBuilder, as they currently can lead to UB
viirya
force-pushed
the
byte_array_into_builder
branch
from
860732c to
5f09047
Compare
tustvold
left a comment
tustvold
left a comment
There was a problem hiding this comment.
Thank you for sticking with this 😄
|
Benchmark runs are scheduled for baseline = 808a982 and contender = ec43d6f. ec43d6f is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
4 participants
Which issue does this PR close?
Closes #1981.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?