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

Reply via email to