bkietz commented on code in PR #46792:
URL: https://github.com/apache/arrow/pull/46792#discussion_r2150698929


##########
cpp/src/arrow/status.h:
##########
@@ -350,6 +343,30 @@ class ARROW_EXPORT [[nodiscard]] Status : public 
util::EqualityComparable<Status
     return FromArgs(code(), std::forward<Args>(args)...).WithDetail(detail());
   }
 
+  /// \brief Apply a functor if the status indicates an error
+  ///
+  /// If the status indicates a success, it is returned as-is.
+  ///
+  /// If the status indicates an error, the given functor is called with the 
status
+  /// as argument.
+  /// If the functor returns a new Status, it is returned.
+  /// If the functor returns a Status-compatible object such as Result<T>, it 
is
+  /// converted to Status and returned.
+  /// If the functor returns void, the original Status is returned.
+  template <typename OnError>
+  Status OrElse(OnError&& on_error) {
+    using RT = decltype(on_error(Status(StatusCode::UnknownError, "")));

Review Comment:
   ```suggestion
       using RT = decltype(on_error(Status()));
   ```
   Since this is unevaluated, we don't need to force an error status



##########
cpp/src/arrow/status.h:
##########
@@ -463,13 +480,16 @@ Status& Status::operator&=(Status&& s) noexcept {
 }
 /// \endcond
 
-namespace internal {
-
-// Extract Status from Status or Result<T>
-// Useful for the status check macros such as RETURN_NOT_OK.
-inline const Status& GenericToStatus(const Status& st) { return st; }
-inline Status GenericToStatus(Status&& st) { return std::move(st); }
+constexpr inline const Status& ToArrowStatus(const Status& st) { return st; }

Review Comment:
   What you're proposing here is essentially a trait; in order to extend 
`my_namespace::Err` with convertibility to status, we implement 
`my_namespace::ToArrowStatus`. This extensibility sounds quite useful, but it 
might be more robust and easier to explain if it were provided as a 
user-specializable template struct as with many other traits:
   
   ```c++
   namespace arrow::internal {
   template <typename T>
   struct IntoStatus;
   }
   
   
   namespace arrow {
   /// The actual entry point
   template <typename T>
   constexpr decltype(auto) ToStatus(T&& t) {
     return IntoStatus<T>::ToStatus(std::forward<T>(t));
   }
   }
   
   namespace arrow::internal {
   template <>
   struct IntoStatus<Status> {
     static constexpr Status& ToStatus(const Status& st) { return st; }
     static constexpr Status&& ToStatus(Status&& st) { return st; }
   };
   
   template <typename T>
   struct IntoStatus<Result<T>> {
     static constexpr Status& ToStatus(const Result<T>& r) { return r.status(); 
}
     static constexpr Status&& ToStatus(Result<T>&& st) { return 
std::move(r).status(); }
   };
   } // namespace arrow
   
   // ...
   
   namespace arrow::internal {
   template <>
   struct IntoStatus<my_namespace::StatusLike> {
     static Status ToStatus(my_namespace::StatusLike v) {
       if (v.value == 42) {
         return Status::OK();
       } else {
         return Status::UnknownError("StatusLike: ", v.value);
       }
     }
   };
   } // namespace arrow::internal
   ```



##########
cpp/src/arrow/status.h:
##########
@@ -350,6 +343,30 @@ class ARROW_EXPORT [[nodiscard]] Status : public 
util::EqualityComparable<Status
     return FromArgs(code(), std::forward<Args>(args)...).WithDetail(detail());
   }
 
+  /// \brief Apply a functor if the status indicates an error
+  ///
+  /// If the status indicates a success, it is returned as-is.
+  ///
+  /// If the status indicates an error, the given functor is called with the 
status
+  /// as argument.
+  /// If the functor returns a new Status, it is returned.
+  /// If the functor returns a Status-compatible object such as Result<T>, it 
is
+  /// converted to Status and returned.
+  /// If the functor returns void, the original Status is returned.
+  template <typename OnError>
+  Status OrElse(OnError&& on_error) {
+    using RT = decltype(on_error(Status(StatusCode::UnknownError, "")));
+    if (ARROW_PREDICT_TRUE(ok())) {
+      return *this;
+    }
+    if constexpr (std::is_same_v<RT, void>) {

Review Comment:
   ```suggestion
       if constexpr (std::is_void_v<RT>) {
   ```



-- 
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...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to