Skip to content

Commit

Permalink
Revise FromIterator for Decimal128Array to use Into instead of Borrow (…
Browse files Browse the repository at this point in the history 
…#2442)

* Change FromIterator for Decimal128Array

* fix clippy

* Fix doc test
  • Loading branch information
@viirya
viirya authored Aug 17, 2022
1 parent 5e8586a commit 3557428
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 72 deletions.
37 changes: 27 additions & 10 deletions arrow/src/array/array_decimal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
// under the License.

use crate::array::ArrayAccessor;

use std::borrow::Borrow;
use std::convert::From;
use std::fmt;
use std::{any::Any, iter::FromIterator};
Expand Down Expand Up @@ -45,9 +43,9 @@ use crate::util::decimal::{BasicDecimal, Decimal256};
///
/// // Create a DecimalArray with the default precision and scale
/// let decimal_array: Decimal128Array = vec![
/// Some(8_887_000_000),
/// Some(8_887_000_000_i128),
/// None,
/// Some(-8_887_000_000),
/// Some(-8_887_000_000_i128),
/// ]
/// .into_iter().collect();
///
Expand Down Expand Up @@ -452,8 +450,8 @@ impl<Ptr: Into<Decimal256>> FromIterator<Option<Ptr>> for Decimal256Array {
}
}

