You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by tu...@apache.org on 2023/06/29 07:12:01 UTC
[arrow-datafusion] branch main updated: Use checked division kernel (#6792)
This is an automated email from the ASF dual-hosted git repository.
tustvold pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 283b8a17d8 Use checked division kernel (#6792)
283b8a17d8 is described below
commit 283b8a17d8cea25d2b2cacb048410ca78d95ec2b
Author: Raphael Taylor-Davies <17...@users.noreply.github.com>
AuthorDate: Thu Jun 29 08:11:54 2023 +0100
Use checked division kernel (#6792)
---
.../src/simplify_expressions/expr_simplifier.rs | 7 +++-
datafusion/physical-expr/src/expressions/binary.rs | 44 +++++++++++++++-------
.../src/expressions/binary/kernels_arrow.rs | 8 ++--
3 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
index 884b39fd57..96a31212c6 100644
--- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
+++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
@@ -1670,9 +1670,12 @@ mod tests {
fn test_simplify_divide_zero_by_zero() {
// 0 / 0 -> null
let expr = lit(0) / lit(0);
- let expected = lit(ScalarValue::Int32(None));
+ let err = try_simplify(expr).unwrap_err();
- assert_eq!(simplify(expr), expected);
+ assert!(
+ matches!(err, DataFusionError::ArrowError(ArrowError::DivideByZero)),
+ "{err}"
+ );
}
#[test]
diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs
index c764692da3..a21ae21ca9 100644
--- a/datafusion/physical-expr/src/expressions/binary.rs
+++ b/datafusion/physical-expr/src/expressions/binary.rs
@@ -24,7 +24,7 @@ use std::{any::Any, sync::Arc};
use arrow::array::*;
use arrow::compute::kernels::arithmetic::{
- add_dyn, add_scalar_dyn as add_dyn_scalar, divide_dyn_opt,
+ add_dyn, add_scalar_dyn as add_dyn_scalar, divide_dyn_checked,
divide_scalar_dyn as divide_dyn_scalar, modulus_dyn,
modulus_scalar_dyn as modulus_dyn_scalar, multiply_dyn,
multiply_scalar_dyn as multiply_dyn_scalar, subtract_dyn,
@@ -63,7 +63,7 @@ use kernels::{
};
use kernels_arrow::{
add_decimal_dyn_scalar, add_dyn_decimal, add_dyn_temporal, divide_decimal_dyn_scalar,
- divide_dyn_opt_decimal, is_distinct_from, is_distinct_from_binary,
+ divide_dyn_checked_decimal, is_distinct_from, is_distinct_from_binary,
is_distinct_from_bool, is_distinct_from_decimal, is_distinct_from_f32,
is_distinct_from_f64, is_distinct_from_null, is_distinct_from_utf8,
is_not_distinct_from, is_not_distinct_from_binary, is_not_distinct_from_bool,
@@ -1223,7 +1223,12 @@ impl BinaryExpr {
binary_primitive_array_op_dyn!(left, right, multiply_dyn, result_type)
}
Divide => {
- binary_primitive_array_op_dyn!(left, right, divide_dyn_opt, result_type)
+ binary_primitive_array_op_dyn!(
+ left,
+ right,
+ divide_dyn_checked,
+ result_type
+ )
}
Modulo => {
binary_primitive_array_op_dyn!(left, right, modulus_dyn, result_type)
@@ -1342,6 +1347,7 @@ mod tests {
use arrow::datatypes::{
ArrowNumericType, Decimal128Type, Field, Int32Type, SchemaRef,
};
+ use arrow_schema::ArrowError;
use datafusion_common::{ColumnStatistics, Result, Statistics};
use datafusion_expr::type_coercion::binary::get_input_types;
@@ -4336,27 +4342,31 @@ mod tests {
Field::new("a", DataType::Int32, true),
Field::new("b", DataType::Int32, true),
]));
- let a = Arc::new(Int32Array::from(vec![8, 32, 128, 512, 2048, 100]));
- let b = Arc::new(Int32Array::from(vec![2, 4, 8, 16, 32, 0]));
+ let a = Arc::new(Int32Array::from(vec![100]));
+ let b = Arc::new(Int32Array::from(vec![0]));
- apply_arithmetic::<Int32Type>(
+ let err = apply_arithmetic::<Int32Type>(
schema,
vec![a, b],
Operator::Divide,
- Int32Array::from(vec![Some(4), Some(8), Some(16), Some(32), Some(64), None]),
- )?;
+ Int32Array::from(vec![Some(4), Some(8), Some(16), Some(32), Some(64)]),
+ )
+ .unwrap_err();
+
+ assert!(
+ matches!(err, DataFusionError::ArrowError(ArrowError::DivideByZero)),
+ "{err}"
+ );
// decimal
let schema = Arc::new(Schema::new(vec![
Field::new("a", DataType::Decimal128(25, 3), true),
Field::new("b", DataType::Decimal128(25, 3), true),
]));
- let left_decimal_array =
- Arc::new(create_decimal_array(&[Some(1234567), Some(1234567)], 25, 3));
- let right_decimal_array =
- Arc::new(create_decimal_array(&[Some(10), Some(0)], 25, 3));
+ let left_decimal_array = Arc::new(create_decimal_array(&[Some(1234567)], 25, 3));
+ let right_decimal_array = Arc::new(create_decimal_array(&[Some(0)], 25, 3));
- apply_arithmetic::<Decimal128Type>(
+ let err = apply_arithmetic::<Decimal128Type>(
schema,
vec![left_decimal_array, right_decimal_array],
Operator::Divide,
@@ -4365,7 +4375,13 @@ mod tests {
38,
29,
),
- )?;
+ )
+ .unwrap_err();
+
+ assert!(
+ matches!(err, DataFusionError::ArrowError(ArrowError::DivideByZero)),
+ "{err}"
+ );
Ok(())
}
diff --git a/datafusion/physical-expr/src/expressions/binary/kernels_arrow.rs b/datafusion/physical-expr/src/expressions/binary/kernels_arrow.rs
index ab1b4b376f..1f59fcd141 100644
--- a/datafusion/physical-expr/src/expressions/binary/kernels_arrow.rs
+++ b/datafusion/physical-expr/src/expressions/binary/kernels_arrow.rs
@@ -19,7 +19,7 @@
//! destined for arrow-rs but are in datafusion until they are ported.
use arrow::compute::{
- add_dyn, add_scalar_dyn, divide_dyn_opt, divide_scalar_dyn, modulus_dyn,
+ add_dyn, add_scalar_dyn, divide_dyn_checked, divide_scalar_dyn, modulus_dyn,
modulus_scalar_dyn, multiply_dyn, multiply_fixed_point, multiply_scalar_dyn,
subtract_dyn, subtract_scalar_dyn, try_unary,
};
@@ -847,7 +847,7 @@ pub(crate) fn multiply_dyn_decimal(
decimal_array_with_precision_scale(array, precision, scale)
}
-pub(crate) fn divide_dyn_opt_decimal(
+pub(crate) fn divide_dyn_checked_decimal(
left: &dyn Array,
right: &dyn Array,
result_type: &DataType,
@@ -860,7 +860,7 @@ pub(crate) fn divide_dyn_opt_decimal(
// Restore to original precision and scale (metadata only)
let (org_precision, org_scale) = get_precision_scale(right.data_type())?;
let array = decimal_array_with_precision_scale(array, org_precision, org_scale)?;
- let array = divide_dyn_opt(&array, right)?;
+ let array = divide_dyn_checked(&array, right)?;
decimal_array_with_precision_scale(array, precision, scale)
}
@@ -2352,7 +2352,7 @@ mod tests {
25,
3,
);
- let result = divide_dyn_opt_decimal(
+ let result = divide_dyn_checked_decimal(
&left_decimal_array,
&right_decimal_array,
&result_type,