Skip to content

Use Vec directly in builders #7984

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 11, 2025
Merged

Use Vec directly in builders #7984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6158903
Use `Vec` directly in `generic_bytes_builder`
liamzwbao Jul 23, 2025
77f0db3
Use `Vec` in `primitive_builder`
liamzwbao Jul 24, 2025
c5dbdd6
Use `Vec` in `generic_list_builder`
liamzwbao Jul 24, 2025
6442579
Improve impl
liamzwbao Jul 26, 2025
e0e2da9
Mark the failed tests
liamzwbao Jul 27, 2025
4d72aa2
Clean up outdated comments
liamzwbao Aug 7, 2025
1d4ffc5
Merge branch 'main' into issue-7383
liamzwbao Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions arrow-array/src/builder/fixed_size_binary_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.

use crate::builder::{ArrayBuilder, UInt8BufferBuilder};
use crate::builder::ArrayBuilder;
use crate::{ArrayRef, FixedSizeBinaryArray};
use arrow_buffer::Buffer;
use arrow_buffer::NullBufferBuilder;
Expand All @@ -42,7 +42,7 @@ use std::sync::Arc;
/// ```
#[derive(Debug)]
pub struct FixedSizeBinaryBuilder {
values_builder: UInt8BufferBuilder,
values_builder: Vec<u8>,
null_buffer_builder: NullBufferBuilder,
value_length: i32,
}
Expand All @@ -61,7 +61,7 @@ impl FixedSizeBinaryBuilder {
"value length ({byte_width}) of the array must >= 0"
);
Self {
values_builder: UInt8BufferBuilder::new(capacity * byte_width as usize),
values_builder: Vec::with_capacity(capacity * byte_width as usize),
null_buffer_builder: NullBufferBuilder::new(capacity),
value_length: byte_width,
}
Expand All @@ -79,7 +79,7 @@ impl FixedSizeBinaryBuilder {
.to_string(),
))
} else {
self.values_builder.append_slice(value.as_ref());
self.values_builder.extend_from_slice(value.as_ref());
self.null_buffer_builder.append_non_null();
Ok(())
}
Expand All @@ -89,15 +89,15 @@ impl FixedSizeBinaryBuilder {
#[inline]
pub fn append_null(&mut self) {
self.values_builder
.append_slice(&vec![0u8; self.value_length as usize][..]);
.extend(std::iter::repeat_n(0u8, self.value_length as usize));
self.null_buffer_builder.append_null();
}

/// Appends `n` `null`s into the builder.
#[inline]
pub fn append_nulls(&mut self, n: usize) {
self.values_builder
.append_slice(&vec![0u8; self.value_length as usize * n][..]);
.extend(std::iter::repeat_n(0u8, self.value_length as usize * n));
self.null_buffer_builder.append_n_nulls(n);
}

Expand All @@ -110,7 +110,7 @@ impl FixedSizeBinaryBuilder {
pub fn finish(&mut self) -> FixedSizeBinaryArray {
let array_length = self.len();
let array_data_builder = ArrayData::builder(DataType::FixedSizeBinary(self.value_length))
.add_buffer(self.values_builder.finish())
.add_buffer(std::mem::take(&mut self.values_builder).into())
.nulls(self.null_buffer_builder.finish())
.len(array_length);
let array_data = unsafe { array_data_builder.build_unchecked() };
Expand Down
46 changes: 24 additions & 22 deletions arrow-array/src/builder/generic_bytes_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
// specific language governing permissions and limitations
// under the License.

use crate::builder::{ArrayBuilder, BufferBuilder, UInt8BufferBuilder};
use crate::builder::ArrayBuilder;
use crate::types::{ByteArrayType, GenericBinaryType, GenericStringType};
use crate::{Array, ArrayRef, GenericByteArray, OffsetSizeTrait};
use arrow_buffer::NullBufferBuilder;
use arrow_buffer::{ArrowNativeType, Buffer, MutableBuffer};
use arrow_buffer::{ArrowNativeType, Buffer, MutableBuffer, NullBufferBuilder, ScalarBuffer};
use arrow_data::ArrayDataBuilder;
use std::any::Any;
use std::sync::Arc;
Expand All @@ -29,8 +28,8 @@ use std::sync::Arc;
/// For building strings, see docs on [`GenericStringBuilder`].
/// For building binary, see docs on [`GenericBinaryBuilder`].
pub struct GenericByteBuilder<T: ByteArrayType> {
value_builder: UInt8BufferBuilder,
offsets_builder: BufferBuilder<T::Offset>,
value_builder: Vec<u8>,
offsets_builder: Vec<T::Offset>,
null_buffer_builder: NullBufferBuilder,
}

Expand All @@ -47,10 +46,10 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
/// - `data_capacity` is the total number of bytes of data to pre-allocate
/// (for all items, not per item).
pub fn with_capacity(item_capacity: usize, data_capacity: usize) -> Self {
let mut offsets_builder = BufferBuilder::<T::Offset>::new(item_capacity + 1);
offsets_builder.append(T::Offset::from_usize(0).unwrap());
let mut offsets_builder = Vec::with_capacity(item_capacity + 1);
offsets_builder.push(T::Offset::from_usize(0).unwrap());
Self {
value_builder: UInt8BufferBuilder::new(data_capacity),
value_builder: Vec::with_capacity(data_capacity),
offsets_builder,
null_buffer_builder: NullBufferBuilder::new(item_capacity),
}
Expand All @@ -67,8 +66,9 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
value_buffer: MutableBuffer,
null_buffer: Option<MutableBuffer>,
) -> Self {
let offsets_builder = BufferBuilder::<T::Offset>::new_from_buffer(offsets_buffer);
let value_builder = BufferBuilder::<u8>::new_from_buffer(value_buffer);
let offsets_builder: Vec<T::Offset> =
ScalarBuffer::<T::Offset>::from(offsets_buffer).into();
let value_builder: Vec<u8> = ScalarBuffer::<u8>::from(value_buffer).into();

let null_buffer_builder = null_buffer
.map(|buffer| NullBufferBuilder::new_from_buffer(buffer, offsets_builder.len() - 1))
Expand Down Expand Up @@ -103,9 +103,10 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
/// [`BinaryArray`]: crate::BinaryArray
#[inline]
pub fn append_value(&mut self, value: impl AsRef<T::Native>) {
self.value_builder.append_slice(value.as_ref().as_ref());
self.value_builder
.extend_from_slice(value.as_ref().as_ref());
self.null_buffer_builder.append(true);
self.offsets_builder.append(self.next_offset());
self.offsets_builder.push(self.next_offset());
}

/// Append an `Option` value into the builder.
Expand All @@ -126,15 +127,16 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
#[inline]
pub fn append_null(&mut self) {
self.null_buffer_builder.append(false);
self.offsets_builder.append(self.next_offset());
self.offsets_builder.push(self.next_offset());
}

/// Appends `n` `null`s into the builder.
#[inline]
pub fn append_nulls(&mut self, n: usize) {
self.null_buffer_builder.append_n_nulls(n);
let next_offset = self.next_offset();
self.offsets_builder.append_n(n, next_offset);
self.offsets_builder
.extend(std::iter::repeat_n(next_offset, n));
}

/// Appends array values and null to this builder as is
Expand All @@ -150,7 +152,7 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
// If the offsets are contiguous, we can append them directly avoiding the need to align
// for example, when the first appended array is not sliced (starts at offset 0)
if self.next_offset() == offsets[0] {
self.offsets_builder.append_slice(&offsets[1..]);
self.offsets_builder.extend_from_slice(&offsets[1..]);
} else {
// Shifting all the offsets
let shift: T::Offset = self.next_offset() - offsets[0];
Expand All @@ -164,11 +166,11 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
intermediate.push(offset + shift)
}

self.offsets_builder.append_slice(&intermediate);
self.offsets_builder.extend_from_slice(&intermediate);
}

// Append underlying values, starting from the first offset and ending at the last offset
self.value_builder.append_slice(
self.value_builder.extend_from_slice(
&array.values().as_slice()[offsets[0].as_usize()..offsets[array.len()].as_usize()],
);

Expand All @@ -184,11 +186,11 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
let array_type = T::DATA_TYPE;
let array_builder = ArrayDataBuilder::new(array_type)
.len(self.len())
.add_buffer(self.offsets_builder.finish())
.add_buffer(self.value_builder.finish())
.add_buffer(std::mem::take(&mut self.offsets_builder).into())
.add_buffer(std::mem::take(&mut self.value_builder).into())
.nulls(self.null_buffer_builder.finish());

self.offsets_builder.append(self.next_offset());
self.offsets_builder.push(self.next_offset());
let array_data = unsafe { array_builder.build_unchecked() };
GenericByteArray::from(array_data)
}
Expand Down Expand Up @@ -340,7 +342,7 @@ pub type GenericStringBuilder<O> = GenericByteBuilder<GenericStringType<O>>;

impl<O: OffsetSizeTrait> std::fmt::Write for GenericStringBuilder<O> {
fn write_str(&mut self, s: &str) -> std::fmt::Result {
self.value_builder.append_slice(s.as_bytes());
self.value_builder.extend_from_slice(s.as_bytes());
Ok(())
}
}
Expand Down Expand Up @@ -394,7 +396,7 @@ pub type GenericBinaryBuilder<O> = GenericByteBuilder<GenericBinaryType<O>>;

impl<O: OffsetSizeTrait> std::io::Write for GenericBinaryBuilder<O> {
fn write(&mut self, bs: &[u8]) -> std::io::Result<usize> {
self.value_builder.append_slice(bs);
self.value_builder.extend_from_slice(bs);
Ok(bs.len())
}

Expand Down
19 changes: 10 additions & 9 deletions arrow-array/src/builder/generic_list_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.

use crate::builder::{ArrayBuilder, BufferBuilder};
use crate::builder::ArrayBuilder;
use crate::{Array, ArrayRef, GenericListArray, OffsetSizeTrait};
use arrow_buffer::NullBufferBuilder;
use arrow_buffer::{Buffer, OffsetBuffer};
Expand Down Expand Up @@ -86,7 +86,7 @@ use std::sync::Arc;
/// [`LargeListArray`]: crate::array::LargeListArray
#[derive(Debug)]
pub struct GenericListBuilder<OffsetSize: OffsetSizeTrait, T: ArrayBuilder> {
offsets_builder: BufferBuilder<OffsetSize>,
offsets_builder: Vec<OffsetSize>,
null_buffer_builder: NullBufferBuilder,
values_builder: T,
field: Option<FieldRef>,
Expand All @@ -108,8 +108,8 @@ impl<OffsetSize: OffsetSizeTrait, T: ArrayBuilder> GenericListBuilder<OffsetSize
/// Creates a new [`GenericListBuilder`] from a given values array builder
/// `capacity` is the number of items to pre-allocate space for in this builder
pub fn with_capacity(values_builder: T, capacity: usize) -> Self {
let mut offsets_builder = BufferBuilder::<OffsetSize>::new(capacity + 1);
offsets_builder.append(OffsetSize::zero());
let mut offsets_builder = Vec::with_capacity(capacity + 1);
offsets_builder.push(OffsetSize::zero());
Self {
offsets_builder,
null_buffer_builder: NullBufferBuilder::new(capacity),
Expand Down Expand Up @@ -192,7 +192,7 @@ where
/// Panics if the length of [`Self::values`] exceeds `OffsetSize::MAX`
#[inline]
pub fn append(&mut self, is_valid: bool) {
self.offsets_builder.append(self.next_offset());
self.offsets_builder.push(self.next_offset());
self.null_buffer_builder.append(is_valid);
}

Expand Down Expand Up @@ -266,15 +266,16 @@ where
/// See [`Self::append_value`] for an example use.
#[inline]
pub fn append_null(&mut self) {
self.offsets_builder.append(self.next_offset());
self.offsets_builder.push(self.next_offset());
self.null_buffer_builder.append_null();
}

/// Appends `n` `null`s into the builder.
#[inline]
pub fn append_nulls(&mut self, n: usize) {
let next_offset = self.next_offset();
self.offsets_builder.append_n(n, next_offset);
self.offsets_builder
.extend(std::iter::repeat_n(next_offset, n));
self.null_buffer_builder.append_n_nulls(n);
}

Expand All @@ -298,10 +299,10 @@ where
let values = self.values_builder.finish();
let nulls = self.null_buffer_builder.finish();

let offsets = self.offsets_builder.finish();
let offsets = Buffer::from_vec(std::mem::take(&mut self.offsets_builder));
// Safety: Safe by construction
let offsets = unsafe { OffsetBuffer::new_unchecked(offsets.into()) };
self.offsets_builder.append(OffsetSize::zero());
self.offsets_builder.push(OffsetSize::zero());

let field = match &self.field {
Some(f) => f.clone(),
Expand Down
23 changes: 11 additions & 12 deletions arrow-array/src/builder/generic_list_view_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@

use crate::builder::ArrayBuilder;
use crate::{ArrayRef, GenericListViewArray, OffsetSizeTrait};
use arrow_buffer::{Buffer, BufferBuilder, NullBufferBuilder, ScalarBuffer};
use arrow_buffer::{Buffer, NullBufferBuilder, ScalarBuffer};
use arrow_schema::{Field, FieldRef};
use std::any::Any;
use std::sync::Arc;

/// Builder for [`GenericListViewArray`]
#[derive(Debug)]
pub struct GenericListViewBuilder<OffsetSize: OffsetSizeTrait, T: ArrayBuilder> {
offsets_builder: BufferBuilder<OffsetSize>,
sizes_builder: BufferBuilder<OffsetSize>,
offsets_builder: Vec<OffsetSize>,
sizes_builder: Vec<OffsetSize>,
null_buffer_builder: NullBufferBuilder,
values_builder: T,
field: Option<FieldRef>,
Expand Down Expand Up @@ -83,8 +83,8 @@ impl<OffsetSize: OffsetSizeTrait, T: ArrayBuilder> GenericListViewBuilder<Offset
/// Creates a new [`GenericListViewBuilder`] from a given values array builder
/// `capacity` is the number of items to pre-allocate space for in this builder
pub fn with_capacity(values_builder: T, capacity: usize) -> Self {
let offsets_builder = BufferBuilder::<OffsetSize>::new(capacity);
let sizes_builder = BufferBuilder::<OffsetSize>::new(capacity);
let offsets_builder = Vec::with_capacity(capacity);
let sizes_builder = Vec::with_capacity(capacity);
Self {
offsets_builder,
null_buffer_builder: NullBufferBuilder::new(capacity),
Expand Down Expand Up @@ -132,8 +132,8 @@ where
/// Panics if the length of [`Self::values`] exceeds `OffsetSize::MAX`
#[inline]
pub fn append(&mut self, is_valid: bool) {
self.offsets_builder.append(self.current_offset);
self.sizes_builder.append(
self.offsets_builder.push(self.current_offset);
self.sizes_builder.push(
OffsetSize::from_usize(
self.values_builder.len() - self.current_offset.to_usize().unwrap(),
)
Expand All @@ -158,9 +158,8 @@ where
/// See [`Self::append_value`] for an example use.
#[inline]
pub fn append_null(&mut self) {
self.offsets_builder.append(self.current_offset);
self.sizes_builder
.append(OffsetSize::from_usize(0).unwrap());
self.offsets_builder.push(self.current_offset);
self.sizes_builder.push(OffsetSize::from_usize(0).unwrap());
self.null_buffer_builder.append_null();
}

Expand All @@ -183,12 +182,12 @@ where
pub fn finish(&mut self) -> GenericListViewArray<OffsetSize> {
let values = self.values_builder.finish();
let nulls = self.null_buffer_builder.finish();
let offsets = self.offsets_builder.finish();
let offsets = Buffer::from_vec(std::mem::take(&mut self.offsets_builder));
self.current_offset = OffsetSize::zero();

// Safety: Safe by construction
let offsets = ScalarBuffer::from(offsets);
let sizes = self.sizes_builder.finish();
let sizes = Buffer::from_vec(std::mem::take(&mut self.sizes_builder));
let sizes = ScalarBuffer::from(sizes);
let field = match &self.field {
Some(f) => f.clone(),
Expand Down
14 changes: 7 additions & 7 deletions arrow-array/src/builder/map_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.

use crate::builder::{ArrayBuilder, BufferBuilder};
use crate::builder::ArrayBuilder;
use crate::{Array, ArrayRef, MapArray, StructArray};
use arrow_buffer::Buffer;
use arrow_buffer::{NullBuffer, NullBufferBuilder};
Expand Down Expand Up @@ -56,7 +56,7 @@ use std::sync::Arc;
/// ```
#[derive(Debug)]
pub struct MapBuilder<K: ArrayBuilder, V: ArrayBuilder> {
offsets_builder: BufferBuilder<i32>,
offsets_builder: Vec<i32>,
null_buffer_builder: NullBufferBuilder,
field_names: MapFieldNames,
key_builder: K,
Expand Down Expand Up @@ -100,8 +100,8 @@ impl<K: ArrayBuilder, V: ArrayBuilder> MapBuilder<K, V> {
value_builder: V,
capacity: usize,
) -> Self {
let mut offsets_builder = BufferBuilder::<i32>::new(capacity + 1);
offsets_builder.append(0);
let mut offsets_builder = Vec::with_capacity(capacity + 1);
offsets_builder.push(0);
Self {
offsets_builder,
null_buffer_builder: NullBufferBuilder::new(capacity),
Expand Down Expand Up @@ -166,7 +166,7 @@ impl<K: ArrayBuilder, V: ArrayBuilder> MapBuilder<K, V> {
self.value_builder.len()
)));
}
self.offsets_builder.append(self.key_builder.len() as i32);
self.offsets_builder.push(self.key_builder.len() as i32);
self.null_buffer_builder.append(is_valid);
Ok(())
}
Expand All @@ -177,8 +177,8 @@ impl<K: ArrayBuilder, V: ArrayBuilder> MapBuilder<K, V> {
// Build the keys
let keys_arr = self.key_builder.finish();
let values_arr = self.value_builder.finish();
let offset_buffer = self.offsets_builder.finish();
self.offsets_builder.append(0);
let offset_buffer = Buffer::from_vec(std::mem::take(&mut self.offsets_builder));
self.offsets_builder.push(0);
let null_bit_buffer = self.null_buffer_builder.finish();

self.finish_helper(keys_arr, values_arr, offset_buffer, null_bit_buffer, len)
Expand Down
Loading
Loading