impl<Ptr: Borrow<Option<i128>>> FromIterator<Ptr> for Decimal128Array {
fn from_iter<I: IntoIterator<Item = Ptr>>(iter: I) -> Self {
impl<Ptr: Into<i128>> FromIterator<Option<Ptr>> for Decimal128Array {
fn from_iter<I: IntoIterator<Item = Option<Ptr>>>(iter: I) -> Self {
let iter = iter.into_iter();
let (lower, upper) = iter.size_hint();
let size_hint = upper.unwrap_or(lower);
Expand All @@ -462,9 +460,9 @@ impl<Ptr: Borrow<Option<i128>>> FromIterator<Ptr> for Decimal128Array {

let buffer: Buffer = iter
.map(|item| {
if let Some(a) = item.borrow() {
if let Some(a) = item {
null_buf.append(true);
*a
a.into()
} else {
null_buf.append(false);
// arbitrary value for NULL
Expand Down Expand Up @@ -547,6 +545,7 @@ impl<'a, const BYTE_WIDTH: usize> BasicDecimalArray<BYTE_WIDTH> {
mod tests {
use crate::array::Decimal256Builder;
use crate::datatypes::{DECIMAL256_MAX_PRECISION, DECIMAL_DEFAULT_SCALE};
use crate::util::decimal::Decimal128;
use crate::{array::Decimal128Builder, datatypes::Field};
use num::{BigInt, Num};

Expand Down Expand Up @@ -749,8 +748,8 @@ mod tests {

#[test]
fn test_decimal_array_fmt_debug() {
let arr = [Some(8887000000), Some(-8887000000), None]
.iter()
let arr = [Some(8887000000_i128), Some(-8887000000_i128), None]
.into_iter()
.collect::<Decimal128Array>()
.with_precision_and_scale(23, 6)
.unwrap();
Expand Down Expand Up @@ -937,4 +936,22 @@ mod tests {
);
assert!(!array.is_null(2));
}

#[test]
fn test_from_iter_decimal128array() {
let array: Decimal128Array = vec![
Some(Decimal128::new_from_i128(38, 10, -100)),
None,
Some(Decimal128::new_from_i128(38, 10, 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!(!array.is_null(0));
assert!(array.is_null(1));
assert_eq!(101_i128, array.value(2).into());
assert!(!array.is_null(2));
}
}
28 changes: 16 additions & 12 deletions arrow/src/array/equal/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -841,8 +841,8 @@ mod tests {
test_equal(&a_slice, &b_slice, false);
}

fn create_decimal_array(data: &[Option<i128>]) -> ArrayData {
data.iter()
fn create_decimal_array(data: Vec<Option<i128>>) -> ArrayData {
data.into_iter()
.collect::<Decimal128Array>()
.with_precision_and_scale(23, 6)
.unwrap()
Expand All @@ -851,40 +851,44 @@ mod tests {

#[test]
fn test_decimal_equal() {
let a = create_decimal_array(&[Some(8_887_000_000), Some(-8_887_000_000)]);
let b = create_decimal_array(&[Some(8_887_000_000), Some(-8_887_000_000)]);
let a = create_decimal_array(vec![Some(8_887_000_000), Some(-8_887_000_000)]);
let b = create_decimal_array(vec![Some(8_887_000_000), Some(-8_887_000_000)]);
test_equal(&a, &b, true);

let b = create_decimal_array(&[Some(15_887_000_000), Some(-8_887_000_000)]);
let b = create_decimal_array(vec![Some(15_887_000_000), Some(-8_887_000_000)]);
test_equal(&a, &b, false);
}

// Test the case where null_count > 0
#[test]
fn test_decimal_null() {
let a = create_decimal_array(&[Some(8_887_000_000), None, Some(-8_887_000_000)]);
let b = create_decimal_array(&[Some(8_887_000_000), None, Some(-8_887_000_000)]);
let a =
create_decimal_array(vec![Some(8_887_000_000), None, Some(-8_887_000_000)]);
let b =
create_decimal_array(vec![Some(8_887_000_000), None, Some(-8_887_000_000)]);
test_equal(&a, &b, true);

let b = create_decimal_array(&[Some(8_887_000_000), Some(-8_887_000_000), None]);
let b =
create_decimal_array(vec![Some(8_887_000_000), Some(-8_887_000_000), None]);
test_equal(&a, &b, false);

let b = create_decimal_array(&[Some(15_887_000_000), None, Some(-8_887_000_000)]);
let b =
create_decimal_array(vec![Some(15_887_000_000), None, Some(-8_887_000_000)]);
test_equal(&a, &b, false);
}

#[test]
fn test_decimal_offsets() {
// Test the case where offset != 0
let a = create_decimal_array(&[
let a = create_decimal_array(vec![
Some(8_887_000_000),
None,
None,
Some(-8_887_000_000),
None,
None,
]);
let b = create_decimal_array(&[
let b = create_decimal_array(vec![
None,
Some(8_887_000_000),
None,
Expand Down Expand Up @@ -914,7 +918,7 @@ mod tests {
let b_slice = b.slice(2, 3);
test_equal(&a_slice, &b_slice, false);

let b = create_decimal_array(&[
let b = create_decimal_array(vec![
None,
None,
None,
Expand Down
4 changes: 2 additions & 2 deletions arrow/src/array/ord.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ pub mod tests {

#[test]
fn test_decimal() -> Result<()> {
let array = vec![Some(5), Some(2), Some(3)]
.iter()
let array = vec![Some(5_i128), Some(2_i128), Some(3_i128)]
.into_iter()
.collect::<Decimal128Array>()
.with_precision_and_scale(23, 6)
.unwrap();
Expand Down
16 changes: 8 additions & 8 deletions arrow/src/array/transform/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,12 +687,12 @@ mod tests {
};

fn create_decimal_array(
array: &[Option<i128>],
array: Vec<Option<i128>>,
precision: usize,
scale: usize,
) -> Decimal128Array {
array
.iter()
.into_iter()
.collect::<Decimal128Array>()
.with_precision_and_scale(precision, scale)
.unwrap()
Expand All @@ -702,36 +702,36 @@ mod tests {
#[cfg(not(feature = "force_validate"))]
fn test_decimal() {
let decimal_array =
create_decimal_array(&[Some(1), Some(2), None, Some(3)], 10, 3);
create_decimal_array(vec![Some(1), Some(2), None, Some(3)], 10, 3);
let arrays = vec![Array::data(&decimal_array)];
let mut a = MutableArrayData::new(arrays, true, 3);
a.extend(0, 0, 3);
a.extend(0, 2, 3);
let result = a.freeze();
let array = Decimal128Array::from(result);
let expected = create_decimal_array(&[Some(1), Some(2), None, None], 10, 3);
let expected = create_decimal_array(vec![Some(1), Some(2), None, None], 10, 3);
assert_eq!(array, expected);
}
#[test]
#[cfg(not(feature = "force_validate"))]
fn test_decimal_offset() {
let decimal_array =
create_decimal_array(&[Some(1), Some(2), None, Some(3)], 10, 3);
create_decimal_array(vec![Some(1), Some(2), None, Some(3)], 10, 3);
let decimal_array = decimal_array.slice(1, 3); // 2, null, 3
let arrays = vec![decimal_array.data()];
let mut a = MutableArrayData::new(arrays, true, 2);
a.extend(0, 0, 2); // 2, null
let result = a.freeze();
let array = Decimal128Array::from(result);
let expected = create_decimal_array(&[Some(2), None], 10, 3);
let expected = create_decimal_array(vec![Some(2), None], 10, 3);
assert_eq!(array, expected);
}

#[test]
#[cfg(not(feature = "force_validate"))]
fn test_decimal_null_offset_nulls() {
let decimal_array =
create_decimal_array(&[Some(1), Some(2), None, Some(3)], 10, 3);
create_decimal_array(vec![Some(1), Some(2), None, Some(3)], 10, 3);
let decimal_array = decimal_array.slice(1, 3); // 2, null, 3
let arrays = vec![decimal_array.data()];
let mut a = MutableArrayData::new(arrays, true, 2);
Expand All @@ -741,7 +741,7 @@ mod tests {
let result = a.freeze();
let array = Decimal128Array::from(result);
let expected = create_decimal_array(
&[Some(2), None, None, None, None, None, Some(3)],
vec![Some(2), None, None, None, None, None, Some(3)],
10,
3,
);
Expand Down
21 changes: 11 additions & 10 deletions arrow/src/compute/kernels/cast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2591,12 +2591,12 @@ mod tests {
}

fn create_decimal_array(
array: &[Option<i128>],
array: Vec<Option<i128>>,
precision: usize,
scale: usize,
) -> Result<Decimal128Array> {
array
.iter()
.into_iter()
.collect::<Decimal128Array>()
.with_precision_and_scale(precision, scale)
}
Expand All @@ -2618,7 +2618,7 @@ mod tests {
let output_type = DataType::Decimal128(20, 4);
assert!(can_cast_types(&input_type, &output_type));
let array = vec![Some(1123456), Some(2123456), Some(3123456), None];
let input_decimal_array = create_decimal_array(&array, 20, 3).unwrap();
let input_decimal_array = create_decimal_array(array, 20, 3).unwrap();
let array = Arc::new(input_decimal_array) as ArrayRef;
generate_cast_test_case!(
&array,
Expand All @@ -2633,7 +2633,7 @@ mod tests {
);
// negative test
let array = vec![Some(123456), None];
let input_decimal_array = create_decimal_array(&array, 10, 0).unwrap();
let input_decimal_array = create_decimal_array(array, 10, 0).unwrap();
let array = Arc::new(input_decimal_array) as ArrayRef;
let result = cast(&array, &DataType::Decimal128(2, 2));
assert!(result.is_err());
Expand All @@ -2647,7 +2647,7 @@ mod tests {
let output_type = DataType::Decimal256(20, 4);
assert!(can_cast_types(&input_type, &output_type));
let array = vec![Some(1123456), Some(2123456), Some(3123456), None];
let input_decimal_array = create_decimal_array(&array, 20, 3).unwrap();
let input_decimal_array = create_decimal_array(array, 20, 3).unwrap();
let array = Arc::new(input_decimal_array) as ArrayRef;
generate_cast_test_case!(
&array,
Expand Down Expand Up @@ -2739,7 +2739,7 @@ mod tests {
assert!(!can_cast_types(&decimal_type, &DataType::UInt8));
let value_array: Vec<Option<i128>> =
vec![Some(125), Some(225), Some(325), None, Some(525)];
let decimal_array = create_decimal_array(&value_array, 38, 2).unwrap();
let decimal_array = create_decimal_array(value_array, 38, 2).unwrap();
let array = Arc::new(decimal_array) as ArrayRef;
// i8
generate_cast_test_case!(
Expand Down Expand Up @@ -2786,7 +2786,7 @@ mod tests {

// overflow test: out of range of max i8
let value_array: Vec<Option<i128>> = vec![Some(24400)];
let decimal_array = create_decimal_array(&value_array, 38, 2).unwrap();
let decimal_array = create_decimal_array(value_array, 38, 2).unwrap();
let array = Arc::new(decimal_array) as ArrayRef;
let casted_array = cast(&array, &DataType::Int8);
assert_eq!(
Expand All @@ -2806,7 +2806,7 @@ mod tests {
Some(112345678),
Some(112345679),
];
let decimal_array = create_decimal_array(&value_array, 38, 2).unwrap();
let decimal_array = create_decimal_array(value_array, 38, 2).unwrap();
let array = Arc::new(decimal_array) as ArrayRef;
generate_cast_test_case!(
&array,
Expand Down Expand Up @@ -2834,7 +2834,7 @@ mod tests {
Some(112345678901234568),
Some(112345678901234560),
];
let decimal_array = create_decimal_array(&value_array, 38, 2).unwrap();
let decimal_array = create_decimal_array(value_array, 38, 2).unwrap();
let array = Arc::new(decimal_array) as ArrayRef;
generate_cast_test_case!(
&array,
Expand Down Expand Up @@ -5280,7 +5280,8 @@ mod tests {
Arc::new(DurationMicrosecondArray::from(vec![1000, 2000])),
Arc::new(DurationNanosecondArray::from(vec![1000, 2000])),
Arc::new(
create_decimal_array(&[Some(1), Some(2), Some(3), None], 38, 0).unwrap(),
create_decimal_array(vec![Some(1), Some(2), Some(3), None], 38, 0)
.unwrap(),
),
]
}
Expand Down
10 changes: 5 additions & 5 deletions arrow/src/compute/kernels/sort.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1062,8 +1062,8 @@ mod tests {
use std::convert::TryFrom;
use std::sync::Arc;

fn create_decimal_array(data: &[Option<i128>]) -> Decimal128Array {
data.iter()
fn create_decimal_array(data: Vec<Option<i128>>) -> Decimal128Array {
data.into_iter()
.collect::<Decimal128Array>()
.with_precision_and_scale(23, 6)
.unwrap()
Expand All @@ -1075,7 +1075,7 @@ mod tests {
limit: Option<usize>,
expected_data: Vec<u32>,
) {
let output = create_decimal_array(&data);
let output = create_decimal_array(data);
let expected = UInt32Array::from(expected_data);
let output =
sort_to_indices(&(Arc::new(output) as ArrayRef), options, limit).unwrap();
Expand All @@ -1088,8 +1088,8 @@ mod tests {
limit: Option<usize>,
expected_data: Vec<Option<i128>>,
) {
let output = create_decimal_array(&data);
let expected = Arc::new(create_decimal_array(&expected_data)) as ArrayRef;
let output = create_decimal_array(data);
let expected = Arc::new(create_decimal_array(expected_data)) as ArrayRef;
let output = match limit {
Some(_) => {
sort_limit(&(Arc::new(output) as ArrayRef), options, limit).unwrap()
Expand Down
Loading

0 comments on commit 3557428

Please sign in to comment.