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 136de0d663 fix: don't panic in `GenericByteArray::from_iter_values`
(#10729)
136de0d663 is described below
commit 136de0d6636aa40c1af846257b561fd88dd52ac4
Author: Emil Ernerfeldt <[email protected]>
AuthorDate: Tue Aug 18 17:38:15 2026 -0700
fix: don't panic in `GenericByteArray::from_iter_values` (#10729)
# Which issue does this PR close?
* Follow-up to https://github.com/apache/arrow-rs/pull/10656
(https://github.com/apache/arrow-rs/pull/10656#discussion_r3771712125)
* Part of https://github.com/apache/arrow-rs/issues/10553
# Rationale for this change
`size_hint` is only a hint: an iterator may report no upper bound at
all, and is [not
required](https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.size_hint)
to yield the number of items it reports. `from_iter_values` used the
upper bound only to pre-allocate, yet panicked when there was none.
# What changes are included in this PR?
`GenericByteArray::from_iter_values` now falls back to the lower bound
when there is no upper bound, and no longer panics. The `# Panics` doc
is updated.
This was the only such case: the remaining `size_hint().1.expect(…)`
sites are all in `unsafe` trusted-length APIs, where the panic guards a
safety contract.
# Are these changes tested?
Yes, new test with an iterator that reports no upper bound, and one that
over-reports.
# Are there any user-facing changes?
One less panic. No breaking changes.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
arrow-array/src/array/byte_array.rs | 37 +++++++++++++++++++++++++++++++------
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/arrow-array/src/array/byte_array.rs
b/arrow-array/src/array/byte_array.rs
index 74b9d0353b..be4ecdee50 100644
--- a/arrow-array/src/array/byte_array.rs
+++ b/arrow-array/src/array/byte_array.rs
@@ -216,18 +216,23 @@ impl<T: ByteArrayType> GenericByteArray<T> {
/// Creates a [`GenericByteArray`] based on an iterator of values without
nulls
///
/// # Panics
- /// Panics if the iterator has no upper bound on its size hint, or if the
total
- /// length of the values exceeds `T::Offset::MAX`
+ /// Panics if the total length of the values exceeds `T::Offset::MAX`
pub fn from_iter_values<Ptr, I>(iter: I) -> Self
where
Ptr: AsRef<T::Native>,
I: IntoIterator<Item = Ptr>,
{
let iter = iter.into_iter();
- let (_, data_len) = iter.size_hint();
- let data_len = data_len.expect("Iterator must be sized"); // panic if
no upper bound.
-
- let mut offsets = MutableBuffer::new((data_len + 1) *
std::mem::size_of::<T::Offset>());
+ // The size hint is only used to pre-allocate: an iterator is free to
yield
+ // a different number of items than it reports.
+ let (lower, upper) = iter.size_hint();
+ let capacity = upper.unwrap_or(lower);
+
+ let mut offsets = MutableBuffer::new(
+ capacity
+ .saturating_add(1)
+ .saturating_mul(std::mem::size_of::<T::Offset>()),
+ );
offsets.push(T::Offset::usize_as(0));
let mut values = MutableBuffer::new(0);
@@ -640,6 +645,26 @@ mod tests {
use crate::{Array, BinaryArray, StringArray};
use arrow_buffer::{Buffer, NullBuffer, OffsetBuffer};
+ /// `from_iter_values` must work with iterators that report no upper size
bound,
+ /// and must not trust the size hint it does get.
+ #[test]
+ fn from_iter_values_untrusted_size_hint() {
+ // No upper bound at all:
+ let no_upper_bound = (0..20).filter(|i| i % 2 == 0).map(|i|
format!("v{i}"));
+ assert_eq!(no_upper_bound.size_hint(), (0, Some(20)));
+ let array = StringArray::from_iter_values(no_upper_bound);
+ assert_eq!(array.len(), 10);
+ assert_eq!(array.value(0), "v0");
+ assert_eq!(array.value(9), "v18");
+
+ // Upper bound larger than the number of yielded values:
+ let too_large_upper_bound = (0..).map(|i|
format!("v{i}")).take_while(|v| v != "v3");
+ assert_eq!(too_large_upper_bound.size_hint(), (0, None));
+ let array = StringArray::from_iter_values(too_large_upper_bound);
+ assert_eq!(array.len(), 3);
+ assert_eq!(array.value(2), "v2");
+ }
+
#[test]
fn try_new() {
let data = Buffer::from_slice_ref("helloworld");
