Jefffrey commented on code in PR #22564:
URL: https://github.com/apache/datafusion/pull/22564#discussion_r3346144341


##########
datafusion/functions/src/math/log.rs:
##########
@@ -106,10 +101,37 @@ fn is_valid_integer_base(base: f64) -> bool {
     base.trunc() == base && base >= 2.0 && base <= u32::MAX as f64
 }
 
+#[inline]
+fn validate_log_value(value: f64) -> Result<(), ArrowError> {
+    if value == 0.0 {
+        Err(ArrowError::ComputeError(
+            "cannot take logarithm of zero".to_string(),
+        ))
+    } else {
+        Ok(())
+    }
+}
+
+#[inline]
+fn validate_log_base(base: f64) -> Result<(), ArrowError> {
+    if base < 0.0 {
+        Err(ArrowError::ComputeError(
+            "cannot take logarithm of a negative number".to_string(),
+        ))
+    } else if base == 1.0 {
+        Err(ArrowError::ComputeError(
+            "division by zero in based logarithm".to_string(),
+        ))

Review Comment:
   We should combine these checks with `compute_float_log` since we're 
duplicating them; I feel we can easily turn these functions generic over 
`Float` and then reuse them within `compute_float_log` below



-- 
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]

Reply via email to