lidavidm commented on a change in pull request #12139:
URL: https://github.com/apache/arrow/pull/12139#discussion_r800081066



##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -214,6 +214,62 @@ struct SubtractChecked {
   }
 };
 
+template <bool is_32bit, int64_t multiple>
+struct SubtractTimeDuration {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_t<is_32bit, T> Call(KernelContext*, Arg0 left, Arg1 right,
+                                       Status* st) {
+    T result = arrow::internal::SafeSignedSubtract(left, 
static_cast<T>(right));
+    if (result < 0) {
+      *st = Status::Invalid(result, " is not within the acceptable range of ", 
"[0, ",
+                            multiple, ") s");
+    }

Review comment:
       I found this reference: https://bugs.python.org/issue1487389#msg54803
   
   I think raising an error is OK. If we need wrap-around, we can add that as a 
separate kernel perhaps.

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -214,6 +214,62 @@ struct SubtractChecked {
   }
 };
 
+template <bool is_32bit, int64_t multiple>
+struct SubtractTimeDuration {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_t<is_32bit, T> Call(KernelContext*, Arg0 left, Arg1 right,
+                                       Status* st) {
+    T result = arrow::internal::SafeSignedSubtract(left, 
static_cast<T>(right));
+    if (result < 0) {
+      *st = Status::Invalid(result, " is not within the acceptable range of ", 
"[0, ",
+                            multiple, ") s");
+    }

Review comment:
       We should still raise an error if the result is >24:00 though.




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


Reply via email to