Skip to content
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.

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

DecimalBuilder should use FixedSizeBinaryBuilder #2092

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions arrow/src/array/array_decimal.rs
Expand Up @@ -172,6 +172,11 @@ pub trait BasicDecimalArray<T: BasicDecimal, U: From<ArrayData>>:
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,
Expand Down Expand Up @@ -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)
Expand Down
67 changes: 24 additions & 43 deletions arrow/src/array/builder/decimal_builder.rs
Expand Up @@ -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};

Expand All @@ -35,7 +34,7 @@ use crate::util::decimal::{BasicDecimal, Decimal256};
///
#[derive(Debug)]
pub struct DecimalBuilder {
builder: FixedSizeListBuilder<UInt8Builder>,
builder: FixedSizeBinaryBuilder,
precision: usize,
scale: usize,

Expand All @@ -49,19 +48,18 @@ pub struct DecimalBuilder {
/// See [`Decimal256Array`] for example.
#[derive(Debug)]
pub struct Decimal256Builder {
builder: FixedSizeListBuilder<UInt8Builder>,
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,
Expand All @@ -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<i128>) -> Result<()> {
let value = if self.value_validation {
Expand All @@ -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<Vec<u8>> {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down