kazantsev-maksim commented on code in PR #5280:
URL: https://github.com/apache/datafusion-comet/pull/5280#discussion_r3741121176
##########
native/spark-expr/src/math_funcs/checked_arithmetic.rs:
##########
@@ -29,40 +30,66 @@ use datafusion::common::DataFusionError;
use datafusion::physical_plan::ColumnarValue;
use std::sync::Arc;
-pub fn try_arithmetic_kernel<T>(
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum MathOp {
+ Add,
+ Sub,
+ Mul,
+ Div,
+}
+
+fn try_arithmetic_kernel<T>(
left: &PrimitiveArray<T>,
right: &PrimitiveArray<T>,
- op: &str,
is_ansi_mode: bool,
+ op: MathOp,
) -> Result<ArrayRef, DataFusionError>
where
T: ArrowPrimitiveType,
{
match op {
- "checked_add" => checked_binary(left, right, is_ansi_mode, false, |l,
r| l.add_checked(r)),
- "checked_sub" => checked_binary(left, right, is_ansi_mode, false, |l,
r| l.sub_checked(r)),
- "checked_mul" => checked_binary(left, right, is_ansi_mode, false, |l,
r| l.mul_checked(r)),
- "checked_div" => checked_binary(left, right, is_ansi_mode, true, |l,
r| l.div_checked(r)),
- _ => Err(DataFusionError::Internal(format!(
- "Unsupported operation: {:?}",
- op
- ))),
+ MathOp::Add => checked_binary(left, right, is_ansi_mode, |l, r|
l.add_checked(r)),
+ MathOp::Sub => checked_binary(left, right, is_ansi_mode, |l, r|
l.sub_checked(r)),
+ MathOp::Mul => checked_binary(left, right, is_ansi_mode, |l, r|
l.mul_checked(r)),
+ MathOp::Div => checked_binary(left, right, is_ansi_mode, |l, r|
l.div_checked(r)),
}
}
+fn ansi_arithmetic_kernel<T>(
+ left: &PrimitiveArray<T>,
+ right: &PrimitiveArray<T>,
+ op: MathOp,
+) -> Result<ArrayRef, DataFusionError>
+where
+ T: ArrowPrimitiveType,
+{
+ let result_array = match op {
+ MathOp::Add => numeric::add(left, right),
+ MathOp::Sub => numeric::sub(left, right),
+ MathOp::Mul => numeric::mul(left, right),
+ MathOp::Div => numeric::div(left, right),
+ };
+
+ result_array.map_err(|e| match e {
+ ArrowError::DivideByZero => divide_by_zero_error().into(),
+ _ => DataFusionError::from(SparkError::ArithmeticOverflow {
+ from_type: String::from("integer"),
+ }),
+ })
+}
+
fn checked_binary<T, F>(
Review Comment:
Great catch! You're completely right. To address this and make the
separation crystal clear, I've pulled that float-specific ANSI path out into a
dedicated helper function called ansi_float_div.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
