Skip to content

Commit

Permalink
Decimal cleanup (apache#2637)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tustvold
tustvold committed Oct 13, 2022
1 parent eeb1261 commit 08569df
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 570 deletions.
10 changes: 9 additions & 1 deletion .github/workflows/arrow.yml
View file
Expand Up @@ -186,6 +186,14 @@ jobs:
- name: Setup Clippy
run: |
rustup component add clippy
- name: Run clippy
- name: Clippy arrow-buffer with all features
run: cargo clippy -p arrow-buffer --all-features
- name: Clippy arrow-data with all features
run: cargo clippy -p arrow-data --all-features
- name: Clippy arrow-schema with all features
run: cargo clippy -p arrow-schema --all-features
- name: Clippy arrow-array with all features
run: cargo clippy -p arrow-array --all-features
- name: Clippy arrow
run: |
cargo clippy -p arrow --features=prettyprint,csv,ipc,test_utils,ffi,ipc_compression,dyn_cmp_dict,dyn_arith_dict --all-targets -- -D warnings
97 changes: 43 additions & 54 deletions arrow-array/src/array/primitive_array.rs
View file
Expand Up @@ -16,7 +16,6 @@
// under the License.

use crate::builder::{BooleanBufferBuilder, BufferBuilder, PrimitiveBuilder};
use crate::decimal::Decimal;
use crate::iterator::PrimitiveIter;
use crate::raw_pointer::RawPtrBox;
use crate::temporal_conversions::{as_date, as_datetime, as_duration, as_time};
Expand Down Expand Up @@ -917,67 +916,61 @@ impl<T: DecimalType + ArrowPrimitiveType> PrimitiveArray<T> {
})
}

