parthchandra commented on code in PR #2136: URL: https://github.com/apache/datafusion-comet/pull/2136#discussion_r2286485784
########## native/spark-expr/src/math_funcs/checked_arithmetic.rs: ########## @@ -84,39 +141,56 @@ where pub fn checked_add( args: &[ColumnarValue], data_type: &DataType, + eval_mode: EvalMode, ) -> Result<ColumnarValue, DataFusionError> { - checked_arithmetic_internal(args, data_type, "checked_add") + checked_arithmetic_internal(args, data_type, "checked_add", eval_mode) } pub fn checked_sub( args: &[ColumnarValue], data_type: &DataType, + eval_mode: EvalMode, ) -> Result<ColumnarValue, DataFusionError> { - checked_arithmetic_internal(args, data_type, "checked_sub") + checked_arithmetic_internal(args, data_type, "checked_sub", eval_mode) } pub fn checked_mul( args: &[ColumnarValue], data_type: &DataType, + eval_mode: EvalMode, ) -> Result<ColumnarValue, DataFusionError> { - checked_arithmetic_internal(args, data_type, "checked_mul") + checked_arithmetic_internal(args, data_type, "checked_mul", eval_mode) } pub fn checked_div( args: &[ColumnarValue], data_type: &DataType, + eval_mode: EvalMode, ) -> Result<ColumnarValue, DataFusionError> { - checked_arithmetic_internal(args, data_type, "checked_div") + checked_arithmetic_internal(args, data_type, "checked_div", eval_mode) } fn checked_arithmetic_internal( args: &[ColumnarValue], data_type: &DataType, op: &str, + eval_mode: EvalMode, ) -> Result<ColumnarValue, DataFusionError> { let left = &args[0]; let right = &args[1]; + let is_ansi_mode = match eval_mode { + EvalMode::Try => false, + EvalMode::Ansi => true, + _ => { + return Err(DataFusionError::Internal(format!( + "Unsupported mode : {:?}", + eval_mode + ))) + } + }; + println!("Eval mode {:?} operation : {}", eval_mode, op); Review Comment: maybe change this to `debug!`? ########## native/spark-expr/src/math_funcs/checked_arithmetic.rs: ########## @@ -27,19 +30,33 @@ pub fn try_arithmetic_kernel<T>( left: &PrimitiveArray<T>, right: &PrimitiveArray<T>, op: &str, + is_ansi_mode: bool, ) -> Result<ArrayRef, DataFusionError> where T: ArrowPrimitiveType, { let len = left.len(); let mut builder = PrimitiveBuilder::<T>::with_capacity(len); + let error_msg = format!("{} : [ARITHMETIC_OVERFLOW] integer overflow. Use 'try_{}' to tolerate overflow and return NULL instead", op, op.split("_").last().unwrap()); Review Comment: Is this used? (Also, if this is meant to bubble up into the logs, then perhaps the message should be more user friendly; end user is hardly likely to know how to use try_foo) -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org