Check overflow in MutableArrayData extend offsets (#3123) #3157
Conversation
| @@ -36,7 +36,7 @@ pub(super) fn extend_offsets<T: ArrowNativeType + Integer>( | |||
| offsets.windows(2).for_each(|offsets| { | |||
| // compute the new offset | |||
| let length = offsets[1] - offsets[0]; | |||
| last_offset = last_offset + length; | |||
| last_offset = last_offset.checked_add(&length).expect("offset overflow"); | |||
There was a problem hiding this comment.
Extend is infallible, so we can't return an Error. We also fairly consistently panic on offset overflow in other kernels, and I wish to eventually remove MutableArrayData, and so I think this is fine for now
There was a problem hiding this comment.
I think panic'ing rather than segfaulting is a much better practice
tustvold
force-pushed
the
check-overflow-in-mutable-array-data-extend
branch
from
ab6e4da to
f808ca9
Compare
Co-authored-by: Andrew Lamb <[email protected]>
|
Benchmark runs are scheduled for baseline = e214ccc and contender = f091cbb. f091cbb is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3123
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?