pub fn value_as_string(&self, row: usize) -> Result<String, ArrowError> {
let p = self.precision()?;
let s = self.scale()?;
Ok(Decimal::<T>::new(p, s, &T::to_native(self.value(row))).to_string())
pub fn value_as_string(&self, row: usize) -> String {
T::format_decimal(self.value(row), self.precision(), self.scale())
}

pub fn precision(&self) -> Result<u8, ArrowError> {
/// Returns the decimal precision of this array
pub fn precision(&self) -> u8 {
match T::BYTE_LENGTH {
16 => {
if let DataType::Decimal128(p, _) = self.data().data_type() {
Ok(*p)
*p
} else {
Err(ArrowError::InvalidArgumentError(format!(
unreachable!(
"Decimal128Array datatype is not DataType::Decimal128 but {}",
self.data_type()
)))
)
}
}
32 => {
if let DataType::Decimal256(p, _) = self.data().data_type() {
Ok(*p)
*p
} else {
Err(ArrowError::InvalidArgumentError(format!(
unreachable!(
"Decimal256Array datatype is not DataType::Decimal256 but {}",
self.data_type()
)))
)
}
}
other => Err(ArrowError::InvalidArgumentError(format!(
"Unsupported byte length for decimal array {}",
other
))),
other => unreachable!("Unsupported byte length for decimal array {}", other),
}
}

pub fn scale(&self) -> Result<u8, ArrowError> {
/// Returns the decimal scale of this array
pub fn scale(&self) -> u8 {
match T::BYTE_LENGTH {
16 => {
if let DataType::Decimal128(_, s) = self.data().data_type() {
Ok(*s)
*s
} else {
Err(ArrowError::InvalidArgumentError(format!(
unreachable!(
"Decimal128Array datatype is not DataType::Decimal128 but {}",
self.data_type()
)))
)
}
}
32 => {
if let DataType::Decimal256(_, s) = self.data().data_type() {
Ok(*s)
*s
} else {
Err(ArrowError::InvalidArgumentError(format!(
unreachable!(
"Decimal256Array datatype is not DataType::Decimal256 but {}",
self.data_type()
)))
)
}
}
other => Err(ArrowError::InvalidArgumentError(format!(
"Unsupported byte length for decimal array {}",
other
))),
other => unreachable!("Unsupported byte length for decimal array {}", other),
}
}
}
Expand Down Expand Up @@ -1564,8 +1557,8 @@ mod tests {
.build()
.unwrap();
let decimal_array = Decimal128Array::from(array_data);
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!(8_887_000_000_i128, decimal_array.value(0));
assert_eq!(-8_887_000_000_i128, decimal_array.value(1));
}

#[test]
Expand All @@ -1576,7 +1569,7 @@ mod tests {
let result = decimal_builder.finish().with_precision_and_scale(5, 3);
assert!(result.is_ok());
let arr = result.unwrap();
assert_eq!("12.345", arr.value_as_string(1).unwrap());
assert_eq!("12.345", arr.value_as_string(1));

// Validate it explicitly
let result = arr.validate_decimal_precision(5);
Expand All @@ -1594,8 +1587,8 @@ mod tests {
let result = decimal_builder.finish().with_precision_and_scale(2, 1);
assert!(result.is_ok());
let arr = result.unwrap();
assert_eq!("9.9", arr.value_as_string(1).unwrap());
assert_eq!("-9.9", arr.value_as_string(3).unwrap());
assert_eq!("9.9", arr.value_as_string(1));
assert_eq!("-9.9", arr.value_as_string(3));

// Validate it explicitly
let result = arr.validate_decimal_precision(2);
Expand All @@ -1611,11 +1604,11 @@ mod tests {
let array = Decimal128Array::from_iter_values(vec![-100, 0, 101].into_iter());
assert_eq!(array.len(), 3);
assert_eq!(array.data_type(), &DataType::Decimal128(38, 10));
assert_eq!(-100_i128, array.value(0).into());
assert_eq!(-100_i128, array.value(0));
assert!(!array.is_null(0));
assert_eq!(0_i128, array.value(1).into());
assert_eq!(0_i128, array.value(1));
assert!(!array.is_null(1));
assert_eq!(101_i128, array.value(2).into());
assert_eq!(101_i128, array.value(2));
assert!(!array.is_null(2));
}

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

Expand Down Expand Up @@ -1661,13 +1654,13 @@ mod tests {
.with_precision_and_scale(6, 3)
.unwrap();

assert_eq!("123.450", arr.value_as_string(0).unwrap());
assert_eq!("-123.450", arr.value_as_string(1).unwrap());
assert_eq!("0.100", arr.value_as_string(2).unwrap());
assert_eq!("-0.100", arr.value_as_string(3).unwrap());
assert_eq!("0.010", arr.value_as_string(4).unwrap());
assert_eq!("-0.010", arr.value_as_string(5).unwrap());
assert_eq!("0.000", arr.value_as_string(6).unwrap());
assert_eq!("123.450", arr.value_as_string(0));
assert_eq!("-123.450", arr.value_as_string(1));
assert_eq!("0.100", arr.value_as_string(2));
assert_eq!("-0.100", arr.value_as_string(3));
assert_eq!("0.010", arr.value_as_string(4));
assert_eq!("-0.010", arr.value_as_string(5));
assert_eq!("0.000", arr.value_as_string(6));
}

#[test]
Expand All @@ -1677,12 +1670,10 @@ mod tests {
.unwrap();

assert_eq!(arr.data_type(), &DataType::Decimal128(20, 2));
assert_eq!(arr.precision().unwrap(), 20);
assert_eq!(arr.scale().unwrap(), 2);
assert_eq!(arr.precision(), 20);
assert_eq!(arr.scale(), 2);

let actual: Vec<_> = (0..arr.len())
.map(|i| arr.value_as_string(i).unwrap())
.collect();
let actual: Vec<_> = (0..arr.len()).map(|i| arr.value_as_string(i)).collect();
let expected = vec!["123.45", "4.56", "78.90", "-1232234234324.32"];

assert_eq!(actual, expected);
Expand Down Expand Up @@ -1748,9 +1739,7 @@ mod tests {
let value2 = i256::from_i128(56789);

let mut array: Decimal256Array =
vec![Some(value1.clone()), None, Some(value2.clone())]
.into_iter()
.collect();
vec![Some(value1), None, Some(value2)].into_iter().collect();
array = array.with_precision_and_scale(76, 10).unwrap();
assert_eq!(array.len(), 3);
assert_eq!(array.data_type(), &DataType::Decimal256(76, 10));
Expand All @@ -1768,10 +1757,10 @@ mod tests {
array = array.with_precision_and_scale(38, 10).unwrap();
assert_eq!(array.len(), 3);
assert_eq!(array.data_type(), &DataType::Decimal128(38, 10));
assert_eq!(-100_i128, array.value(0).into());
assert_eq!(-100_i128, array.value(0));
assert!(!array.is_null(0));
assert!(array.is_null(1));
assert_eq!(101_i128, array.value(2).into());
assert_eq!(101_i128, array.value(2));
assert!(!array.is_null(2));
}

Expand Down

0 comments on commit 08569df

Please sign in to comment.