From fecfdd78bf7b218e13bb673016c2b58b23d16fd5 Mon Sep 17 00:00:00 2001 From: plat1ko Date: Tue, 16 Aug 2022 14:40:35 +0800 Subject: [PATCH] [enhancement](status) Fix Status related macros to enable RVO or move ctor (#11753) --- be/src/common/status.cpp | 21 ++++++++------------- be/src/common/status.h | 30 +++++++++++++++--------------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/be/src/common/status.cpp b/be/src/common/status.cpp index a56d99606f..ca1a148706 100644 --- a/be/src/common/status.cpp +++ b/be/src/common/status.cpp @@ -39,33 +39,28 @@ public: Initializer init; // Used to init the error_states array Status::Status(const TStatus& s) { - if (s.status_code != TStatusCode::OK) { + _code = s.status_code; + if (_code != TStatusCode::OK) { // It is ok to set precise code == 1 here, because we do not know the precise code // just from thrift's TStatus - _code = s.status_code; _precise_code = 1; if (!s.error_msgs.empty()) { _err_msg = s.error_msgs[0]; } - } else { - _code = 0; } } // TODO yiguolei, maybe should init PStatus's precise code because OLAPInternal Error may // tranfer precise code during BRPC Status::Status(const PStatus& s) { - TStatusCode::type code = (TStatusCode::type)s.status_code(); - if (code != TStatusCode::OK) { + _code = (TStatusCode::type)s.status_code(); + if (_code != TStatusCode::OK) { // It is ok to set precise code == 1 here, because we do not know the precise code // just from thrift's TStatus - _code = code; _precise_code = 1; if (s.error_msgs_size() > 0) { _err_msg = s.error_msgs(0); } - } else { - _code = 0; } } @@ -113,7 +108,7 @@ void Status::to_protobuf(PStatus* s) const { } } -std::string Status::code_as_string() const { +const char* Status::code_as_string() const { switch (code()) { case TStatusCode::OK: return "OK"; @@ -172,10 +167,10 @@ std::string Status::code_as_string() const { case TStatusCode::DATA_QUALITY_ERROR: return "Data quality error"; default: { - return fmt::format("Unknown code({})", code()); + return "Unknown code"; } } - return std::string(); + return "Unknown code"; } std::string Status::to_string() const { @@ -212,7 +207,7 @@ std::string Status::to_json() const { writer.StartObject(); // status writer.Key("status"); - writer.String(code_as_string().c_str()); + writer.String(code_as_string()); // msg writer.Key("msg"); if (ok()) { diff --git a/be/src/common/status.h b/be/src/common/status.h index 285d84c478..6dc42e70fa 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -241,7 +241,7 @@ enum ErrorCode { class Status { public: - Status() : _code(0) {} + Status() : _code(TStatusCode::OK), _precise_code(0) {} // copy c'tor makes copy of error detail so Status can be returned by value Status(const Status& rhs) = default; @@ -393,7 +393,7 @@ public: static Status ConstructErrorStatus(int16_t precise_code); - bool ok() const { return _code == 0; } + bool ok() const { return _code == TStatusCode::OK; } bool is_cancelled() const { return code() == TStatusCode::CANCELLED; } bool is_mem_limit_exceeded() const { return code() == TStatusCode::MEM_LIMIT_EXCEEDED; } @@ -448,7 +448,7 @@ public: /// @return A string representation of the status code, without the message /// text or sub code information. - std::string code_as_string() const; + const char* code_as_string() const; TStatusCode::type code() const { return ok() ? TStatusCode::OK : static_cast(_code); @@ -490,7 +490,7 @@ public: } private: - int8_t _code; + TStatusCode::type _code; int16_t _precise_code; std::string _err_msg; }; @@ -502,18 +502,18 @@ inline std::ostream& operator<<(std::ostream& ostr, const Status& param) { } // some generally useful macros -#define RETURN_IF_ERROR(stmt) \ - do { \ - const Status& _status_ = (stmt); \ - if (UNLIKELY(!_status_.ok())) { \ - return _status_; \ - } \ +#define RETURN_IF_ERROR(stmt) \ + do { \ + Status _status_ = (stmt); \ + if (UNLIKELY(!_status_.ok())) { \ + return _status_; \ + } \ } while (false) // End _get_next_span after last call to get_next method #define RETURN_IF_ERROR_AND_CHECK_SPAN(stmt, get_next_span, done) \ do { \ - const auto& _status_ = (stmt); \ + Status _status_ = (stmt); \ auto _span = (get_next_span); \ if (UNLIKELY(_span && (!_status_.ok() || done))) { \ _span->End(); \ @@ -533,7 +533,7 @@ inline std::ostream& operator<<(std::ostream& ostr, const Status& param) { #define EXIT_IF_ERROR(stmt) \ do { \ - const Status& _status_ = (stmt); \ + Status _status_ = (stmt); \ if (UNLIKELY(!_status_.ok())) { \ LOG(ERROR) << _status_.get_error_msg(); \ exit(1); \ @@ -543,7 +543,7 @@ inline std::ostream& operator<<(std::ostream& ostr, const Status& param) { /// @brief Emit a warning if @c to_call returns a bad status. #define WARN_IF_ERROR(to_call, warning_prefix) \ do { \ - const Status& _s = (to_call); \ + Status _s = (to_call); \ if (UNLIKELY(!_s.ok())) { \ LOG(WARNING) << (warning_prefix) << ": " << _s.to_string(); \ } \ @@ -551,7 +551,7 @@ inline std::ostream& operator<<(std::ostream& ostr, const Status& param) { #define RETURN_WITH_WARN_IF_ERROR(stmt, ret_code, warning_prefix) \ do { \ - const Status& _s = (stmt); \ + Status _s = (stmt); \ if (UNLIKELY(!_s.ok())) { \ LOG(WARNING) << (warning_prefix) << ", error: " << _s.to_string(); \ return ret_code; \ @@ -560,7 +560,7 @@ inline std::ostream& operator<<(std::ostream& ostr, const Status& param) { #define RETURN_NOT_OK_STATUS_WITH_WARN(stmt, warning_prefix) \ do { \ - const Status& _s = (stmt); \ + Status _s = (stmt); \ if (UNLIKELY(!_s.ok())) { \ LOG(WARNING) << (warning_prefix) << ", error: " << _s.to_string(); \ return _s; \