From 04808c908ffd4b990eda48787537462fdc7f87bb Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Fri, 25 Dec 2020 15:24:07 +0000 Subject: [PATCH 1/5] Removed unused trait and Result. --- rust/arrow/examples/tensor_builder.rs | 2 +- rust/arrow/src/array/builder.rs | 227 +++++++++++--------------- rust/arrow/src/array/mod.rs | 21 ++- rust/arrow/src/tensor.rs | 16 +- 4 files changed, 111 insertions(+), 155 deletions(-) diff --git a/rust/arrow/examples/tensor_builder.rs b/rust/arrow/examples/tensor_builder.rs index 19421715ceb..923ffdb30a6 100644 --- a/rust/arrow/examples/tensor_builder.rs +++ b/rust/arrow/examples/tensor_builder.rs @@ -18,7 +18,7 @@ ///! Tensor builder example extern crate arrow; -use arrow::array::*; //{BufferBuilderTrait, Int32BufferBuilder, Float32BufferBuilder}; +use arrow::array::*; //{Int32BufferBuilder, Float32BufferBuilder}; use arrow::buffer::Buffer; use arrow::datatypes::ToByteSlice; use arrow::error::Result; diff --git a/rust/arrow/src/array/builder.rs b/rust/arrow/src/array/builder.rs index 3526c03c50c..d86fc40808e 100644 --- a/rust/arrow/src/array/builder.rs +++ b/rust/arrow/src/array/builder.rs @@ -36,7 +36,7 @@ use crate::util::bit_util; /// Converts a `MutableBuffer` to a `BufferBuilder`. /// /// `slots` is the number of array slots currently represented in the `MutableBuffer`. -pub(crate) fn mutable_buffer_to_builder( +pub(crate) fn mutable_buffer_to_builder( mutable_buffer: MutableBuffer, slots: usize, ) -> BufferBuilder { @@ -50,7 +50,7 @@ pub(crate) fn mutable_buffer_to_builder( /// Converts a `BufferBuilder` into it's underlying `MutableBuffer`. /// /// `From` is not implemented because associated type bounds are unstable. -pub(crate) fn builder_to_mutable_buffer( +pub(crate) fn builder_to_mutable_buffer( builder: BufferBuilder, ) -> MutableBuffer { builder.buffer @@ -58,12 +58,6 @@ pub(crate) fn builder_to_mutable_buffer( /// Builder for creating a [`Buffer`](crate::buffer::Buffer) object. /// -/// This builder is implemented for primitive types and creates a -/// buffer with a zero-copy `build()` method. -/// -/// See trait [`BufferBuilderTrait`](crate::array::BufferBuilderTrait) -/// for further documentation and examples. -/// /// A [`Buffer`](crate::buffer::Buffer) is the underlying data /// structure of Arrow's [`Arrays`](crate::array::Array). /// @@ -86,20 +80,13 @@ pub(crate) fn builder_to_mutable_buffer( /// # } /// ``` #[derive(Debug)] -pub struct BufferBuilder { +pub struct BufferBuilder { buffer: MutableBuffer, len: usize, _marker: PhantomData, } -/// Trait for simplifying the construction of [`Buffers`](crate::buffer::Buffer). -/// -/// This trait is used mainly to offer separate implementations for -/// numeric types and boolean types, while still be able to call methods on buffer builder -/// with generic primitive type. -/// Separate implementations of this trait allow to add implementation-details, -/// e.g. the implementation for boolean types uses bit-packing. -pub trait BufferBuilderTrait { +impl BufferBuilder { /// Creates a new builder with initial capacity for _at least_ `capacity` /// elements of type `T`. /// @@ -120,7 +107,16 @@ pub trait BufferBuilderTrait { /// /// assert!(builder.capacity() >= 10); /// ``` - fn new(capacity: usize) -> Self; + #[inline] + pub fn new(capacity: usize) -> Self { + let buffer = MutableBuffer::new(capacity * mem::size_of::()); + + Self { + buffer, + len: 0, + _marker: PhantomData, + } + } /// Returns the current number of array elements in the internal buffer. /// @@ -134,7 +130,9 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(builder.len(), 1); /// ``` - fn len(&self) -> usize; + pub fn len(&self) -> usize { + self.len + } /// Returns whether the internal buffer is empty. /// @@ -148,14 +146,19 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(builder.is_empty(), false); /// ``` - fn is_empty(&self) -> bool; + pub fn is_empty(&self) -> bool { + self.len == 0 + } /// Returns the actual capacity (number of elements) of the internal buffer. /// /// Note: the internal capacity returned by this method might be larger than /// what you'd expect after setting the capacity in the `new()` or `reserve()` /// functions. - fn capacity(&self) -> usize; + pub fn capacity(&self) -> usize { + let byte_capacity = self.buffer.capacity(); + byte_capacity / std::mem::size_of::() + } /// Increases the number of elements in the internal buffer by `n` /// and resizes the buffer as needed. @@ -174,7 +177,12 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(builder.len(), 2); /// ``` - fn advance(&mut self, n: usize) -> Result<()>; + #[inline] + pub fn advance(&mut self, i: usize) { + let new_buffer_len = (self.len + i) * mem::size_of::(); + self.buffer.resize(new_buffer_len); + self.len += i; + } /// Reserves memory for _at least_ `n` more elements of type `T`. /// @@ -188,7 +196,12 @@ pub trait BufferBuilderTrait { /// /// assert!(builder.capacity() >= 20); /// ``` - fn reserve(&mut self, n: usize); + #[inline] + pub fn reserve(&mut self, n: usize) { + let new_capacity = self.len + n; + let byte_capacity = mem::size_of::() * new_capacity; + self.buffer.reserve(byte_capacity); + } /// Appends a value of type `T` into the builder, /// growing the internal buffer as needed. @@ -203,7 +216,16 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(builder.len(), 1); /// ``` - fn append(&mut self, value: T::Native) -> Result<()>; + #[inline] + pub fn append(&mut self, v: T) { + self.reserve(1); + self.write_bytes(v.to_byte_slice(), 1); + } + + fn write_bytes(&mut self, bytes: &[u8], len_added: usize) { + self.buffer.extend_from_slice(bytes); + self.len += len_added; + } /// Appends a value of type `T` into the builder N times, /// growing the internal buffer as needed. @@ -218,7 +240,13 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(builder.len(), 10); /// ``` - fn append_n(&mut self, n: usize, value: T::Native) -> Result<()>; + #[inline] + pub fn append_n(&mut self, n: usize, v: T) { + self.reserve(n); + for _ in 0..n { + self.write_bytes(v.to_byte_slice(), 1); + } + } /// Appends a slice of type `T`, growing the internal buffer as needed. /// @@ -232,7 +260,13 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(builder.len(), 3); /// ``` - fn append_slice(&mut self, slice: &[T::Native]) -> Result<()>; + #[inline] + pub fn append_slice(&mut self, slice: &[T]) { + let array_slots = slice.len(); + self.reserve(array_slots); + + self.write_bytes(slice.to_byte_slice(), array_slots); + } /// Resets this builder and returns an immutable [`Buffer`](crate::buffer::Buffer). /// @@ -248,76 +282,8 @@ pub trait BufferBuilderTrait { /// /// assert_eq!(unsafe { buffer.typed_data::() }, &[42, 44, 46]); /// ``` - fn finish(&mut self) -> Buffer; -} - -impl BufferBuilderTrait for BufferBuilder { - #[inline] - fn new(capacity: usize) -> Self { - let buffer = MutableBuffer::new(capacity * mem::size_of::()); - - Self { - buffer, - len: 0, - _marker: PhantomData, - } - } - - fn len(&self) -> usize { - self.len - } - - fn is_empty(&self) -> bool { - self.len == 0 - } - - fn capacity(&self) -> usize { - let byte_capacity = self.buffer.capacity(); - byte_capacity / T::get_byte_width() - } - - #[inline] - fn advance(&mut self, i: usize) -> Result<()> { - let new_buffer_len = (self.len + i) * mem::size_of::(); - self.buffer.resize(new_buffer_len); - self.len += i; - Ok(()) - } - - #[inline] - fn reserve(&mut self, n: usize) { - let new_capacity = self.len + n; - let byte_capacity = mem::size_of::() * new_capacity; - self.buffer.reserve(byte_capacity); - } - - #[inline] - fn append(&mut self, v: T::Native) -> Result<()> { - self.reserve(1); - self.write_bytes(v.to_byte_slice(), 1); - Ok(()) - } - - #[inline] - fn append_n(&mut self, n: usize, v: T::Native) -> Result<()> { - self.reserve(n); - for _ in 0..n { - self.write_bytes(v.to_byte_slice(), 1); - } - Ok(()) - } - - #[inline] - fn append_slice(&mut self, slice: &[T::Native]) -> Result<()> { - let array_slots = slice.len(); - self.reserve(array_slots); - - self.write_bytes(slice.to_byte_slice(), array_slots); - Ok(()) - } - #[inline] - fn finish(&mut self) -> Buffer { + pub fn finish(&mut self) -> Buffer { let buf = std::mem::replace(&mut self.buffer, MutableBuffer::new(0)); self.len = 0; buf.freeze() @@ -436,16 +402,6 @@ impl BooleanBufferBuilder { } } -impl BufferBuilder { - /// Writes a byte slice to the underlying buffer and updates the `len`, i.e. the - /// number array elements in the builder. Also, converts the `io::Result` - /// required by the `Write` trait to the Arrow `Result` type. - fn write_bytes(&mut self, bytes: &[u8], len_added: usize) { - self.buffer.extend_from_slice(bytes); - self.len += len_added; - } -} - /// Trait for dealing with different array builders at runtime pub trait ArrayBuilder: Any { /// Returns the number of array slots in the builder @@ -590,7 +546,7 @@ impl ArrayBuilder for BooleanBuilder { /// Array builder for fixed-width primitive types #[derive(Debug)] pub struct PrimitiveBuilder { - values_builder: BufferBuilder, + values_builder: BufferBuilder, bitmap_builder: BooleanBufferBuilder, } @@ -630,7 +586,7 @@ impl PrimitiveBuilder { /// Creates a new primitive array builder pub fn new(capacity: usize) -> Self { Self { - values_builder: BufferBuilder::::new(capacity), + values_builder: BufferBuilder::::new(capacity), bitmap_builder: BooleanBufferBuilder::new(capacity), } } @@ -643,14 +599,14 @@ impl PrimitiveBuilder { /// Appends a value of type `T` into the builder pub fn append_value(&mut self, v: T::Native) -> Result<()> { self.bitmap_builder.append(true)?; - self.values_builder.append(v)?; + self.values_builder.append(v); Ok(()) } /// Appends a null slot into the builder pub fn append_null(&mut self) -> Result<()> { self.bitmap_builder.append(false)?; - self.values_builder.advance(1)?; + self.values_builder.advance(1); Ok(()) } @@ -666,7 +622,7 @@ impl PrimitiveBuilder { /// Appends a slice of type `T` into the builder pub fn append_slice(&mut self, v: &[T::Native]) -> Result<()> { self.bitmap_builder.append_n(v.len(), true)?; - self.values_builder.append_slice(v)?; + self.values_builder.append_slice(v); Ok(()) } @@ -682,7 +638,8 @@ impl PrimitiveBuilder { )); } self.bitmap_builder.append_slice(is_valid)?; - self.values_builder.append_slice(values) + self.values_builder.append_slice(values); + Ok(()) } /// Builds the `PrimitiveArray` and reset this builder. @@ -727,7 +684,7 @@ impl PrimitiveBuilder { /// Array builder for `ListArray` #[derive(Debug)] pub struct ListBuilder { - offsets_builder: Int32BufferBuilder, + offsets_builder: BufferBuilder, bitmap_builder: BooleanBufferBuilder, values_builder: T, len: usize, @@ -744,7 +701,7 @@ impl ListBuilder { /// `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 = Int32BufferBuilder::new(capacity + 1); - offsets_builder.append(0).unwrap(); + offsets_builder.append(0); Self { offsets_builder, bitmap_builder: BooleanBufferBuilder::new(capacity), @@ -804,7 +761,7 @@ where /// Finish the current variable-length list array slot pub fn append(&mut self, is_valid: bool) -> Result<()> { self.offsets_builder - .append(self.values_builder.len() as i32)?; + .append(self.values_builder.len() as i32); self.bitmap_builder.append(is_valid)?; self.len += 1; Ok(()) @@ -825,7 +782,7 @@ where let offset_buffer = self.offsets_builder.finish(); let null_bit_buffer = self.bitmap_builder.finish(); let nulls = null_bit_buffer.count_set_bits(); - self.offsets_builder.append(0).unwrap(); + self.offsets_builder.append(0); let data = ArrayData::builder(DataType::List(Box::new(Field::new( "item", values_data.data_type().clone(), @@ -845,7 +802,7 @@ where /// Array builder for `ListArray` #[derive(Debug)] pub struct LargeListBuilder { - offsets_builder: Int64BufferBuilder, + offsets_builder: BufferBuilder, bitmap_builder: BooleanBufferBuilder, values_builder: T, len: usize, @@ -862,7 +819,7 @@ impl LargeListBuilder { /// `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 = Int64BufferBuilder::new(capacity + 1); - offsets_builder.append(0).unwrap(); + offsets_builder.append(0); Self { offsets_builder, bitmap_builder: BooleanBufferBuilder::new(capacity), @@ -922,7 +879,7 @@ where /// Finish the current variable-length list array slot pub fn append(&mut self, is_valid: bool) -> Result<()> { self.offsets_builder - .append(self.values_builder.len() as i64)?; + .append(self.values_builder.len() as i64); self.bitmap_builder.append(is_valid)?; self.len += 1; Ok(()) @@ -943,7 +900,7 @@ where let offset_buffer = self.offsets_builder.finish(); let null_bit_buffer = self.bitmap_builder.finish(); let nulls = null_bit_buffer.count_set_bits(); - self.offsets_builder.append(0).unwrap(); + self.offsets_builder.append(0); let data = ArrayData::builder(DataType::LargeList(Box::new(Field::new( "item", values_data.data_type().clone(), @@ -982,7 +939,7 @@ impl FixedSizeListBuilder { /// `capacity` is the number of items to pre-allocate space for in this builder pub fn with_capacity(values_builder: T, length: i32, capacity: usize) -> Self { let mut offsets_builder = Int32BufferBuilder::new(capacity + 1); - offsets_builder.append(0).unwrap(); + offsets_builder.append(0); Self { bitmap_builder: BooleanBufferBuilder::new(capacity), values_builder, @@ -1871,9 +1828,9 @@ impl FieldData { .values_buffer .take() .expect("Values buffer was never created"); - let mut builder: BufferBuilder = + let mut builder: BufferBuilder = mutable_buffer_to_builder(values_buffer, self.slots); - builder.append(v)?; + builder.append(v); let mutable_buffer = builder_to_mutable_buffer(builder); self.values_buffer = Some(mutable_buffer); @@ -1891,9 +1848,9 @@ impl FieldData { .values_buffer .take() .expect("Values buffer was never created"); - let mut builder: BufferBuilder = + let mut builder: BufferBuilder = mutable_buffer_to_builder(values_buffer, self.slots); - builder.advance(1)?; + builder.advance(1); let mutable_buffer = builder_to_mutable_buffer(builder); self.values_buffer = Some(mutable_buffer); self.slots += 1; @@ -1993,7 +1950,7 @@ impl UnionBuilder { .expect("Cannot be None") .append(false)?; - self.type_id_builder.append(i8::default())?; + self.type_id_builder.append(i8::default()); // Handle sparse union if self.value_offset_builder.is_none() { @@ -2030,12 +1987,12 @@ impl UnionBuilder { } }, }; - self.type_id_builder.append(field_data.type_id)?; + self.type_id_builder.append(field_data.type_id); match &mut self.value_offset_builder { // Dense Union Some(offset_builder) => { - offset_builder.append(field_data.slots as i32)?; + offset_builder.append(field_data.slots as i32); } // Sparse Union None => { @@ -2373,7 +2330,7 @@ mod tests { #[test] fn test_builder_i32_alloc_zero_bytes() { let mut b = Int32BufferBuilder::new(0); - b.append(123).unwrap(); + b.append(123); let a = b.finish(); assert_eq!(4, a.len()); } @@ -2382,7 +2339,7 @@ mod tests { fn test_builder_i32() { let mut b = Int32BufferBuilder::new(5); for i in 0..5 { - b.append(i).unwrap(); + b.append(i); } assert_eq!(16, b.capacity()); let a = b.finish(); @@ -2394,7 +2351,7 @@ mod tests { let mut b = Int32BufferBuilder::new(2); assert_eq!(16, b.capacity()); for i in 0..20 { - b.append(i).unwrap(); + b.append(i); } assert_eq!(32, b.capacity()); let a = b.finish(); @@ -2406,7 +2363,7 @@ mod tests { let mut b = Int32BufferBuilder::new(5); assert_eq!(16, b.capacity()); for i in 0..10 { - b.append(i).unwrap(); + b.append(i); } let mut a = b.finish(); assert_eq!(40, a.len()); @@ -2415,7 +2372,7 @@ mod tests { // Try build another buffer after cleaning up. for i in 0..20 { - b.append(i).unwrap() + b.append(i) } assert_eq!(32, b.capacity()); a = b.finish(); @@ -2442,13 +2399,13 @@ mod tests { #[test] fn test_append_slice() { let mut b = UInt8BufferBuilder::new(0); - b.append_slice(b"Hello, ").unwrap(); - b.append_slice(b"World!").unwrap(); + b.append_slice(b"Hello, "); + b.append_slice(b"World!"); let buffer = b.finish(); assert_eq!(13, buffer.len()); let mut b = Int32BufferBuilder::new(0); - b.append_slice(&[32, 54]).unwrap(); + b.append_slice(&[32, 54]); let buffer = b.finish(); assert_eq!(8, buffer.len()); } diff --git a/rust/arrow/src/array/mod.rs b/rust/arrow/src/array/mod.rs index 4cd83e309c0..298fc632997 100644 --- a/rust/arrow/src/array/mod.rs +++ b/rust/arrow/src/array/mod.rs @@ -179,18 +179,17 @@ pub use self::array_string::StringOffsetSizeTrait; pub use self::builder::BooleanBufferBuilder; pub use self::builder::BufferBuilder; -pub use self::builder::BufferBuilderTrait; -pub type Int8BufferBuilder = BufferBuilder; -pub type Int16BufferBuilder = BufferBuilder; -pub type Int32BufferBuilder = BufferBuilder; -pub type Int64BufferBuilder = BufferBuilder; -pub type UInt8BufferBuilder = BufferBuilder; -pub type UInt16BufferBuilder = BufferBuilder; -pub type UInt32BufferBuilder = BufferBuilder; -pub type UInt64BufferBuilder = BufferBuilder; -pub type Float32BufferBuilder = BufferBuilder; -pub type Float64BufferBuilder = BufferBuilder; +pub type Int8BufferBuilder = BufferBuilder; +pub type Int16BufferBuilder = BufferBuilder; +pub type Int32BufferBuilder = BufferBuilder; +pub type Int64BufferBuilder = BufferBuilder; +pub type UInt8BufferBuilder = BufferBuilder; +pub type UInt16BufferBuilder = BufferBuilder; +pub type UInt32BufferBuilder = BufferBuilder; +pub type UInt64BufferBuilder = BufferBuilder; +pub type Float32BufferBuilder = BufferBuilder; +pub type Float64BufferBuilder = BufferBuilder; pub type TimestampSecondBufferBuilder = BufferBuilder; pub type TimestampMillisecondBufferBuilder = BufferBuilder; diff --git a/rust/arrow/src/tensor.rs b/rust/arrow/src/tensor.rs index 92c5cfee7b1..35e45a25c38 100644 --- a/rust/arrow/src/tensor.rs +++ b/rust/arrow/src/tensor.rs @@ -359,7 +359,7 @@ mod tests { fn test_tensor() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); let tensor = Int32Tensor::try_new(buf, Some(vec![2, 8]), None, None).unwrap(); @@ -374,7 +374,7 @@ mod tests { fn test_new_row_major() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); let tensor = Int32Tensor::new_row_major(buf, Some(vec![2, 8]), None).unwrap(); @@ -392,7 +392,7 @@ mod tests { fn test_new_column_major() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); let tensor = Int32Tensor::new_column_major(buf, Some(vec![2, 8]), None).unwrap(); @@ -410,7 +410,7 @@ mod tests { fn test_with_names() { let mut builder = Int64BufferBuilder::new(8); for i in 0..8 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); let names = vec!["Dim 1", "Dim 2"]; @@ -431,7 +431,7 @@ mod tests { fn test_inconsistent_strides() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); @@ -447,7 +447,7 @@ mod tests { fn test_inconsistent_names() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); @@ -467,7 +467,7 @@ mod tests { fn test_incorrect_shape() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); @@ -482,7 +482,7 @@ mod tests { fn test_incorrect_stride() { let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); From d01232ddfdb23bbbff755ecd98d1939b8f763725 Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Sat, 26 Dec 2020 06:30:54 +0000 Subject: [PATCH 2/5] Removed result. --- rust/arrow/src/array/builder.rs | 69 ++++++++++---------- rust/arrow/src/compute/kernels/comparison.rs | 20 +++--- 2 files changed, 43 insertions(+), 46 deletions(-) diff --git a/rust/arrow/src/array/builder.rs b/rust/arrow/src/array/builder.rs index d86fc40808e..f415bd12f30 100644 --- a/rust/arrow/src/array/builder.rs +++ b/rust/arrow/src/array/builder.rs @@ -320,11 +320,10 @@ impl BooleanBufferBuilder { } #[inline] - pub fn advance(&mut self, i: usize) -> Result<()> { + pub fn advance(&mut self, i: usize) { let new_buffer_len = bit_util::ceil(self.len + i, 8); self.buffer.resize(new_buffer_len); self.len += i; - Ok(()) } #[inline] @@ -340,7 +339,7 @@ impl BooleanBufferBuilder { } #[inline] - pub fn append(&mut self, v: bool) -> Result<()> { + pub fn append(&mut self, v: bool) { self.reserve(1); if v { let data = unsafe { @@ -352,11 +351,10 @@ impl BooleanBufferBuilder { bit_util::set_bit(data, self.len); } self.len += 1; - Ok(()) } #[inline] - pub fn append_n(&mut self, n: usize, v: bool) -> Result<()> { + pub fn append_n(&mut self, n: usize, v: bool) { self.reserve(n); if n != 0 && v { let data = unsafe { @@ -368,11 +366,10 @@ impl BooleanBufferBuilder { (self.len..self.len + n).for_each(|i| bit_util::set_bit(data, i)) } self.len += n; - Ok(()) } #[inline] - pub fn append_slice(&mut self, slice: &[bool]) -> Result<()> { + pub fn append_slice(&mut self, slice: &[bool]) { let array_slots = slice.len(); self.reserve(array_slots); @@ -387,7 +384,6 @@ impl BooleanBufferBuilder { } self.len += 1; } - Ok(()) } #[inline] @@ -454,15 +450,15 @@ impl BooleanBuilder { /// Appends a value of type `T` into the builder pub fn append_value(&mut self, v: bool) -> Result<()> { - self.bitmap_builder.append(true)?; - self.values_builder.append(v)?; + self.bitmap_builder.append(true); + self.values_builder.append(v); Ok(()) } /// Appends a null slot into the builder pub fn append_null(&mut self) -> Result<()> { - self.bitmap_builder.append(false)?; - self.values_builder.advance(1)?; + self.bitmap_builder.append(false); + self.values_builder.advance(1); Ok(()) } @@ -477,8 +473,8 @@ impl BooleanBuilder { /// Appends a slice of type `T` into the builder pub fn append_slice(&mut self, v: &[bool]) -> Result<()> { - self.bitmap_builder.append_n(v.len(), true)?; - self.values_builder.append_slice(v)?; + self.bitmap_builder.append_n(v.len(), true); + self.values_builder.append_slice(v); Ok(()) } @@ -489,8 +485,9 @@ impl BooleanBuilder { "Value and validity lengths must be equal".to_string(), )); } - self.bitmap_builder.append_slice(is_valid)?; - self.values_builder.append_slice(values) + self.bitmap_builder.append_slice(is_valid); + self.values_builder.append_slice(values); + Ok(()) } /// Builds the [BooleanArray] and reset this builder. @@ -598,14 +595,14 @@ impl PrimitiveBuilder { /// Appends a value of type `T` into the builder pub fn append_value(&mut self, v: T::Native) -> Result<()> { - self.bitmap_builder.append(true)?; + self.bitmap_builder.append(true); self.values_builder.append(v); Ok(()) } /// Appends a null slot into the builder pub fn append_null(&mut self) -> Result<()> { - self.bitmap_builder.append(false)?; + self.bitmap_builder.append(false); self.values_builder.advance(1); Ok(()) } @@ -621,7 +618,7 @@ impl PrimitiveBuilder { /// Appends a slice of type `T` into the builder pub fn append_slice(&mut self, v: &[T::Native]) -> Result<()> { - self.bitmap_builder.append_n(v.len(), true)?; + self.bitmap_builder.append_n(v.len(), true); self.values_builder.append_slice(v); Ok(()) } @@ -637,7 +634,7 @@ impl PrimitiveBuilder { "Value and validity lengths must be equal".to_string(), )); } - self.bitmap_builder.append_slice(is_valid)?; + self.bitmap_builder.append_slice(is_valid); self.values_builder.append_slice(values); Ok(()) } @@ -762,7 +759,7 @@ where pub fn append(&mut self, is_valid: bool) -> Result<()> { self.offsets_builder .append(self.values_builder.len() as i32); - self.bitmap_builder.append(is_valid)?; + self.bitmap_builder.append(is_valid); self.len += 1; Ok(()) } @@ -880,7 +877,7 @@ where pub fn append(&mut self, is_valid: bool) -> Result<()> { self.offsets_builder .append(self.values_builder.len() as i64); - self.bitmap_builder.append(is_valid)?; + self.bitmap_builder.append(is_valid); self.len += 1; Ok(()) } @@ -1002,7 +999,7 @@ where /// Finish the current variable-length list array slot pub fn append(&mut self, is_valid: bool) -> Result<()> { - self.bitmap_builder.append(is_valid)?; + self.bitmap_builder.append(is_valid); self.len += 1; Ok(()) } @@ -1742,7 +1739,7 @@ impl StructBuilder { /// Appends an element (either null or non-null) to the struct. The actual elements /// should be appended for each child sub-array in a consistent way. pub fn append(&mut self, is_valid: bool) -> Result<()> { - self.bitmap_builder.append(is_valid)?; + self.bitmap_builder.append(is_valid); self.len += 1; Ok(()) } @@ -1836,7 +1833,7 @@ impl FieldData { self.slots += 1; if let Some(b) = &mut self.bitmap_builder { - b.append(true)? + b.append(true) }; Ok(()) } @@ -1855,7 +1852,7 @@ impl FieldData { self.values_buffer = Some(mutable_buffer); self.slots += 1; self.null_count += 1; - b.append(false)?; + b.append(false); }; Ok(()) } @@ -1941,14 +1938,14 @@ impl UnionBuilder { if self.bitmap_builder.is_none() { let mut builder = BooleanBufferBuilder::new(self.len + 1); for _ in 0..self.len { - builder.append(true)?; + builder.append(true); } self.bitmap_builder = Some(builder) } self.bitmap_builder .as_mut() .expect("Cannot be None") - .append(false)?; + .append(false); self.type_id_builder.append(i8::default()); @@ -2008,7 +2005,7 @@ impl UnionBuilder { // Update the bitmap builder if it exists if let Some(b) = &mut self.bitmap_builder { - b.append(true)?; + b.append(true); } self.len += 1; Ok(()) @@ -2439,17 +2436,17 @@ mod tests { #[test] fn test_write_bytes() { let mut b = BooleanBufferBuilder::new(4); - b.append(false).unwrap(); - b.append(true).unwrap(); - b.append(false).unwrap(); - b.append(true).unwrap(); + b.append(false); + b.append(true); + b.append(false); + b.append(true); assert_eq!(4, b.len()); assert_eq!(512, b.capacity()); let buffer = b.finish(); assert_eq!(1, buffer.len()); let mut b = BooleanBufferBuilder::new(4); - b.append_slice(&[false, true, false, true]).unwrap(); + b.append_slice(&[false, true, false, true]); assert_eq!(4, b.len()); assert_eq!(512, b.capacity()); let buffer = b.finish(); @@ -2499,9 +2496,9 @@ mod tests { for i in 0..10 { if i == 3 || i == 6 || i == 9 { - builder.append(true).unwrap(); + builder.append(true); } else { - builder.append(false).unwrap(); + builder.append(false); } } let buf2 = builder.finish(); diff --git a/rust/arrow/src/compute/kernels/comparison.rs b/rust/arrow/src/compute/kernels/comparison.rs index 35f93304371..f77ce57f76c 100644 --- a/rust/arrow/src/compute/kernels/comparison.rs +++ b/rust/arrow/src/compute/kernels/comparison.rs @@ -163,7 +163,7 @@ pub fn like_utf8(left: &StringArray, right: &StringArray) -> Result Result if !right.contains(is_like_pattern) { // fast path, can use equals for i in 0..left.len() { - result.append(left.value(i) == right)?; + result.append(left.value(i) == right); } } else if right.ends_with('%') && !right[..right.len() - 1].contains(is_like_pattern) { // fast path, can use starts_with for i in 0..left.len() { - result.append(left.value(i).starts_with(&right[..right.len() - 1]))?; + result.append(left.value(i).starts_with(&right[..right.len() - 1])); } } else if right.starts_with('%') && !right[1..].contains(is_like_pattern) { // fast path, can use ends_with for i in 0..left.len() { - result.append(left.value(i).ends_with(&right[1..]))?; + result.append(left.value(i).ends_with(&right[1..])); } } else { let re_pattern = right.replace("%", ".*").replace("_", "."); @@ -213,7 +213,7 @@ pub fn like_utf8_scalar(left: &StringArray, right: &str) -> Result for i in 0..left.len() { let haystack = left.value(i); - result.append(re.is_match(haystack))?; + result.append(re.is_match(haystack)); } }; @@ -259,7 +259,7 @@ pub fn nlike_utf8(left: &StringArray, right: &StringArray) -> Result Result Result Date: Sat, 26 Dec 2020 06:24:22 +0000 Subject: [PATCH 3/5] Migrated parquet. --- rust/parquet/src/arrow/array_reader.rs | 14 ++++++------ rust/parquet/src/arrow/record_reader.rs | 29 ++++++++++--------------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/rust/parquet/src/arrow/array_reader.rs b/rust/parquet/src/arrow/array_reader.rs index f4d2a79fd8d..eb9548a8622 100644 --- a/rust/parquet/src/arrow/array_reader.rs +++ b/rust/parquet/src/arrow/array_reader.rs @@ -25,10 +25,10 @@ use std::vec::Vec; use arrow::array::{ Array, ArrayData, ArrayDataBuilder, ArrayDataRef, ArrayRef, BinaryArray, - BinaryBuilder, BooleanArray, BooleanBufferBuilder, BufferBuilderTrait, - FixedSizeBinaryArray, FixedSizeBinaryBuilder, GenericListArray, Int16BufferBuilder, - ListBuilder, OffsetSizeTrait, PrimitiveArray, PrimitiveBuilder, StringArray, - StringBuilder, StructArray, + BinaryBuilder, BooleanArray, BooleanBufferBuilder, FixedSizeBinaryArray, + FixedSizeBinaryBuilder, GenericListArray, Int16BufferBuilder, ListBuilder, + OffsetSizeTrait, PrimitiveArray, PrimitiveBuilder, StringArray, StringBuilder, + StructArray, }; use arrow::buffer::{Buffer, MutableBuffer}; use arrow::datatypes::{ @@ -292,7 +292,7 @@ impl ArrayReader for PrimitiveArrayReader { let mut boolean_buffer = BooleanBufferBuilder::new(record_data.len()); for e in record_data.data() { - boolean_buffer.append(*e > 0)?; + boolean_buffer.append(*e > 0); } record_data = boolean_buffer.finish(); } @@ -1082,7 +1082,7 @@ impl ArrayReader for StructArrayReader { if !not_null { null_count += 1; } - bitmap_builder.append(not_null)?; + bitmap_builder.append(not_null); } // Now we can build array data @@ -1110,7 +1110,7 @@ impl ArrayReader for StructArrayReader { .get_rep_levels() .map(|data| -> Result { let mut buffer = Int16BufferBuilder::new(children_array_len); - buffer.append_slice(data)?; + buffer.append_slice(data); Ok(buffer.finish()) }) .transpose()?; diff --git a/rust/parquet/src/arrow/record_reader.rs b/rust/parquet/src/arrow/record_reader.rs index 14908de6d43..0a7c17fdcfb 100644 --- a/rust/parquet/src/arrow/record_reader.rs +++ b/rust/parquet/src/arrow/record_reader.rs @@ -249,7 +249,7 @@ impl RecordReader { self.null_bitmap .as_mut() .unwrap() - .append(old_bitmap.is_set(i))?; + .append(old_bitmap.is_set(i)); } Ok(Some(old_bitmap.into_buffer())) @@ -357,9 +357,8 @@ impl RecordReader { "Definition levels should exist when data is less than levels!" ) })?; - (0..levels_read).try_for_each(|idx| { - null_buffer.append(def_levels[idx] == max_def_level) - })?; + (0..levels_read) + .for_each(|idx| null_buffer.append(def_levels[idx] == max_def_level)); } let values_read = max(values_read, levels_read); @@ -439,9 +438,7 @@ mod tests { use crate::schema::parser::parse_message_type; use crate::schema::types::SchemaDescriptor; use crate::util::test_common::page_util::{DataPageBuilder, DataPageBuilderImpl}; - use arrow::array::{ - BooleanBufferBuilder, BufferBuilderTrait, Int16BufferBuilder, Int32BufferBuilder, - }; + use arrow::array::{BooleanBufferBuilder, Int16BufferBuilder, Int32BufferBuilder}; use arrow::bitmap::Bitmap; use std::sync::Arc; @@ -529,7 +526,7 @@ mod tests { } let mut bb = Int32BufferBuilder::new(7); - bb.append_slice(&[4, 7, 6, 3, 2, 8, 9]).unwrap(); + bb.append_slice(&[4, 7, 6, 3, 2, 8, 9]); let expected_buffer = bb.finish(); assert_eq!( expected_buffer, @@ -617,7 +614,7 @@ mod tests { // Verify result record data let mut bb = Int32BufferBuilder::new(7); - bb.append_slice(&[0, 7, 0, 6, 3, 0, 8]).unwrap(); + bb.append_slice(&[0, 7, 0, 6, 3, 0, 8]); let expected_buffer = bb.finish(); assert_eq!( expected_buffer, @@ -626,8 +623,7 @@ mod tests { // Verify result def levels let mut bb = Int16BufferBuilder::new(7); - bb.append_slice(&[1i16, 2i16, 0i16, 2i16, 2i16, 0i16, 2i16]) - .unwrap(); + bb.append_slice(&[1i16, 2i16, 0i16, 2i16, 2i16, 0i16, 2i16]); let expected_def_levels = bb.finish(); assert_eq!( Some(expected_def_levels), @@ -636,8 +632,7 @@ mod tests { // Verify bitmap let mut bb = BooleanBufferBuilder::new(7); - bb.append_slice(&[false, true, false, true, true, false, true]) - .unwrap(); + bb.append_slice(&[false, true, false, true, true, false, true]); let expected_bitmap = Bitmap::from(bb.finish()); assert_eq!( Some(expected_bitmap), @@ -727,7 +722,7 @@ mod tests { // Verify result record data let mut bb = Int32BufferBuilder::new(9); - bb.append_slice(&[4, 0, 0, 7, 6, 3, 2, 8, 9]).unwrap(); + bb.append_slice(&[4, 0, 0, 7, 6, 3, 2, 8, 9]); let expected_buffer = bb.finish(); assert_eq!( expected_buffer, @@ -736,8 +731,7 @@ mod tests { // Verify result def levels let mut bb = Int16BufferBuilder::new(9); - bb.append_slice(&[2i16, 0i16, 1i16, 2i16, 2i16, 2i16, 2i16, 2i16, 2i16]) - .unwrap(); + bb.append_slice(&[2i16, 0i16, 1i16, 2i16, 2i16, 2i16, 2i16, 2i16, 2i16]); let expected_def_levels = bb.finish(); assert_eq!( Some(expected_def_levels), @@ -746,8 +740,7 @@ mod tests { // Verify bitmap let mut bb = BooleanBufferBuilder::new(9); - bb.append_slice(&[true, false, false, true, true, true, true, true, true]) - .unwrap(); + bb.append_slice(&[true, false, false, true, true, true, true, true, true]); let expected_bitmap = Bitmap::from(bb.finish()); assert_eq!( Some(expected_bitmap), From 5fd72e979666face07714cb127e9947a0f6d0a33 Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Sat, 26 Dec 2020 06:44:26 +0000 Subject: [PATCH 4/5] Migrated example. --- rust/arrow/examples/tensor_builder.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rust/arrow/examples/tensor_builder.rs b/rust/arrow/examples/tensor_builder.rs index 923ffdb30a6..1ef53920e04 100644 --- a/rust/arrow/examples/tensor_builder.rs +++ b/rust/arrow/examples/tensor_builder.rs @@ -30,7 +30,7 @@ fn main() -> Result<()> { // to match the required size for each buffer let mut builder = Int32BufferBuilder::new(16); for i in 0..16 { - builder.append(i).unwrap(); + builder.append(i); } let buf = builder.finish(); @@ -43,10 +43,10 @@ fn main() -> Result<()> { // Creating a tensor using float type buffer builder let mut builder = Float32BufferBuilder::new(4); - builder.append(1.0).unwrap(); - builder.append(2.0).unwrap(); - builder.append(3.0).unwrap(); - builder.append(4.0).unwrap(); + builder.append(1.0); + builder.append(2.0); + builder.append(3.0); + builder.append(4.0); let buf = builder.finish(); // When building the tensor the buffer and shape are necessary From bde093f4d79eea8e62ed1e946f5952eaeea0aa8b Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Sat, 26 Dec 2020 07:05:12 +0000 Subject: [PATCH 5/5] Migrated docs. --- rust/arrow/src/array/builder.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/rust/arrow/src/array/builder.rs b/rust/arrow/src/array/builder.rs index f415bd12f30..3dff38be846 100644 --- a/rust/arrow/src/array/builder.rs +++ b/rust/arrow/src/array/builder.rs @@ -67,7 +67,7 @@ pub(crate) fn builder_to_mutable_buffer( /// # Example: /// /// ``` -/// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; +/// use arrow::array::UInt8BufferBuilder; /// /// # fn main() -> arrow::error::Result<()> { /// let mut builder = UInt8BufferBuilder::new(100); @@ -91,17 +91,17 @@ impl BufferBuilder { /// elements of type `T`. /// /// The capacity can later be manually adjusted with the - /// [`reserve()`](BufferBuilderTrait::reserve) method. + /// [`reserve()`](BufferBuilder::reserve) method. /// Also the - /// [`append()`](BufferBuilderTrait::append), - /// [`append_slice()`](BufferBuilderTrait::append_slice) and - /// [`advance()`](BufferBuilderTrait::advance) + /// [`append()`](BufferBuilder::append), + /// [`append_slice()`](BufferBuilder::append_slice) and + /// [`advance()`](BufferBuilder::advance) /// methods automatically increase the capacity if needed. /// /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// @@ -123,7 +123,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.append(42); @@ -139,7 +139,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.append(42); @@ -163,14 +163,14 @@ impl BufferBuilder { /// Increases the number of elements in the internal buffer by `n` /// and resizes the buffer as needed. /// - /// The values of the newly added elements are undefined. + /// The values of the newly added elements are 0. /// This method is usually used when appending `NULL` values to the buffer /// as they still require physical memory space. /// /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.advance(2); @@ -189,7 +189,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.reserve(10); @@ -209,7 +209,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.append(42); @@ -233,7 +233,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.append_n(10, 42); @@ -253,7 +253,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.append_slice(&[42, 44, 46]); @@ -273,7 +273,7 @@ impl BufferBuilder { /// # Example: /// /// ``` - /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait}; + /// use arrow::array::UInt8BufferBuilder; /// /// let mut builder = UInt8BufferBuilder::new(10); /// builder.append_slice(&[42, 44, 46]);