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

Add Decimal128 API and use it in DecimalArray and DecimalBuilder #1871

Merged
merged 3 commits into from Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
44 changes: 15 additions & 29 deletions arrow/src/array/array_binary.rs
Expand Up @@ -33,6 +33,7 @@ use crate::datatypes::{
};
use crate::error::{ArrowError, Result};
use crate::util::bit_util;
use crate::util::decimal::Decimal128;
use crate::{buffer::MutableBuffer, datatypes::DataType};

/// See [`BinaryArray`] and [`LargeBinaryArray`] for storing
Expand Down Expand Up @@ -756,7 +757,7 @@ impl Array for FixedSizeBinaryArray {
/// .unwrap();
///
/// assert_eq!(&DataType::Decimal(23, 6), decimal_array.data_type());
/// assert_eq!(8_887_000_000, decimal_array.value(0));
/// assert_eq!(8_887_000_000_i128, decimal_array.value(0).as_i128());
/// assert_eq!("8887.000000", decimal_array.value_as_string(0));
/// assert_eq!(3, decimal_array.len());
/// assert_eq!(1, decimal_array.null_count());
Expand All @@ -775,8 +776,8 @@ pub struct DecimalArray {
}

impl DecimalArray {
/// Returns the element at index `i` as i128.
pub fn value(&self, i: usize) -> i128 {
/// Returns the element at index `i`.
pub fn value(&self, i: usize) -> Decimal128 {
assert!(i < self.data.len(), "DecimalArray out of bounds access");
let offset = i.checked_add(self.data.offset()).unwrap();
let raw_val = unsafe {
Expand All @@ -787,10 +788,11 @@ impl DecimalArray {
)
};
let as_array = raw_val.try_into();
match as_array {
let integer = match as_array {
Ok(v) if raw_val.len() == 16 => i128::from_le_bytes(v),
_ => panic!("DecimalArray elements are not 128bit integers."),
}
};
Decimal128::new_from_i128(self.precision, self.scale, integer)
}

/// Returns the offset for the element at index `i`.
Expand Down Expand Up @@ -821,23 +823,7 @@ impl DecimalArray {

#[inline]
pub fn value_as_string(&self, row: usize) -> String {
let value = self.value(row);
let value_str = value.to_string();

if self.scale == 0 {
value_str
} else {
let (sign, rest) = value_str.split_at(if value >= 0 { 0 } else { 1 });

if rest.len() > self.scale {
// Decimal separator is in the middle of the string
let (whole, decimal) = value_str.split_at(value_str.len() - self.scale);
format!("{}.{}", whole, decimal)
} else {
// String has to be padded
format!("{}0.{:0>width$}", sign, rest, width = self.scale)
}
}
self.value(row).as_string()
}

pub fn from_fixed_size_list_array(
Expand Down Expand Up @@ -1498,8 +1484,8 @@ mod tests {
.build()
.unwrap();
let decimal_array = DecimalArray::from(array_data);
assert_eq!(8_887_000_000, decimal_array.value(0));
assert_eq!(-8_887_000_000, decimal_array.value(1));
assert_eq!(8_887_000_000_i128, decimal_array.value(0).into());
assert_eq!(-8_887_000_000_i128, decimal_array.value(1).into());
assert_eq!(16, decimal_array.value_length());
}

Expand Down Expand Up @@ -1550,11 +1536,11 @@ mod tests {
let array = DecimalArray::from_iter_values(vec![-100, 0, 101].into_iter());
assert_eq!(array.len(), 3);
assert_eq!(array.data_type(), &DataType::Decimal(38, 10));
assert_eq!(-100, array.value(0));
assert_eq!(-100_i128, array.value(0).into());
assert!(!array.is_null(0));
assert_eq!(0, array.value(1));
assert_eq!(0_i128, array.value(1).into());
assert!(!array.is_null(1));
assert_eq!(101, array.value(2));
assert_eq!(101_i128, array.value(2).into());
assert!(!array.is_null(2));
}

Expand All @@ -1563,10 +1549,10 @@ mod tests {
let array: DecimalArray = vec![Some(-100), None, Some(101)].into_iter().collect();
assert_eq!(array.len(), 3);
assert_eq!(array.data_type(), &DataType::Decimal(38, 10));
assert_eq!(-100, array.value(0));
assert_eq!(-100_i128, array.value(0).into());
assert!(!array.is_null(0));
assert!(array.is_null(1));
assert_eq!(101, array.value(2));
assert_eq!(101_i128, array.value(2).into());
assert!(!array.is_null(2));
}

Expand Down
31 changes: 26 additions & 5 deletions arrow/src/array/builder.rs
Expand Up @@ -1478,11 +1478,11 @@ impl DecimalBuilder {
/// Automatically calls the `append` method to delimit the slice appended in as a
/// distinct array element.
#[inline]
pub fn append_value(&mut self, value: i128) -> Result<()> {
pub fn append_value(&mut self, value: impl Into<i128>) -> Result<()> {
let value = if self.value_validation {
validate_decimal_precision(value, self.precision)?
validate_decimal_precision(value.into(), self.precision)?
} else {
value
value.into()
};

let value_as_bytes = Self::from_i128_to_fixed_size_bytes(
Expand Down Expand Up @@ -2530,6 +2530,7 @@ mod tests {

use crate::array::Array;
use crate::bitmap::Bitmap;
use crate::util::decimal::Decimal128;

#[test]
fn test_builder_i32_empty() {
Expand Down Expand Up @@ -3442,9 +3443,29 @@ mod tests {
fn test_decimal_builder() {
let mut builder = DecimalBuilder::new(30, 38, 6);

builder.append_value(8_887_000_000).unwrap();
builder.append_value(8_887_000_000_i128).unwrap();
builder.append_null().unwrap();
builder.append_value(-8_887_000_000).unwrap();
builder.append_value(-8_887_000_000_i128).unwrap();
let decimal_array: DecimalArray = builder.finish();

assert_eq!(&DataType::Decimal(38, 6), decimal_array.data_type());
assert_eq!(3, decimal_array.len());
assert_eq!(1, decimal_array.null_count());
assert_eq!(32, decimal_array.value_offset(2));
assert_eq!(16, decimal_array.value_length());
}

#[test]
fn test_decimal_builder_with_decimal128() {
let mut builder = DecimalBuilder::new(30, 38, 6);

builder
.append_value(Decimal128::new_from_i128(30, 38, 8_887_000_000_i128))
.unwrap();
builder.append_null().unwrap();
builder
.append_value(Decimal128::new_from_i128(30, 38, -8_887_000_000_i128))
.unwrap();
let decimal_array: DecimalArray = builder.finish();

assert_eq!(&DataType::Decimal(38, 6), decimal_array.data_type());
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/array/equal_json.rs
Expand Up @@ -370,7 +370,7 @@ impl JsonEqual for DecimalArray {
self.is_valid(i)
&& (s
.parse::<i128>()
.map_or_else(|_| false, |v| v == self.value(i)))
.map_or_else(|_| false, |v| v == self.value(i).as_i128()))
}
JNull => self.is_null(i),
_ => false,
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/array/iterator.rs
Expand Up @@ -425,7 +425,7 @@ impl<'a> std::iter::Iterator for DecimalIter<'a> {
if self.array.is_null(old) {
Some(None)
} else {
Some(Some(self.array.value(old)))
Some(Some(self.array.value(old).as_i128()))
}
}
}
Expand Down
41 changes: 21 additions & 20 deletions arrow/src/compute/kernels/cast.rs
Expand Up @@ -353,7 +353,7 @@ macro_rules! cast_decimal_to_integer {
if array.is_null(i) {
value_builder.append_null()?;
} else {
let v = array.value(i) / div;
let v = array.value(i).as_i128() / div;
// check the overflow
// For example: Decimal(128,10,0) as i8
// 128 is out of range i8
Expand Down Expand Up @@ -383,7 +383,7 @@ macro_rules! cast_decimal_to_float {
} else {
// The range of f32 or f64 is larger than i128, we don't need to check overflow.
// cast the i128 to f64 will lose precision, for example the `112345678901234568` will be as `112345678901234560`.
let v = (array.value(i) as f64 / div) as $NATIVE_TYPE;
let v = (array.value(i).as_i128() as f64 / div) as $NATIVE_TYPE;
value_builder.append_value(v)?;
}
}
Expand Down Expand Up @@ -2196,6 +2196,7 @@ where
#[cfg(test)]
mod tests {
use super::*;
use crate::util::decimal::Decimal128;
use crate::{buffer::Buffer, util::display::array_value_to_string};

macro_rules! generate_cast_test_case {
Expand Down Expand Up @@ -2247,9 +2248,9 @@ mod tests {
DecimalArray,
&output_type,
vec![
Some(11234560_i128),
Some(21234560_i128),
Some(31234560_i128),
Some(Decimal128::new_from_i128(20, 4, 11234560_i128)),
Some(Decimal128::new_from_i128(20, 4, 21234560_i128)),
Some(Decimal128::new_from_i128(20, 4, 31234560_i128)),
None
]
);
Expand Down Expand Up @@ -2426,11 +2427,11 @@ mod tests {
DecimalArray,
&decimal_type,
vec![
Some(1000000_i128),
Some(2000000_i128),
Some(3000000_i128),
Some(Decimal128::new_from_i128(38, 6, 1000000_i128)),
Some(Decimal128::new_from_i128(38, 6, 2000000_i128)),
Some(Decimal128::new_from_i128(38, 6, 3000000_i128)),
None,
Some(5000000_i128)
Some(Decimal128::new_from_i128(38, 6, 5000000_i128))
]
);
}
Expand Down Expand Up @@ -2458,12 +2459,12 @@ mod tests {
DecimalArray,
&decimal_type,
vec![
Some(1100000_i128),
Some(2200000_i128),
Some(4400000_i128),
Some(Decimal128::new_from_i128(38, 6, 1100000_i128)),
Some(Decimal128::new_from_i128(38, 6, 2200000_i128)),
Some(Decimal128::new_from_i128(38, 6, 4400000_i128)),
None,
Some(1123456_i128),
Some(1123456_i128),
Some(Decimal128::new_from_i128(38, 6, 1123456_i128)),
Some(Decimal128::new_from_i128(38, 6, 1123456_i128)),
]
);

Expand All @@ -2483,13 +2484,13 @@ mod tests {
DecimalArray,
&decimal_type,
vec![
Some(1100000_i128),
Some(2200000_i128),
Some(4400000_i128),
Some(Decimal128::new_from_i128(38, 6, 1100000_i128)),
Some(Decimal128::new_from_i128(38, 6, 2200000_i128)),
Some(Decimal128::new_from_i128(38, 6, 4400000_i128)),
None,
Some(1123456_i128),
Some(1123456_i128),
Some(1123456_i128),
Some(Decimal128::new_from_i128(38, 6, 1123456_i128)),
Some(Decimal128::new_from_i128(38, 6, 1123456_i128)),
Some(Decimal128::new_from_i128(38, 6, 1123456_i128)),
]
);
}
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/compute/kernels/sort.rs
Expand Up @@ -504,7 +504,7 @@ where
.expect("Unable to downcast to decimal array");
let valids = value_indices
.into_iter()
.map(|index| (index, decimal_array.value(index as usize)))
.map(|index| (index, decimal_array.value(index as usize).as_i128()))
.collect::<Vec<(u32, i128)>>();
sort_primitive_inner(decimal_values, null_indices, cmp, options, limit, valids)
}
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/compute/kernels/take.rs
Expand Up @@ -524,7 +524,7 @@ where
if decimal_values.is_null(index) {
Ok(None)
} else {
Ok(Some(decimal_values.value(index)))
Ok(Some(decimal_values.value(index).as_i128()))
}
});
let t: Result<Option<Option<_>>> = t.transpose();
Expand Down