From e3f0cc7be746285b218a06e320dbec9c179626c8 Mon Sep 17 00:00:00 2001 From: remzi <13716567376yh@gmail.com> Date: Mon, 18 Jul 2022 10:31:31 +0800 Subject: [PATCH 1/2] decimal builder uses fixed binary builder Signed-off-by: remzi <13716567376yh@gmail.com> --- arrow/src/array/builder/decimal_builder.rs | 67 ++++++++-------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/arrow/src/array/builder/decimal_builder.rs b/arrow/src/array/builder/decimal_builder.rs index 033de8976e3..20dce7679d7 100644 --- a/arrow/src/array/builder/decimal_builder.rs +++ b/arrow/src/array/builder/decimal_builder.rs @@ -21,8 +21,7 @@ use std::sync::Arc; use crate::array::array_decimal::{BasicDecimalArray, Decimal256Array}; use crate::array::ArrayRef; use crate::array::DecimalArray; -use crate::array::UInt8Builder; -use crate::array::{ArrayBuilder, FixedSizeListBuilder}; +use crate::array::{ArrayBuilder, FixedSizeBinaryBuilder}; use crate::error::{ArrowError, Result}; @@ -35,7 +34,7 @@ use crate::util::decimal::{BasicDecimal, Decimal256}; /// #[derive(Debug)] pub struct DecimalBuilder { - builder: FixedSizeListBuilder, + builder: FixedSizeBinaryBuilder, precision: usize, scale: usize, @@ -49,19 +48,18 @@ pub struct DecimalBuilder { /// See [`Decimal256Array`] for example. #[derive(Debug)] pub struct Decimal256Builder { - builder: FixedSizeListBuilder, + builder: FixedSizeBinaryBuilder, precision: usize, scale: usize, } impl DecimalBuilder { - /// Creates a new `DecimalBuilder`, `capacity` is the number of bytes in the values + const BYTE_LENGTH: i32 = 16; + /// Creates a new [`DecimalBuilder`], `capacity` is the number of bytes in the values /// array pub fn new(capacity: usize, precision: usize, scale: usize) -> Self { - let values_builder = UInt8Builder::new(capacity); - let byte_width = 16; Self { - builder: FixedSizeListBuilder::new(values_builder, byte_width), + builder: FixedSizeBinaryBuilder::new(capacity, Self::BYTE_LENGTH), precision, scale, value_validation: true, @@ -78,10 +76,7 @@ impl DecimalBuilder { self.value_validation = false; } - /// Appends a byte slice into the builder. - /// - /// Automatically calls the `append` method to delimit the slice appended in as a - /// distinct array element. + /// Appends a decimal value into the builder. #[inline] pub fn append_value(&mut self, value: impl Into) -> Result<()> { let value = if self.value_validation { @@ -90,19 +85,14 @@ impl DecimalBuilder { value.into() }; - let value_as_bytes = Self::from_i128_to_fixed_size_bytes( - value, - self.builder.value_length() as usize, - )?; - if self.builder.value_length() != value_as_bytes.len() as i32 { + let value_as_bytes = + Self::from_i128_to_fixed_size_bytes(value, Self::BYTE_LENGTH as usize)?; + if Self::BYTE_LENGTH != value_as_bytes.len() as i32 { return Err(ArrowError::InvalidArgumentError( "Byte slice does not have the same length as DecimalBuilder value lengths".to_string() )); } - self.builder - .values() - .append_slice(value_as_bytes.as_slice())?; - self.builder.append(true) + self.builder.append_value(value_as_bytes.as_slice()) } pub(crate) fn from_i128_to_fixed_size_bytes(v: i128, size: usize) -> Result> { @@ -119,14 +109,12 @@ impl DecimalBuilder { /// Append a null value to the array. #[inline] pub fn append_null(&mut self) -> Result<()> { - let length: usize = self.builder.value_length() as usize; - self.builder.values().append_slice(&vec![0u8; length][..])?; - self.builder.append(false) + self.builder.append_null() } /// Builds the `DecimalArray` and reset this builder. pub fn finish(&mut self) -> DecimalArray { - DecimalArray::from_fixed_size_list_array( + DecimalArray::from_fixed_size_binary_array( self.builder.finish(), self.precision, self.scale, @@ -167,52 +155,45 @@ impl ArrayBuilder for DecimalBuilder { } impl Decimal256Builder { - /// Creates a new `Decimal256Builder`, `capacity` is the number of bytes in the values + const BYTE_LENGTH: i32 = 32; + /// Creates a new [`Decimal256Builder`], `capacity` is the number of bytes in the values /// array pub fn new(capacity: usize, precision: usize, scale: usize) -> Self { - let values_builder = UInt8Builder::new(capacity); - let byte_width = 32; Self { - builder: FixedSizeListBuilder::new(values_builder, byte_width), + builder: FixedSizeBinaryBuilder::new(capacity, Self::BYTE_LENGTH), precision, scale, } } - /// Appends a byte slice into the builder. - /// - /// Automatically calls the `append` method to delimit the slice appended in as a - /// distinct array element. + /// Appends a [`Decimal256`] number into the builder. #[inline] pub fn append_value(&mut self, value: &Decimal256) -> Result<()> { - let value_as_bytes = value.raw_value(); - if self.precision != value.precision() || self.scale != value.scale() { return Err(ArrowError::InvalidArgumentError( "Decimal value does not have the same precision or scale as Decimal256Builder".to_string() )); } - if self.builder.value_length() != value_as_bytes.len() as i32 { + let value_as_bytes = value.raw_value(); + + if Self::BYTE_LENGTH != value_as_bytes.len() as i32 { return Err(ArrowError::InvalidArgumentError( "Byte slice does not have the same length as Decimal256Builder value lengths".to_string() )); } - self.builder.values().append_slice(value_as_bytes)?; - self.builder.append(true) + self.builder.append_value(value_as_bytes) } /// Append a null value to the array. #[inline] pub fn append_null(&mut self) -> Result<()> { - let length: usize = self.builder.value_length() as usize; - self.builder.values().append_slice(&vec![0u8; length][..])?; - self.builder.append(false) + self.builder.append_null() } - /// Builds the `Decimal256Array` and reset this builder. + /// Builds the [`Decimal256Array`] and reset this builder. pub fn finish(&mut self) -> Decimal256Array { - Decimal256Array::from_fixed_size_list_array( + Decimal256Array::from_fixed_size_binary_array( self.builder.finish(), self.precision, self.scale, From d20628a3c8329f48ea2abd2de7c7498632ab2c3e Mon Sep 17 00:00:00 2001 From: remzi <13716567376yh@gmail.com> Date: Mon, 18 Jul 2022 10:45:17 +0800 Subject: [PATCH 2/2] deprecate from_fixed_size_binary_array Signed-off-by: remzi <13716567376yh@gmail.com> --- arrow/src/array/array_decimal.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arrow/src/array/array_decimal.rs b/arrow/src/array/array_decimal.rs index ccb1fe05284..64b2c3b3b19 100644 --- a/arrow/src/array/array_decimal.rs +++ b/arrow/src/array/array_decimal.rs @@ -172,6 +172,11 @@ pub trait BasicDecimalArray>: U::from(array_data) } + /// Build a decimal array from [`FixedSizeListArray`]. + /// + /// NB: This function does not validate that each value is in the permissible + /// range for a decimal. And, the null buffer of the child array will be ignored. + #[deprecated(note = "please use `from_fixed_size_binary_array` instead")] fn from_fixed_size_list_array( v: FixedSizeListArray, precision: usize, @@ -707,6 +712,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn test_decimal_array_from_fixed_size_list() { let value_data = ArrayData::builder(DataType::UInt8) .offset(16)