Skip to content

Commit

Permalink
Add Decimal128 API and use it in DecimalArray and DecimalBuilder (#1871
Browse files Browse the repository at this point in the history 
)

* Add Decimal128

* Fix clippy

* Add code comment
  • Loading branch information
@viirya
viirya committed Jun 16, 2022
1 parent b10d58b commit f0bf7f9
Show file tree
Hide file tree
Showing 11 changed files with 219 additions and 60 deletions.
44 changes: 15 additions & 29 deletions arrow/src/array/array_binary.rs
View file
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
View file
Expand Up @@ -1567,11 +1567,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 @@ -2619,6 +2619,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 @@ -3531,9 +3532,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
View file
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
View file
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
View file
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
View file
Expand Up @@ -503,7 +503,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
View file
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

0 comments on commit f0bf7f9

Please sign in to comment.