This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new d082fa3 [Minor] Clean up DecimalArray API Usage (#1869)
d082fa3 is described below
commit d082fa3f66188ab2bbf913cde593026240c1fc0e
Author: Andrew Lamb <[email protected]>
AuthorDate: Sun Mar 6 07:11:41 2022 -0500
[Minor] Clean up DecimalArray API Usage (#1869)
* Clean up decimal array creation
* Refactor a bit more
* Update the next
!
* cleanup
* update
* port over min/max
* Update sum
* Use max scale / precision from arrow
* fmt
* Fixup
* clippy
---
datafusion-common/src/lib.rs | 4 +-
datafusion-common/src/scalar.rs | 45 ++------
.../src/coercion_rule/binary_rule.rs | 7 +-
.../src/expressions/average.rs | 47 ++++----
datafusion-physical-expr/src/expressions/cast.rs | 80 ++++++++-----
.../src/expressions/min_max.rs | 127 ++++++++++-----------
datafusion-physical-expr/src/expressions/sum.rs | 76 ++++++------
datafusion/src/physical_plan/hash_utils.rs | 16 +--
datafusion/src/scalar.rs | 4 +-
datafusion/src/sql/utils.rs | 6 +-
10 files changed, 198 insertions(+), 214 deletions(-)
diff --git a/datafusion-common/src/lib.rs b/datafusion-common/src/lib.rs
index fdcb7d4..d39020f 100644
--- a/datafusion-common/src/lib.rs
+++ b/datafusion-common/src/lib.rs
@@ -25,6 +25,4 @@ mod scalar;
pub use column::Column;
pub use dfschema::{DFField, DFSchema, DFSchemaRef, ExprSchema, ToDFSchema};
pub use error::{DataFusionError, Result};
-pub use scalar::{
- ScalarType, ScalarValue, MAX_PRECISION_FOR_DECIMAL128,
MAX_SCALE_FOR_DECIMAL128,
-};
+pub use scalar::{ScalarType, ScalarValue};
diff --git a/datafusion-common/src/scalar.rs b/datafusion-common/src/scalar.rs
index d7c6c6b..4a1dde1 100644
--- a/datafusion-common/src/scalar.rs
+++ b/datafusion-common/src/scalar.rs
@@ -26,6 +26,7 @@ use arrow::{
Float64Type, Int16Type, Int32Type, Int64Type, Int8Type, IntervalUnit,
TimeUnit,
TimestampMicrosecondType, TimestampMillisecondType,
TimestampNanosecondType,
TimestampSecondType, UInt16Type, UInt32Type, UInt64Type, UInt8Type,
+ DECIMAL_MAX_PRECISION,
},
};
use ordered_float::OrderedFloat;
@@ -34,11 +35,6 @@ use std::convert::{Infallible, TryInto};
use std::str::FromStr;
use std::{convert::TryFrom, fmt, iter::repeat, sync::Arc};
-// TODO may need to be moved to arrow-rs
-/// The max precision and scale for decimal128
-pub const MAX_PRECISION_FOR_DECIMAL128: usize = 38;
-pub const MAX_SCALE_FOR_DECIMAL128: usize = 38;
-
/// Represents a dynamically typed, nullable single value.
/// This is the single-valued counter-part of arrow’s `Array`.
#[derive(Clone)]
@@ -542,7 +538,7 @@ impl ScalarValue {
scale: usize,
) -> Result<Self> {
// make sure the precision and scale is valid
- if precision <= MAX_PRECISION_FOR_DECIMAL128 && scale <= precision {
+ if precision <= DECIMAL_MAX_PRECISION && scale <= precision {
return Ok(ScalarValue::Decimal128(Some(value), precision, scale));
}
return Err(DataFusionError::Internal(format!(
@@ -985,26 +981,15 @@ impl ScalarValue {
precision: &usize,
scale: &usize,
) -> Result<DecimalArray> {
- // collect the value as Option<i128>
let array = scalars
.into_iter()
.map(|element: ScalarValue| match element {
ScalarValue::Decimal128(v1, _, _) => v1,
_ => unreachable!(),
})
- .collect::<Vec<Option<i128>>>();
-
- // build the decimal array using the Decimal Builder
- let mut builder = DecimalBuilder::new(array.len(), *precision, *scale);
- array.iter().for_each(|element| match element {
- None => {
- builder.append_null().unwrap();
- }
- Some(v) => {
- builder.append_value(*v).unwrap();
- }
- });
- Ok(builder.finish())
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(*precision, *scale)?;
+ Ok(array)
}
fn iter_to_array_list(
@@ -1080,21 +1065,11 @@ impl ScalarValue {
scale: &usize,
size: usize,
) -> DecimalArray {
- let mut builder = DecimalBuilder::new(size, *precision, *scale);
- match value {
- None => {
- for _i in 0..size {
- builder.append_null().unwrap();
- }
- }
- Some(v) => {
- let v = *v;
- for _i in 0..size {
- builder.append_value(v).unwrap();
- }
- }
- };
- builder.finish()
+ std::iter::repeat(value)
+ .take(size)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(*precision, *scale)
+ .unwrap()
}
/// Converts a scalar value into an array of `size` rows.
diff --git a/datafusion-physical-expr/src/coercion_rule/binary_rule.rs
b/datafusion-physical-expr/src/coercion_rule/binary_rule.rs
index 2207079..ac23f2b 100644
--- a/datafusion-physical-expr/src/coercion_rule/binary_rule.rs
+++ b/datafusion-physical-expr/src/coercion_rule/binary_rule.rs
@@ -17,10 +17,9 @@
//! Coercion rules for matching argument types for binary operators
-use arrow::datatypes::DataType;
+use arrow::datatypes::{DataType, DECIMAL_MAX_PRECISION, DECIMAL_MAX_SCALE};
use datafusion_common::DataFusionError;
use datafusion_common::Result;
-use datafusion_common::{MAX_PRECISION_FOR_DECIMAL128,
MAX_SCALE_FOR_DECIMAL128};
use datafusion_expr::Operator;
/// Coercion rules for all binary operators. Returns the output type
@@ -261,8 +260,8 @@ fn mathematics_numerical_coercion(
fn create_decimal_type(precision: usize, scale: usize) -> DataType {
DataType::Decimal(
- MAX_PRECISION_FOR_DECIMAL128.min(precision),
- MAX_SCALE_FOR_DECIMAL128.min(scale),
+ DECIMAL_MAX_PRECISION.min(precision),
+ DECIMAL_MAX_SCALE.min(scale),
)
}
diff --git a/datafusion-physical-expr/src/expressions/average.rs
b/datafusion-physical-expr/src/expressions/average.rs
index acdbf8b..8888ee9 100644
--- a/datafusion-physical-expr/src/expressions/average.rs
+++ b/datafusion-physical-expr/src/expressions/average.rs
@@ -23,15 +23,13 @@ use std::sync::Arc;
use crate::{AggregateExpr, PhysicalExpr};
use arrow::compute;
-use arrow::datatypes::DataType;
+use arrow::datatypes::{DataType, DECIMAL_MAX_PRECISION, DECIMAL_MAX_SCALE};
use arrow::{
array::{ArrayRef, UInt64Array},
datatypes::Field,
};
+use datafusion_common::ScalarValue;
use datafusion_common::{DataFusionError, Result};
-use datafusion_common::{
- ScalarValue, MAX_PRECISION_FOR_DECIMAL128, MAX_SCALE_FOR_DECIMAL128,
-};
use datafusion_expr::Accumulator;
use super::{format_state_name, sum};
@@ -50,8 +48,8 @@ pub fn avg_return_type(arg_type: &DataType) ->
Result<DataType> {
DataType::Decimal(precision, scale) => {
// in the spark, the result type is DECIMAL(min(38,precision+4),
min(38,scale+4)).
// ref:
https://github.com/apache/spark/blob/fcf636d9eb8d645c24be3db2d599aba2d7e2955a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Average.scala#L66
- let new_precision = MAX_PRECISION_FOR_DECIMAL128.min(*precision +
4);
- let new_scale = MAX_SCALE_FOR_DECIMAL128.min(*scale + 4);
+ let new_precision = DECIMAL_MAX_PRECISION.min(*precision + 4);
+ let new_scale = DECIMAL_MAX_SCALE.min(*scale + 4);
Ok(DataType::Decimal(new_precision, new_scale))
}
DataType::Int8
@@ -237,11 +235,12 @@ mod tests {
#[test]
fn avg_decimal() -> Result<()> {
// test agg
- let mut decimal_builder = DecimalBuilder::new(6, 10, 0);
- for i in 1..7 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..7)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
@@ -254,15 +253,12 @@ mod tests {
#[test]
fn avg_decimal_with_nulls() -> Result<()> {
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- if i == 2 {
- decimal_builder.append_null()?;
- } else {
- decimal_builder.append_value(i)?;
- }
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(|i| if i == 2 { None } else { Some(i) })
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
@@ -275,11 +271,12 @@ mod tests {
#[test]
fn avg_decimal_all_nulls() -> Result<()> {
// test agg
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for _i in 1..6 {
- decimal_builder.append_null()?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ std::iter::repeat(None)
+ .take(6)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
diff --git a/datafusion-physical-expr/src/expressions/cast.rs
b/datafusion-physical-expr/src/expressions/cast.rs
index 1b157e3..2870c9b 100644
--- a/datafusion-physical-expr/src/expressions/cast.rs
+++ b/datafusion-physical-expr/src/expressions/cast.rs
@@ -161,8 +161,8 @@ mod tests {
use crate::expressions::col;
use arrow::{
array::{
- Array, DecimalArray, DecimalBuilder, Float32Array, Float64Array,
Int16Array,
- Int32Array, Int64Array, Int8Array, StringArray,
Time64NanosecondArray,
+ Array, DecimalArray, Float32Array, Float64Array, Int16Array,
Int32Array,
+ Int64Array, Int8Array, StringArray, Time64NanosecondArray,
TimestampNanosecondArray, UInt32Array,
},
datatypes::*,
@@ -268,23 +268,16 @@ mod tests {
}};
}
- fn create_decimal_array(
- array: &[i128],
- precision: usize,
- scale: usize,
- ) -> Result<DecimalArray> {
- let mut decimal_builder = DecimalBuilder::new(array.len(), precision,
scale);
- for value in array {
- decimal_builder.append_value(*value)?
- }
- decimal_builder.append_null()?;
- Ok(decimal_builder.finish())
- }
-
#[test]
fn test_cast_decimal_to_decimal() -> Result<()> {
- let array: Vec<i128> = vec![1234, 2222, 3, 4000, 5000];
- let decimal_array = create_decimal_array(&array, 10, 3)?;
+ let array = vec![1234, 2222, 3, 4000, 5000];
+
+ let decimal_array = array
+ .iter()
+ .map(|v| Some(*v))
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 3)?;
+
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 3),
@@ -301,7 +294,12 @@ mod tests {
DEFAULT_DATAFUSION_CAST_OPTIONS
);
- let decimal_array = create_decimal_array(&array, 10, 3)?;
+ let decimal_array = array
+ .iter()
+ .map(|v| Some(*v))
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 3)?;
+
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 3),
@@ -323,9 +321,12 @@ mod tests {
#[test]
fn test_cast_decimal_to_numeric() -> Result<()> {
- let array: Vec<i128> = vec![1, 2, 3, 4, 5];
+ let array = vec![Some(1), Some(2), Some(3), Some(4), Some(5), None];
// decimal to i8
- let decimal_array = create_decimal_array(&array, 10, 0)?;
+ let decimal_array = array
+ .iter()
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?;
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 0),
@@ -341,8 +342,12 @@ mod tests {
],
DEFAULT_DATAFUSION_CAST_OPTIONS
);
+
// decimal to i16
- let decimal_array = create_decimal_array(&array, 10, 0)?;
+ let decimal_array = array
+ .iter()
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?;
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 0),
@@ -358,8 +363,12 @@ mod tests {
],
DEFAULT_DATAFUSION_CAST_OPTIONS
);
+
// decimal to i32
- let decimal_array = create_decimal_array(&array, 10, 0)?;
+ let decimal_array = array
+ .iter()
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?;
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 0),
@@ -375,8 +384,12 @@ mod tests {
],
DEFAULT_DATAFUSION_CAST_OPTIONS
);
+
// decimal to i64
- let decimal_array = create_decimal_array(&array, 10, 0)?;
+ let decimal_array = array
+ .iter()
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?;
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 0),
@@ -392,9 +405,20 @@ mod tests {
],
DEFAULT_DATAFUSION_CAST_OPTIONS
);
+
// decimal to float32
- let array: Vec<i128> = vec![1234, 2222, 3, 4000, 5000];
- let decimal_array = create_decimal_array(&array, 10, 3)?;
+ let array = vec![
+ Some(1234),
+ Some(2222),
+ Some(3),
+ Some(4000),
+ Some(5000),
+ None,
+ ];
+ let decimal_array = array
+ .iter()
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 3)?;
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(10, 3),
@@ -410,8 +434,12 @@ mod tests {
],
DEFAULT_DATAFUSION_CAST_OPTIONS
);
+
// decimal to float64
- let decimal_array = create_decimal_array(&array, 20, 6)?;
+ let decimal_array = array
+ .into_iter()
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(20, 6)?;
generic_decimal_to_other_test_cast!(
decimal_array,
DataType::Decimal(20, 6),
diff --git a/datafusion-physical-expr/src/expressions/min_max.rs
b/datafusion-physical-expr/src/expressions/min_max.rs
index be3ea95..a599d65 100644
--- a/datafusion-physical-expr/src/expressions/min_max.rs
+++ b/datafusion-physical-expr/src/expressions/min_max.rs
@@ -556,7 +556,6 @@ mod tests {
use crate::expressions::col;
use crate::expressions::tests::aggregate;
use crate::generic_test_op;
- use arrow::array::DecimalBuilder;
use arrow::datatypes::*;
use arrow::record_batch::RecordBatch;
use datafusion_common::Result;
@@ -572,32 +571,33 @@ mod tests {
assert_eq!(result, left);
// min batch
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
let result = min_batch(&array)?;
assert_eq!(result, ScalarValue::Decimal128(Some(1), 10, 0));
- // min batch without values
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- let array: ArrayRef = Arc::new(decimal_builder.finish());
- let result = min_batch(&array)?;
- assert_eq!(ScalarValue::Decimal128(None, 10, 0), result);
- let mut decimal_builder = DecimalBuilder::new(0, 10, 0);
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ // min batch without values
+ let array: ArrayRef = Arc::new(
+ std::iter::repeat(None)
+ .take(0)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
let result = min_batch(&array)?;
assert_eq!(ScalarValue::Decimal128(None, 10, 0), result);
// min batch with agg
- let mut decimal_builder = DecimalBuilder::new(6, 10, 0);
- decimal_builder.append_null().unwrap();
- for i in 1..6 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
@@ -610,11 +610,12 @@ mod tests {
#[test]
fn min_decimal_all_nulls() -> Result<()> {
// min batch all nulls
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for _i in 1..6 {
- decimal_builder.append_null()?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ std::iter::repeat(None)
+ .take(6)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
@@ -627,15 +628,13 @@ mod tests {
#[test]
fn min_decimal_with_nulls() -> Result<()> {
// min batch with nulls
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- if i == 2 {
- decimal_builder.append_null()?;
- } else {
- decimal_builder.append_value(i as i128)?;
- }
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(|i| if i == 2 { None } else { Some(i) })
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
+
generic_test_op!(
array,
DataType::Decimal(10, 0),
@@ -662,30 +661,32 @@ mod tests {
assert_eq!(expect.to_string(), result.unwrap_err().to_string());
// max batch
- let mut decimal_builder = DecimalBuilder::new(5, 10, 5);
- for i in 1..6 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 5)?,
+ );
let result = max_batch(&array)?;
assert_eq!(result, ScalarValue::Decimal128(Some(5), 10, 5));
+
// max batch without values
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ std::iter::repeat(None)
+ .take(0)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
let result = max_batch(&array)?;
assert_eq!(ScalarValue::Decimal128(None, 10, 0), result);
- let mut decimal_builder = DecimalBuilder::new(0, 10, 0);
- let array: ArrayRef = Arc::new(decimal_builder.finish());
- let result = max_batch(&array)?;
- assert_eq!(ScalarValue::Decimal128(None, 10, 0), result);
// max batch with agg
- let mut decimal_builder = DecimalBuilder::new(6, 10, 0);
- decimal_builder.append_null().unwrap();
- for i in 1..6 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
@@ -697,15 +698,12 @@ mod tests {
#[test]
fn max_decimal_with_nulls() -> Result<()> {
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- if i == 2 {
- decimal_builder.append_null()?;
- } else {
- decimal_builder.append_value(i as i128)?;
- }
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(|i| if i == 2 { None } else { Some(i) })
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
@@ -717,11 +715,12 @@ mod tests {
#[test]
fn max_decimal_all_nulls() -> Result<()> {
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for _i in 1..6 {
- decimal_builder.append_null()?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ std::iter::repeat(None)
+ .take(6)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(10, 0),
diff --git a/datafusion-physical-expr/src/expressions/sum.rs
b/datafusion-physical-expr/src/expressions/sum.rs
index f2c19f5..9945620 100644
--- a/datafusion-physical-expr/src/expressions/sum.rs
+++ b/datafusion-physical-expr/src/expressions/sum.rs
@@ -23,7 +23,7 @@ use std::sync::Arc;
use crate::{AggregateExpr, PhysicalExpr};
use arrow::compute;
-use arrow::datatypes::DataType;
+use arrow::datatypes::{DataType, DECIMAL_MAX_PRECISION};
use arrow::{
array::{
ArrayRef, Float32Array, Float64Array, Int16Array, Int32Array,
Int64Array,
@@ -31,8 +31,7 @@ use arrow::{
},
datatypes::Field,
};
-use datafusion_common::{DataFusionError, Result};
-use datafusion_common::{ScalarValue, MAX_PRECISION_FOR_DECIMAL128};
+use datafusion_common::{DataFusionError, Result, ScalarValue};
use datafusion_expr::Accumulator;
use super::format_state_name;
@@ -63,7 +62,7 @@ pub fn sum_return_type(arg_type: &DataType) ->
Result<DataType> {
DataType::Decimal(precision, scale) => {
// in the spark, the result type is DECIMAL(min(38,precision+10),
s)
// ref:
https://github.com/apache/spark/blob/fcf636d9eb8d645c24be3db2d599aba2d7e2955a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Sum.scala#L66
- let new_precision = MAX_PRECISION_FOR_DECIMAL128.min(*precision +
10);
+ let new_precision = DECIMAL_MAX_PRECISION.min(*precision + 10);
Ok(DataType::Decimal(new_precision, *scale))
}
other => Err(DataFusionError::Plan(format!(
@@ -377,7 +376,6 @@ mod tests {
use super::*;
use crate::expressions::col;
use crate::generic_test_op;
- use arrow::array::DecimalBuilder;
use arrow::datatypes::*;
use arrow::record_batch::RecordBatch;
use datafusion_common::Result;
@@ -419,20 +417,22 @@ mod tests {
);
// test sum batch
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
let result = sum_batch(&array)?;
assert_eq!(ScalarValue::Decimal128(Some(15), 10, 0), result);
// test agg
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- decimal_builder.append_value(i as i128)?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
generic_test_op!(
array,
@@ -452,28 +452,22 @@ mod tests {
assert_eq!(ScalarValue::Decimal128(Some(123), 10, 2), result);
// test with batch
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for i in 1..6 {
- if i == 2 {
- decimal_builder.append_null()?;
- } else {
- decimal_builder.append_value(i)?;
- }
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(|i| if i == 2 { None } else { Some(i) })
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
let result = sum_batch(&array)?;
assert_eq!(ScalarValue::Decimal128(Some(13), 10, 0), result);
// test agg
- let mut decimal_builder = DecimalBuilder::new(5, 35, 0);
- for i in 1..6 {
- if i == 2 {
- decimal_builder.append_null()?;
- } else {
- decimal_builder.append_value(i)?;
- }
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ (1..6)
+ .map(|i| if i == 2 { None } else { Some(i) })
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(35, 0)?,
+ );
generic_test_op!(
array,
DataType::Decimal(35, 0),
@@ -492,20 +486,16 @@ mod tests {
assert_eq!(ScalarValue::Decimal128(None, 10, 2), result);
// test with batch
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for _i in 1..6 {
- decimal_builder.append_null()?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
+ let array: ArrayRef = Arc::new(
+ std::iter::repeat(None)
+ .take(6)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(10, 0)?,
+ );
let result = sum_batch(&array)?;
assert_eq!(ScalarValue::Decimal128(None, 10, 0), result);
// test agg
- let mut decimal_builder = DecimalBuilder::new(5, 10, 0);
- for _i in 1..6 {
- decimal_builder.append_null()?;
- }
- let array: ArrayRef = Arc::new(decimal_builder.finish());
generic_test_op!(
array,
DataType::Decimal(10, 0),
diff --git a/datafusion/src/physical_plan/hash_utils.rs
b/datafusion/src/physical_plan/hash_utils.rs
index 00073a6..4e503b1 100644
--- a/datafusion/src/physical_plan/hash_utils.rs
+++ b/datafusion/src/physical_plan/hash_utils.rs
@@ -564,7 +564,6 @@ pub fn create_hashes<'a>(
#[cfg(test)]
mod tests {
use crate::from_slice::FromSlice;
- use arrow::array::DecimalBuilder;
use arrow::{array::DictionaryArray, datatypes::Int8Type};
use std::sync::Arc;
@@ -572,14 +571,15 @@ mod tests {
#[test]
fn create_hashes_for_decimal_array() -> Result<()> {
- let mut builder = DecimalBuilder::new(4, 20, 3);
- let array: Vec<i128> = vec![1, 2, 3, 4];
- for value in &array {
- builder.append_value(*value)?;
- }
- let array_ref = Arc::new(builder.finish());
+ let array = vec![1, 2, 3, 4]
+ .into_iter()
+ .map(Some)
+ .collect::<DecimalArray>()
+ .with_precision_and_scale(20, 3)
+ .unwrap();
+ let array_ref = Arc::new(array);
let random_state = RandomState::with_seeds(0, 0, 0, 0);
- let hashes_buff = &mut vec![0; array.len()];
+ let hashes_buff = &mut vec![0; array_ref.len()];
let hashes = create_hashes(&[array_ref], &random_state, hashes_buff)?;
assert_eq!(hashes.len(), 4);
Ok(())
diff --git a/datafusion/src/scalar.rs b/datafusion/src/scalar.rs
index 7dc9475..774b8eb 100644
--- a/datafusion/src/scalar.rs
+++ b/datafusion/src/scalar.rs
@@ -17,9 +17,7 @@
//! ScalarValue reimported from datafusion-common
-pub use datafusion_common::{
- ScalarType, ScalarValue, MAX_PRECISION_FOR_DECIMAL128,
MAX_SCALE_FOR_DECIMAL128,
-};
+pub use datafusion_common::{ScalarType, ScalarValue};
#[cfg(test)]
mod tests {
diff --git a/datafusion/src/sql/utils.rs b/datafusion/src/sql/utils.rs
index cbe40d6..8ec0a49 100644
--- a/datafusion/src/sql/utils.rs
+++ b/datafusion/src/sql/utils.rs
@@ -17,12 +17,12 @@
//! SQL Utility Functions
-use arrow::datatypes::DataType;
+use arrow::datatypes::{DataType, DECIMAL_MAX_PRECISION};
use sqlparser::ast::Ident;
use crate::logical_plan::ExprVisitable;
use crate::logical_plan::{Expr, LogicalPlan};
-use crate::scalar::{ScalarValue, MAX_PRECISION_FOR_DECIMAL128};
+use crate::scalar::ScalarValue;
use crate::{
error::{DataFusionError, Result},
logical_plan::{Column, ExpressionVisitor, Recursion},
@@ -522,7 +522,7 @@ pub(crate) fn make_decimal_type(
}
(Some(p), Some(s)) => {
// Arrow decimal is i128 meaning 38 maximum decimal digits
- if (p as usize) > MAX_PRECISION_FOR_DECIMAL128 || s > p {
+ if (p as usize) > DECIMAL_MAX_PRECISION || s > p {
return Err(DataFusionError::Internal(format!(
"For decimal(precision, scale) precision must be less than
or equal to 38 and scale can't be greater than precision. Got ({}, {})",
p, s
