This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 684d64a580 Add `OffsetBuffer::first`/`last` to always return offset &
remove some `unwrap`s (#10759)
684d64a580 is described below
commit 684d64a580ce2e1c6a8cf6972be278e4165db458
Author: Emil Ernerfeldt <[email protected]>
AuthorDate: Sat Aug 29 10:09:23 2026 +0200
Add `OffsetBuffer::first`/`last` to always return offset & remove some
`unwrap`s (#10759)
# Which issue does this PR close?
- Part of https://github.com/apache/arrow-rs/issues/10553
# Rationale for this change
This is one of four PRs splitting up the `clippy::missing_panics_doc`
work.
1. #10755 - return errors from fallible functions
2. #10759 - remove unreachable panics
3. #10760 - document the panics that genuinely remain
4. #10761 - `#[expect]` the unreachable ones, so the lint can be turned
on
# What changes are included in this PR?
Replaces `unwrap`/`expect` calls that cannot fail with non-panicking
equivalents.
# Are these changes tested?
Covered by the existing tests. Every change here is a rewrite of code
whose
panic was unreachable, so there is no new behavior to test.
# Are there any user-facing changes?
Yes - `OffsetBuffer::last/first` no longer returns `Option`
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
arrow-array/src/array/byte_array.rs | 6 ++--
arrow-array/src/array/byte_view_array.rs | 5 +--
arrow-array/src/array/list_array.rs | 2 +-
arrow-array/src/array/map_array.rs | 4 +--
.../src/builder/generic_bytes_view_builder.rs | 27 +++++++++-------
.../src/builder/generic_list_view_builder.rs | 2 +-
arrow-array/src/types.rs | 10 +++---
arrow-buffer/src/bigint/mod.rs | 19 ++++++++----
arrow-buffer/src/buffer/mutable.rs | 18 +++++------
arrow-buffer/src/buffer/offset.rs | 36 ++++++++++++++++++++++
arrow-buffer/src/buffer/run.rs | 20 ++++++------
arrow-buffer/src/builder/boolean.rs | 18 +++++++----
arrow-buffer/src/builder/null.rs | 29 ++++++++---------
arrow-buffer/src/bytes.rs | 6 ++--
arrow-buffer/src/pool.rs | 13 +++++++-
arrow-cast/src/base64.rs | 2 +-
arrow-cast/src/cast/list.rs | 2 +-
arrow-cast/src/parse.rs | 2 +-
arrow-flight/src/sql/metadata/sql_info.rs | 7 ++---
arrow-ipc/src/convert.rs | 18 +++++------
arrow-ipc/src/reader.rs | 20 +++++++-----
arrow-ipc/src/writer.rs | 19 +++++++-----
arrow-row/src/lib.rs | 4 +--
arrow-select/src/concat.rs | 10 +++---
arrow-string/src/regexp.rs | 10 +++---
parquet/src/bloom_filter/mod.rs | 5 +--
parquet/src/file/metadata/footer_tail.rs | 2 +-
27 files changed, 186 insertions(+), 130 deletions(-)
diff --git a/arrow-array/src/array/byte_array.rs
b/arrow-array/src/array/byte_array.rs
index 728aeb342a..bf191c325b 100644
--- a/arrow-array/src/array/byte_array.rs
+++ b/arrow-array/src/array/byte_array.rs
@@ -295,10 +295,8 @@ impl<T: ByteArrayType> GenericByteArray<T> {
/// Returns true if all data within this array is ASCII
pub fn is_ascii(&self) -> bool {
- let offsets = self.value_offsets();
- let start = offsets.first().unwrap();
- let end = offsets.last().unwrap();
- self.value_data()[start.as_usize()..end.as_usize()].is_ascii()
+ let offsets = &self.value_offsets;
+
self.value_data()[offsets.first().as_usize()..offsets.last().as_usize()].is_ascii()
}
/// Returns the offset values in the offsets buffer
diff --git a/arrow-array/src/array/byte_view_array.rs
b/arrow-array/src/array/byte_view_array.rs
index 0ff1a77c49..ca7abe9aea 100644
--- a/arrow-array/src/array/byte_view_array.rs
+++ b/arrow-array/src/array/byte_view_array.rs
@@ -1037,10 +1037,7 @@ where
fn from(byte_array: &GenericByteArray<FROM>) -> Self {
let offsets = byte_array.offsets();
- let can_reuse_buffer = match offsets.last() {
- Some(offset) => offset.as_usize() < u32::MAX as usize,
- None => true,
- };
+ let can_reuse_buffer = offsets.last().as_usize() < u32::MAX as usize;
if can_reuse_buffer {
// build views directly pointing to the existing buffer
diff --git a/arrow-array/src/array/list_array.rs
b/arrow-array/src/array/list_array.rs
index ccab56db99..430750f48a 100644
--- a/arrow-array/src/array/list_array.rs
+++ b/arrow-array/src/array/list_array.rs
@@ -213,7 +213,7 @@ impl<OffsetSize: OffsetSizeTrait>
GenericListArray<OffsetSize> {
nulls: Option<NullBuffer>,
) -> Result<Self, ArrowError> {
let len = offsets.len() - 1; // Offsets guaranteed to not be empty
- let end_offset = offsets.last().unwrap().as_usize();
+ let end_offset = offsets.last().as_usize();
// don't need to check other values of `offsets` because they are
checked
// during construction of `OffsetBuffer`
if end_offset > values.len() {
diff --git a/arrow-array/src/array/map_array.rs
b/arrow-array/src/array/map_array.rs
index 9c7996cab3..919619aed3 100644
--- a/arrow-array/src/array/map_array.rs
+++ b/arrow-array/src/array/map_array.rs
@@ -70,7 +70,7 @@ impl MapArray {
ordered: bool,
) -> Result<Self, ArrowError> {
let len = offsets.len() - 1; // Offsets guaranteed to not be empty
- let end_offset = offsets.last().unwrap().as_usize();
+ let end_offset = offsets.last().as_usize();
// don't need to check other values of `offsets` because they are
checked
// during construction of `OffsetBuffer`
if end_offset > entries.len() {
@@ -259,7 +259,7 @@ impl MapArray {
let end = *unsafe { self.value_offsets().get_unchecked(i + 1) };
let start = *unsafe { self.value_offsets().get_unchecked(i) };
self.entries
- .slice(start.to_usize().unwrap(), (end -
start).to_usize().unwrap())
+ .slice(start.as_usize(), (end - start).as_usize())
}
/// Returns ith value of this map array.
diff --git a/arrow-array/src/builder/generic_bytes_view_builder.rs
b/arrow-array/src/builder/generic_bytes_view_builder.rs
index b68c5d2352..6ff209ef74 100644
--- a/arrow-array/src/builder/generic_bytes_view_builder.rs
+++ b/arrow-array/src/builder/generic_bytes_view_builder.rs
@@ -350,14 +350,19 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
ArrowError::InvalidArgumentError(format!("String length {} exceeds
u32::MAX", v.len()))
})?;
- if length <= MAX_INLINE_VIEW_LEN {
- let mut view_buffer = [0; 16];
- view_buffer[0..4].copy_from_slice(&length.to_le_bytes());
- view_buffer[4..4 + v.len()].copy_from_slice(v);
- self.views_buffer.push(u128::from_le_bytes(view_buffer));
- self.null_buffer_builder.append_non_null();
- return Ok(());
- }
+ // Anything at most `MAX_INLINE_VIEW_LEN` bytes long is inlined;
everything else
+ // needs a four byte prefix, which `first_chunk` gives us without any
indexing.
+ let prefix = match v.first_chunk::<4>() {
+ Some(prefix) if length > MAX_INLINE_VIEW_LEN =>
u32::from_le_bytes(*prefix),
+ _ => {
+ let mut view_buffer = [0; 16];
+ view_buffer[0..4].copy_from_slice(&length.to_le_bytes());
+ view_buffer[4..4 + v.len()].copy_from_slice(v);
+ self.views_buffer.push(u128::from_le_bytes(view_buffer));
+ self.null_buffer_builder.append_non_null();
+ return Ok(());
+ }
+ };
// Deduplication if:
// (1) deduplication is enabled.
@@ -416,8 +421,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
let view = ByteView {
length,
- // This won't panic as we checked the length of prefix earlier.
- prefix: u32::from_le_bytes(v[0..4].try_into().unwrap()),
+ prefix,
buffer_index,
offset,
};
@@ -702,7 +706,8 @@ pub fn make_view(data: &[u8], block_id: u32, offset: u32)
-> u128 {
_ => {
let view = ByteView {
length: len as u32,
- prefix: u32::from_le_bytes(data[0..4].try_into().unwrap()),
+ // this arm only matches lengths above 12, so there are at
least four bytes
+ prefix: u32::from_le_bytes([data[0], data[1], data[2],
data[3]]),
buffer_index: block_id,
offset,
};
diff --git a/arrow-array/src/builder/generic_list_view_builder.rs
b/arrow-array/src/builder/generic_list_view_builder.rs
index 925ea80940..e39cecbf65 100644
--- a/arrow-array/src/builder/generic_list_view_builder.rs
+++ b/arrow-array/src/builder/generic_list_view_builder.rs
@@ -163,7 +163,7 @@ where
#[inline]
pub fn append_null(&mut self) {
self.offsets_builder.push(self.current_offset);
- self.sizes_builder.push(OffsetSize::from_usize(0).unwrap());
+ self.sizes_builder.push(OffsetSize::zero());
self.null_buffer_builder.append_null();
}
diff --git a/arrow-array/src/types.rs b/arrow-array/src/types.rs
index 51b6b277fb..e0cc57bae5 100644
--- a/arrow-array/src/types.rs
+++ b/arrow-array/src/types.rs
@@ -963,7 +963,7 @@ impl Date32Type {
///
/// Returns `Some(NaiveDate)` if it fits, `None` otherwise.
pub fn to_naive_date_opt(i: <Date32Type as ArrowPrimitiveType>::Native) ->
Option<NaiveDate> {
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
+ let epoch = NaiveDate::default();
let d = Duration::try_days(i as i64)?;
epoch.checked_add_signed(d)
}
@@ -974,7 +974,7 @@ impl Date32Type {
///
/// * `d` - The NaiveDate to convert
pub fn from_naive_date(d: NaiveDate) -> <Date32Type as
ArrowPrimitiveType>::Native {
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
+ let epoch = NaiveDate::default();
d.sub(epoch).num_days() as <Date32Type as ArrowPrimitiveType>::Native
}
@@ -1245,7 +1245,7 @@ impl Date64Type {
///
/// Returns `Some(NaiveDateTime)` if it fits, `None` otherwise.
pub fn to_naive_date_opt(i: <Date64Type as ArrowPrimitiveType>::Native) ->
Option<NaiveDate> {
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
+ let epoch = NaiveDate::default();
let d = Duration::try_milliseconds(i)?;
epoch.checked_add_signed(d)
}
@@ -1256,7 +1256,7 @@ impl Date64Type {
///
/// * `d` - The NaiveDate to convert
pub fn from_naive_date(d: NaiveDate) -> <Date64Type as
ArrowPrimitiveType>::Native {
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
+ let epoch = NaiveDate::default();
d.sub(epoch).num_milliseconds()
}
@@ -1745,7 +1745,7 @@ impl<O: OffsetSizeTrait> ByteArrayType for
GenericBinaryType<O> {
fn validate(offsets: &OffsetBuffer<Self::Offset>, values: &Buffer) ->
Result<(), ArrowError> {
// offsets are guaranteed to be monotonically increasing and non-empty
- let max_offset = offsets.last().unwrap().as_usize();
+ let max_offset = offsets.last().as_usize();
if values.len() < max_offset {
return Err(ArrowError::InvalidArgumentError(format!(
"Maximum offset of {max_offset} is larger than values of
length {}",
diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs
index 6811e2e1ed..aa1d5d48c8 100644
--- a/arrow-buffer/src/bigint/mod.rs
+++ b/arrow-buffer/src/bigint/mod.rs
@@ -716,19 +716,26 @@ impl i256 {
return Some(0);
}
+ /// `10^32`
+ const POW10_32: i256 =
i256::from_i128(100_000_000_000_000_000_000_000_000_000_000);
+
+ /// `10^64`
+ const POW10_64: i256 = i256::from_parts(
+ 146_510_663_073_550_942_663_504_491_129_887_260_672,
+ 29_387_358_770_557_187_699_218_413,
+ );
+
// Layered approach to calculate logarithm using i128 log operations
only
// Consult int_log10.rs stdlib implementiation for u128
- let pow_64: i256 = i256::from(10).checked_pow(64).unwrap();
- let pow_32: i256 = i256::from(10).checked_pow(32).unwrap();
- if self >= pow_64 {
- let value = self.checked_div(pow_64)?;
+ if self >= POW10_64 {
+ let value = self.checked_div(POW10_64)?;
// self is between 10^64 and 10^77 (~i256::MAX).
// `value` is 14 digits max (10^77 / 10^64 = 10^13),
// so it fits to `low` u128
debug_assert!(value.high == 0);
Some(64 + value.low.checked_ilog10()?)
- } else if self >= pow_32 {
- let value = self.checked_div(pow_32)?;
+ } else if self >= POW10_32 {
+ let value = self.checked_div(POW10_32)?;
// self is between 10^32 and 10^64.
// `value` is 33 digits max (10^64/10^32=10^32)
// so it fits to `low` 128-bit value
diff --git a/arrow-buffer/src/buffer/mutable.rs
b/arrow-buffer/src/buffer/mutable.rs
index 1dcfaf67f4..150fe44043 100644
--- a/arrow-buffer/src/buffer/mutable.rs
+++ b/arrow-buffer/src/buffer/mutable.rs
@@ -27,7 +27,7 @@ use crate::{
};
#[cfg(feature = "pool")]
-use crate::pool::{MemoryPool, MemoryReservation};
+use crate::pool::{MemoryPool, MemoryReservation, lock_reservation};
#[cfg(feature = "pool")]
use std::sync::Mutex;
@@ -238,7 +238,7 @@ impl MutableBuffer {
let len = bytes.len();
let data = bytes.ptr();
#[cfg(feature = "pool")]
- let reservation = bytes.reservation.lock().unwrap().take();
+ let reservation = lock_reservation(&bytes.reservation).take();
mem::forget(bytes);
Ok(Self {
@@ -447,7 +447,7 @@ impl MutableBuffer {
self.layout = new_layout;
#[cfg(feature = "pool")]
{
- if let Some(reservation) =
self.reservation.lock().unwrap().as_mut() {
+ if let Some(reservation) =
lock_reservation(&self.reservation).as_mut() {
reservation.resize(self.layout.size());
}
}
@@ -464,7 +464,7 @@ impl MutableBuffer {
self.len = len;
#[cfg(feature = "pool")]
{
- if let Some(reservation) =
self.reservation.lock().unwrap().as_mut() {
+ if let Some(reservation) =
lock_reservation(&self.reservation).as_mut() {
reservation.resize(self.len);
}
}
@@ -484,7 +484,7 @@ impl MutableBuffer {
self.len = new_len;
#[cfg(feature = "pool")]
{
- if let Some(reservation) =
self.reservation.lock().unwrap().as_mut() {
+ if let Some(reservation) =
lock_reservation(&self.reservation).as_mut() {
reservation.resize(self.len);
}
}
@@ -573,7 +573,7 @@ impl MutableBuffer {
self.len = 0;
#[cfg(feature = "pool")]
{
- if let Some(reservation) =
self.reservation.lock().unwrap().as_mut() {
+ if let Some(reservation) =
lock_reservation(&self.reservation).as_mut() {
reservation.resize(self.len);
}
}
@@ -608,8 +608,8 @@ impl MutableBuffer {
let bytes = unsafe { Bytes::new(self.data, self.len,
Deallocation::Standard(self.layout)) };
#[cfg(feature = "pool")]
{
- let reservation = self.reservation.lock().unwrap().take();
- *bytes.reservation.lock().unwrap() = reservation;
+ let reservation = lock_reservation(&self.reservation).take();
+ *lock_reservation(&bytes.reservation) = reservation;
}
std::mem::forget(self);
Buffer::from(bytes)
@@ -934,7 +934,7 @@ impl MutableBuffer {
/// multiple arrays.
#[cfg(feature = "pool")]
pub fn claim(&self, pool: &dyn MemoryPool) {
- *self.reservation.lock().unwrap() =
Some(pool.reserve(self.capacity()));
+ *lock_reservation(&self.reservation) =
Some(pool.reserve(self.capacity()));
}
}
diff --git a/arrow-buffer/src/buffer/offset.rs
b/arrow-buffer/src/buffer/offset.rs
index f1dcdfd5aa..12823c9eb5 100644
--- a/arrow-buffer/src/buffer/offset.rs
+++ b/arrow-buffer/src/buffer/offset.rs
@@ -177,6 +177,42 @@ impl<O: ArrowNativeType> OffsetBuffer<O> {
Self(ScalarBuffer::from(offsets))
}
+ /// The first offset, i.e. the start of the first range.
+ ///
+ /// An [`OffsetBuffer`] is never empty, so this always returns an offset.
+ ///
+ /// ```
+ /// # use arrow_buffer::OffsetBuffer;
+ /// let offsets = OffsetBuffer::<i32>::from_lengths([1, 3, 5]);
+ /// assert_eq!(offsets.first(), 0);
+ /// assert_eq!(OffsetBuffer::<i32>::new_empty().first(), 0);
+ /// ```
+ #[inline]
+ pub fn first(&self) -> O {
+ self.0
+ .first()
+ .copied()
+ .expect("An `OffsetBuffer` is never empty")
+ }
+
+ /// The last offset, i.e. the end of the last range.
+ ///
+ /// An [`OffsetBuffer`] is never empty, so this always returns an offset.
+ ///
+ /// ```
+ /// # use arrow_buffer::OffsetBuffer;
+ /// let offsets = OffsetBuffer::<i32>::from_lengths([1, 3, 5]);
+ /// assert_eq!(offsets.last(), 9);
+ /// assert_eq!(OffsetBuffer::<i32>::new_empty().last(), 0);
+ /// ```
+ #[inline]
+ pub fn last(&self) -> O {
+ self.0
+ .last()
+ .copied()
+ .expect("An `OffsetBuffer` is never empty")
+ }
+
/// Get an Iterator over the lengths of this [`OffsetBuffer`]
///
/// ```
diff --git a/arrow-buffer/src/buffer/run.rs b/arrow-buffer/src/buffer/run.rs
index c65e3b5cc6..6c9a4cb52e 100644
--- a/arrow-buffer/src/buffer/run.rs
+++ b/arrow-buffer/src/buffer/run.rs
@@ -209,8 +209,9 @@ where
&self.run_ends[start..=end]
};
physical_slice.iter().map(move |&val| {
+ // `len` is at most the largest run end, so it always fits in `E`
let val = val.as_usize().saturating_sub(offset).min(len);
- E::from_usize(val).unwrap()
+ E::usize_as(val)
})
}
@@ -230,8 +231,8 @@ where
///
/// The result is arbitrary if `logical_index >= self.len()`.
pub fn get_physical_index(&self, logical_index: usize) -> usize {
- let logical_index = E::usize_as(self.logical_offset + logical_index);
- let cmp = |p: &E| p.partial_cmp(&logical_index).unwrap();
+ let logical_index = self.logical_offset + logical_index;
+ let cmp = |p: &E| p.as_usize().cmp(&logical_index);
match self.run_ends.binary_search_by(cmp) {
Ok(idx) => idx + 1,
@@ -337,16 +338,13 @@ where
// Instead of sorting `logical_indices` directly, sort the
`ordered_indices`
// whose values are index of `logical_indices`
- ordered_indices.sort_unstable_by(|lhs, rhs| {
- logical_indices[*lhs]
- .partial_cmp(&logical_indices[*rhs])
- .unwrap()
- });
+ ordered_indices.sort_unstable_by_key(|&idx|
logical_indices[idx].as_usize());
// Return early if all the logical indices cannot be converted to
physical indices.
- let largest_logical_index =
logical_indices[*ordered_indices.last().unwrap()].as_usize();
- if largest_logical_index >= len {
- return Err(logical_indices[*ordered_indices.last().unwrap()]);
+ // `ordered_indices` has `indices_len` entries, and the empty case
returned above.
+ let largest_logical_index =
logical_indices[ordered_indices[indices_len - 1]];
+ if largest_logical_index.as_usize() >= len {
+ return Err(largest_logical_index);
}
// Skip some physical indices based on offset.
diff --git a/arrow-buffer/src/builder/boolean.rs
b/arrow-buffer/src/builder/boolean.rs
index 3c6239e4c8..68cb2cef29 100644
--- a/arrow-buffer/src/builder/boolean.rs
+++ b/arrow-buffer/src/builder/boolean.rs
@@ -161,9 +161,11 @@ impl BooleanBufferBuilder {
self.len = len;
let remainder = self.len % 8;
- if remainder != 0 {
+ if remainder != 0
+ && let Some(last) = self.buffer.as_mut().last_mut()
+ {
let mask = (1_u8 << remainder).wrapping_sub(1);
- *self.buffer.as_mut().last_mut().unwrap() &= mask;
+ *last &= mask;
}
}
@@ -248,14 +250,18 @@ impl BooleanBufferBuilder {
let cur_remainder = self.len % 8;
let new_remainder = new_len % 8;
- if cur_remainder != 0 {
+ if cur_remainder != 0
+ && let Some(last) = self.buffer.as_slice_mut().last_mut()
+ {
// Pad last byte with 1s
- *self.buffer.as_slice_mut().last_mut().unwrap() |= !((1 <<
cur_remainder) - 1)
+ *last |= !((1 << cur_remainder) - 1);
}
self.buffer.resize(new_len_bytes, 0xFF);
- if new_remainder != 0 {
+ if new_remainder != 0
+ && let Some(last) = self.buffer.as_slice_mut().last_mut()
+ {
// Clear remaining bits
- *self.buffer.as_slice_mut().last_mut().unwrap() &= (1 <<
new_remainder) - 1
+ *last &= (1 << new_remainder) - 1;
}
self.len = new_len;
}
diff --git a/arrow-buffer/src/builder/null.rs b/arrow-buffer/src/builder/null.rs
index b1b5511010..ee58ab591e 100644
--- a/arrow-buffer/src/builder/null.rs
+++ b/arrow-buffer/src/builder/null.rs
@@ -129,16 +129,14 @@ impl NullBufferBuilder {
/// to indicate that these `n` items are nulls.
#[inline]
pub fn append_n_nulls(&mut self, n: usize) {
- self.materialize_if_needed();
- self.bitmap_builder.as_mut().unwrap().append_n(n, false);
+ self.materialize_if_needed().append_n(n, false);
}
/// Appends a `false` into the builder
/// to indicate that this item is null.
#[inline]
pub fn append_null(&mut self) {
- self.materialize_if_needed();
- self.bitmap_builder.as_mut().unwrap().append(false);
+ self.materialize_if_needed().append(false);
}
/// Appends a boolean value into the builder.
@@ -158,8 +156,7 @@ impl NullBufferBuilder {
/// Panics for the same reasons as [`BooleanBufferBuilder::set_bit`]
#[inline]
pub fn set_bit(&mut self, index: usize, v: bool) {
- self.materialize_if_needed();
- self.bitmap_builder.as_mut().unwrap().set_bit(index, v);
+ self.materialize_if_needed().set_bit(index, v);
}
/// Gets a bit in the buffer at `index`
@@ -194,7 +191,7 @@ impl NullBufferBuilder {
pub fn append_slice(&mut self, slice: &[bool]) {
// First check if not already materialized before checking if there
are any nulls
if self.bitmap_builder.is_none() && slice.iter().any(|v| !v) {
- self.materialize()
+ self.materialize_if_needed();
}
if let Some(buf) = self.bitmap_builder.as_mut() {
buf.append_slice(slice)
@@ -244,19 +241,17 @@ impl NullBufferBuilder {
Some(self.bitmap_builder.as_ref()?.as_slice())
}
- fn materialize_if_needed(&mut self) {
- if self.bitmap_builder.is_none() {
- self.materialize()
- }
+ fn materialize_if_needed(&mut self) -> &mut BooleanBufferBuilder {
+ let (len, capacity) = (self.len, self.capacity);
+ self.bitmap_builder
+ .get_or_insert_with(|| Self::materialize(len, capacity))
}
#[cold]
- fn materialize(&mut self) {
- if self.bitmap_builder.is_none() {
- let mut b = BooleanBufferBuilder::new(self.len.max(self.capacity));
- b.append_n(self.len, true);
- self.bitmap_builder = Some(b);
- }
+ fn materialize(len: usize, capacity: usize) -> BooleanBufferBuilder {
+ let mut b = BooleanBufferBuilder::new(len.max(capacity));
+ b.append_n(len, true);
+ b
}
/// Return a mutable reference to the inner bitmap slice.
diff --git a/arrow-buffer/src/bytes.rs b/arrow-buffer/src/bytes.rs
index 07b08da43d..de9f7befe6 100644
--- a/arrow-buffer/src/bytes.rs
+++ b/arrow-buffer/src/bytes.rs
@@ -27,7 +27,7 @@ use crate::alloc::Deallocation;
use crate::buffer::dangling_ptr;
#[cfg(feature = "pool")]
-use crate::pool::{MemoryPool, MemoryReservation};
+use crate::pool::{MemoryPool, MemoryReservation, lock_reservation};
#[cfg(feature = "pool")]
use std::sync::Mutex;
@@ -110,7 +110,7 @@ impl Bytes {
/// Register this [`Bytes`] with the provided [`MemoryPool`], replacing
any prior reservation.
#[cfg(feature = "pool")]
pub(crate) fn claim(&self, pool: &dyn MemoryPool) {
- *self.reservation.lock().unwrap() =
Some(pool.reserve(self.capacity()));
+ *lock_reservation(&self.reservation) =
Some(pool.reserve(self.capacity()));
}
/// Resize the memory reservation of this buffer
@@ -118,7 +118,7 @@ impl Bytes {
/// This is a no-op if this buffer doesn't have a reservation.
#[cfg(feature = "pool")]
fn resize_reservation(&self, new_size: usize) {
- let mut guard = self.reservation.lock().unwrap();
+ let mut guard = lock_reservation(&self.reservation);
if let Some(mut reservation) = guard.take() {
// Resize the reservation
reservation.resize(new_size);
diff --git a/arrow-buffer/src/pool.rs b/arrow-buffer/src/pool.rs
index 95bd308a35..6acae3ffdb 100644
--- a/arrow-buffer/src/pool.rs
+++ b/arrow-buffer/src/pool.rs
@@ -30,8 +30,8 @@
//! ```
use std::fmt::Debug;
-use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::{Arc, Mutex, MutexGuard, PoisonError};
/// A memory reservation within a [`MemoryPool`] that is freed on drop
pub trait MemoryReservation: Debug + Send + Sync {
@@ -147,6 +147,17 @@ impl MemoryReservation for Tracker {
}
}
+/// Lock a memory reservation, recovering from a poisoned lock.
+///
+/// A poisoned lock only means that some other thread panicked. The
reservation it
+/// guards is plain size accounting, so there is no broken invariant to
protect, and
+/// recovering it is always preferable to panicking.
+pub(crate) fn lock_reservation(
+ reservation: &Mutex<Option<Box<dyn MemoryReservation>>>,
+) -> MutexGuard<'_, Option<Box<dyn MemoryReservation>>> {
+ reservation.lock().unwrap_or_else(PoisonError::into_inner)
+}
+
#[cfg(test)]
mod tests {
use super::*;
diff --git a/arrow-cast/src/base64.rs b/arrow-cast/src/base64.rs
index b444f8d1a8..0cd724df7c 100644
--- a/arrow-cast/src/base64.rs
+++ b/arrow-cast/src/base64.rs
@@ -43,7 +43,7 @@ pub fn b64_encode<E: Engine, O: OffsetSizeTrait>(
encoded_len(len, engine.config().encode_padding()).unwrap()
});
let offsets = OffsetBuffer::<O>::from_lengths(lengths);
- let buffer_len = offsets.last().unwrap().as_usize();
+ let buffer_len = offsets.last().as_usize();
let mut buffer = vec![0_u8; buffer_len];
let mut offset = 0;
diff --git a/arrow-cast/src/cast/list.rs b/arrow-cast/src/cast/list.rs
index fc07768a73..837715e885 100644
--- a/arrow-cast/src/cast/list.rs
+++ b/arrow-cast/src/cast/list.rs
@@ -326,7 +326,7 @@ pub(crate) fn cast_list<I: OffsetSizeTrait, O:
OffsetSizeTrait>(
let offsets = list.offsets();
let nulls = list.nulls().cloned();
- if offsets.last().unwrap().as_usize() > O::MAX_OFFSET {
+ if offsets.last().as_usize() > O::MAX_OFFSET {
return Err(ArrowError::ComputeError(format!(
"Offset overflow when casting from {} to {}",
array.data_type(),
diff --git a/arrow-cast/src/parse.rs b/arrow-cast/src/parse.rs
index b6eb2692b3..c90214d467 100644
--- a/arrow-cast/src/parse.rs
+++ b/arrow-cast/src/parse.rs
@@ -185,7 +185,7 @@ pub fn string_to_datetime<T: TimeZone>(timezone: &T, s:
&str) -> Result<DateTime
let parser = TimestampParser::new(bytes);
let date = parser.date().ok_or_else(|| err("error parsing date"))?;
if bytes.len() == 10 {
- let datetime = date.and_time(NaiveTime::from_hms_opt(0, 0,
0).unwrap());
+ let datetime = date.and_time(NaiveTime::MIN);
return timezone
.from_local_datetime(&datetime)
.single()
diff --git a/arrow-flight/src/sql/metadata/sql_info.rs
b/arrow-flight/src/sql/metadata/sql_info.rs
index ec4f901b84..5ccf567c6b 100644
--- a/arrow-flight/src/sql/metadata/sql_info.rs
+++ b/arrow-flight/src/sql/metadata/sql_info.rs
@@ -356,11 +356,8 @@ impl SqlInfoDataBuilder {
let mut name_builder = UInt32Builder::new();
let mut value_builder = SqlInfoUnionBuilder::new();
- let mut names: Vec<_> = self.infos.keys().copied().collect();
- names.sort_unstable();
-
- for key in names {
- let (name, value) = self.infos.get_key_value(&key).unwrap();
+ // `infos` is a `BTreeMap`, so it iterates in sorted order already
+ for (name, value) in &self.infos {
name_builder.append_value(*name);
value_builder.append_value(value)?
}
diff --git a/arrow-ipc/src/convert.rs b/arrow-ipc/src/convert.rs
index 586b89055d..9affcd6b4a 100644
--- a/arrow-ipc/src/convert.rs
+++ b/arrow-ipc/src/convert.rs
@@ -278,18 +278,18 @@ pub fn try_schema_from_ipc_buffer(buffer: &[u8]) ->
Result<Schema, ArrowError> {
));
}
- let (len, buffer) = if buffer[..4] == CONTINUATION_MARKER {
- if buffer.len() < 8 {
- return Err(ArrowError::ParseError(
- "The buffer length is less than 8 and missing the length of
buffer".to_string(),
- ));
- }
- buffer[4..].split_at(4)
+ let rest = if buffer[..4] == CONTINUATION_MARKER {
+ &buffer[4..]
} else {
- buffer.split_at(4)
+ buffer
+ };
+ let Some((len, buffer)) = rest.split_first_chunk::<4>() else {
+ return Err(ArrowError::ParseError(
+ "The buffer length is less than 8 and missing the length of
buffer".to_string(),
+ ));
};
- let len = <i32>::from_le_bytes(len.try_into().unwrap());
+ let len = i32::from_le_bytes(*len);
if len < 0 {
return Err(ArrowError::ParseError(format!(
"The encapsulated message's reported length is negative ({len})"
diff --git a/arrow-ipc/src/reader.rs b/arrow-ipc/src/reader.rs
index 7d757b534f..c8eb45515b 100644
--- a/arrow-ipc/src/reader.rs
+++ b/arrow-ipc/src/reader.rs
@@ -562,19 +562,23 @@ impl<'a> RecordBatchDecoder<'a> {
// project fields
for (idx, field) in schema.fields().iter().enumerate() {
// A projected field can appear more than once, so collect all
matching positions.
- let mut child = None;
+ let mut decoded = None;
for (proj_idx, projected_idx) in projection.iter().enumerate()
{
if *projected_idx == idx {
- if child.is_none() {
- child = Some(self.create_array(field, &mut
variadic_counts)?);
- }
-
// Reuse the decoded array for duplicate projection
entries.
- arrays.push((proj_idx,
child.as_ref().unwrap().clone()));
+ let child = match decoded.clone() {
+ Some(child) => child,
+ None => {
+ let child = self.create_array(field, &mut
variadic_counts)?;
+ decoded = Some(Arc::clone(&child));
+ child
+ }
+ };
+ arrays.push((proj_idx, child));
}
}
- if child.is_none() {
+ if decoded.is_none() {
self.skip_field(field, &mut variadic_counts)?;
}
}
@@ -949,7 +953,7 @@ pub fn read_footer_length(buf: [u8; 10]) -> Result<usize,
ArrowError> {
}
// read footer length
- let footer_len = i32::from_le_bytes(buf[..4].try_into().unwrap());
+ let footer_len = i32::from_le_bytes([buf[0], buf[1], buf[2], buf[3]]);
footer_len
.try_into()
.map_err(|_| ArrowError::ParseError(format!("Invalid footer length:
{footer_len}")))
diff --git a/arrow-ipc/src/writer.rs b/arrow-ipc/src/writer.rs
index 8f55e0fe67..11a3030e03 100644
--- a/arrow-ipc/src/writer.rs
+++ b/arrow-ipc/src/writer.rs
@@ -469,14 +469,17 @@ impl IpcWriteOptions {
write_legacy_ipc_format: bool,
metadata_version: crate::MetadataVersion,
) -> Result<Self, ArrowError> {
- let is_alignment_valid =
- alignment == 8 || alignment == 16 || alignment == 32 || alignment
== 64;
- if !is_alignment_valid {
- return Err(ArrowError::InvalidArgumentError(
- "Alignment should be 8, 16, 32, or 64.".to_string(),
- ));
- }
- let alignment: u8 = u8::try_from(alignment).expect("range already
checked");
+ let alignment: u8 = match alignment {
+ 8 => 8,
+ 16 => 16,
+ 32 => 32,
+ 64 => 64,
+ _ => {
+ return Err(ArrowError::InvalidArgumentError(
+ "Alignment should be 8, 16, 32, or 64.".to_string(),
+ ));
+ }
+ };
match metadata_version {
crate::MetadataVersion::V1
| crate::MetadataVersion::V2
diff --git a/arrow-row/src/lib.rs b/arrow-row/src/lib.rs
index 5f016ec7e6..284743f388 100644
--- a/arrow-row/src/lib.rs
+++ b/arrow-row/src/lib.rs
@@ -4779,7 +4779,7 @@ mod tests {
F: FnOnce(&mut StdRng, usize) -> ArrayRef,
{
let offsets = OffsetBuffer::<i32>::from_lengths((0..len).map(|_|
rng.random_range(0..10)));
- let values_len = offsets.last().unwrap().to_usize().unwrap();
+ let values_len = offsets.last().as_usize();
let values = values(rng, values_len);
let nulls = NullBuffer::from_iter((0..len).map(|_|
rng.random_bool(valid_percent)));
let field = Arc::new(Field::new_list_field(values.data_type().clone(),
true));
@@ -4835,7 +4835,7 @@ mod tests {
ValuesFn: FnOnce(&mut StdRng, usize) -> ArrayRef,
{
let offsets = OffsetBuffer::<i32>::from_lengths((0..len).map(|_|
rng.random_range(0..10)));
- let entries_len = offsets.last().unwrap().to_usize().unwrap();
+ let entries_len = offsets.last().as_usize();
let keys = gen_keys(rng, entries_len);
let values = gen_values(rng, entries_len);
let nulls = NullBuffer::from_iter((0..len).map(|_|
rng.random_bool(valid_percent)));
diff --git a/arrow-select/src/concat.rs b/arrow-select/src/concat.rs
index eece5ffbaf..72dd239bbb 100644
--- a/arrow-select/src/concat.rs
+++ b/arrow-select/src/concat.rs
@@ -159,7 +159,7 @@ fn concat_lists<OffsetSize: OffsetSizeTrait>(
output_len += l.len();
list_has_nulls |= l.null_count() != 0;
list_has_slices |= l.offsets()[0] > OffsetSize::zero()
- || l.offsets().last().unwrap().as_usize() < l.values().len();
+ || l.offsets().last().as_usize() < l.values().len();
})
.collect::<Vec<_>>();
@@ -184,7 +184,7 @@ fn concat_lists<OffsetSize: OffsetSizeTrait>(
// we concatenate them below only the relevant values are included
let offsets = l.offsets();
let start_offset = offsets[0].as_usize();
- let end_offset = offsets.last().unwrap().as_usize();
+ let end_offset = offsets.last().as_usize();
sliced_values.push(l.values().slice(start_offset, end_offset -
start_offset));
}
sliced_values.iter().map(|a| a.as_ref()).collect()
@@ -224,7 +224,7 @@ fn concat_maps(
output_len += m.len();
map_has_nulls |= m.null_count() != 0;
map_has_slices |=
- m.offsets()[0] > 0 || m.offsets().last().unwrap().as_usize() <
m.entries().len();
+ m.offsets()[0] > 0 || m.offsets().last().as_usize() <
m.entries().len();
})
.collect::<Vec<_>>();
@@ -247,7 +247,7 @@ fn concat_maps(
for m in &maps {
let offsets = m.offsets();
let start_offset = offsets[0].as_usize();
- let end_offset = offsets.last().unwrap().as_usize();
+ let end_offset = offsets.last().as_usize();
let entries_arr: &dyn Array = m.entries();
sliced_entries.push(entries_arr.slice(start_offset, end_offset -
start_offset));
}
@@ -1042,7 +1042,7 @@ mod tests {
// verify that this test covers the case when the first offset is
zero, but the
// last offset doesn't cover the entire array
assert_eq!(list1_array.offsets()[0].as_usize(), 0);
- assert!(list1_array.offsets().last().unwrap().as_usize() <
list1_array.values().len());
+ assert!(list1_array.offsets().last().as_usize() <
list1_array.values().len());
let array_result = concat(&[&list1_array, &list2_array]).unwrap();
let expected = list1_values.chain(list2);
diff --git a/arrow-string/src/regexp.rs b/arrow-string/src/regexp.rs
index 07520a2090..0474f2a7b8 100644
--- a/arrow-string/src/regexp.rs
+++ b/arrow-string/src/regexp.rs
@@ -433,14 +433,14 @@ pub fn regexp_match(
None => (None, None),
};
- if is_flags_scalar.is_some() && is_rhs_scalar != is_flags_scalar.unwrap() {
+ if is_flags_scalar.is_some_and(|is_flags_scalar| is_rhs_scalar !=
is_flags_scalar) {
return Err(ArrowError::ComputeError(
"regexp_match() requires both pattern and flags to be either
scalar or array"
.to_string(),
));
}
- if flags_array.is_some() && rhs.data_type() != flags.unwrap().data_type() {
+ if flags.is_some_and(|flags| rhs.data_type() != flags.data_type()) {
return Err(ArrowError::ComputeError(
"regexp_match() requires both pattern and flags to be either Utf8,
Utf8View or LargeUtf8"
.to_string(),
@@ -461,7 +461,7 @@ pub fn regexp_match(
}
};
- if regex.is_none() {
+ let Some(regex) = regex else {
return Ok(new_null_array(
&DataType::List(Arc::new(Field::new_list_field(
array.data_type().clone(),
@@ -469,9 +469,7 @@ pub fn regexp_match(
))),
array.len(),
));
- }
-
- let regex = regex.unwrap();
+ };
let pattern = if let Some(flag) = flag {
format!("(?{flag}){regex}")
diff --git a/parquet/src/bloom_filter/mod.rs b/parquet/src/bloom_filter/mod.rs
index 30310a3882..f92a04dd31 100644
--- a/parquet/src/bloom_filter/mod.rs
+++ b/parquet/src/bloom_filter/mod.rs
@@ -405,8 +405,9 @@ impl Sbbf {
.chunks_exact(4 * 8)
.map(|chunk| {
let mut block = Block::ZERO;
- for (i, word) in chunk.chunks_exact(4).enumerate() {
- block[i] = u32::from_le_bytes(word.try_into().unwrap());
+ let (words, _remainder) = chunk.as_chunks::<4>();
+ for (i, word) in words.iter().enumerate() {
+ block[i] = u32::from_le_bytes(*word);
}
block
})
diff --git a/parquet/src/file/metadata/footer_tail.rs
b/parquet/src/file/metadata/footer_tail.rs
index c33bc7a25c..41f18db70a 100644
--- a/parquet/src/file/metadata/footer_tail.rs
+++ b/parquet/src/file/metadata/footer_tail.rs
@@ -67,7 +67,7 @@ impl FooterTail {
return Err(general_err!("Invalid Parquet file. Corrupt footer"));
};
// get the metadata length from the footer
- let metadata_len = u32::from_le_bytes(slice[..4].try_into().unwrap());
+ let metadata_len = u32::from_le_bytes([slice[0], slice[1], slice[2],
slice[3]]);
Ok(FooterTail {
// u32 won't be larger than usize in most cases
