IMPALA-5433: Mark single-argument Status c'tors as explicit Avoid unexpected errors by marking Status c'tors as explicit. Per Google style guide, >1 arg c'tors are not marked as explicit to allow for copy-initialization.
Change-Id: I64cb51151673566df973db7b0dd574b7ead27377 Testing: compiled impalad and be-test. Reviewed-on: http://gerrit.cloudera.org:8080/7077 Reviewed-by: Henry Robinson <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/f0065d37 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/f0065d37 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/f0065d37 Branch: refs/heads/master Commit: f0065d376f9a0cfbefc20164c87137df56363166 Parents: f59371d Author: Henry Robinson <[email protected]> Authored: Sun Jun 4 22:26:35 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Jun 6 02:07:53 2017 +0000 ---------------------------------------------------------------------- be/src/common/status.h | 24 +++++++++++------------- be/src/exec/hdfs-parquet-scanner.cc | 3 ++- be/src/runtime/coordinator.cc | 2 +- be/src/util/codec.cc | 2 +- 4 files changed, 15 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0065d37/be/src/common/status.h ---------------------------------------------------------------------- diff --git a/be/src/common/status.h b/be/src/common/status.h index 9fa303b..a48470b 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -107,7 +107,7 @@ class Status { /// Status using only the error code as a parameter. This can be used for error messages /// that don't take format parameters. - Status(TErrorCode::type code); + explicit Status(TErrorCode::type code); /// These constructors are used if the caller wants to indicate a non-successful /// execution and supply a client-facing error message. This is the preferred way of @@ -140,12 +140,20 @@ class Status { /// Used when the ErrorMsg is created as an intermediate value that is either passed to /// the Status or to the RuntimeState. - Status(const ErrorMsg& e); + explicit Status(const ErrorMsg& e); /// This constructor creates a Status with a default error code of GENERAL and is not /// intended for statuses that might be client-visible. /// TODO: deprecate - Status(const std::string& error_msg); + explicit Status(const std::string& error_msg); + + /// "Copy" c'tor from TStatus. + /// Retains the TErrorCode value and the message + explicit Status(const TStatus& status); + + /// "Copy c'tor from HS2 TStatus. + /// Retains the TErrorCode value and the message + explicit Status(const apache::hive::service::cli::thrift::TStatus& hs2_status); /// Create a status instance that represents an expected error and will not be logged static Status Expected(const ErrorMsg& e); @@ -176,19 +184,9 @@ class Status { if (UNLIKELY(msg_ != NULL)) FreeMessage(); } - /// "Copy" c'tor from TStatus. - /// Retains the TErrorCode value and the message - Status(const TStatus& status); - - /// same as previous c'tor /// Retains the TErrorCode value and the message Status& operator=(const TStatus& status); - /// "Copy c'tor from HS2 TStatus. - /// Retains the TErrorCode value and the message - Status(const apache::hive::service::cli::thrift::TStatus& hs2_status); - - /// same as previous c'tor /// Retains the TErrorCode value and the message Status& operator=(const apache::hive::service::cli::thrift::TStatus& hs2_status); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0065d37/be/src/exec/hdfs-parquet-scanner.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-parquet-scanner.cc b/be/src/exec/hdfs-parquet-scanner.cc index 4e2c5fd..634d4ec 100644 --- a/be/src/exec/hdfs-parquet-scanner.cc +++ b/be/src/exec/hdfs-parquet-scanner.cc @@ -958,10 +958,11 @@ Status HdfsParquetScanner::AssembleRows( DCHECK(scratch_batch_->AtEnd()); *skip_row_group = true; if (num_tuples_mismatch && continue_execution) { - parse_status_.MergeStatus(Substitute("Corrupt Parquet file '$0': column '$1' " + Status err(Substitute("Corrupt Parquet file '$0': column '$1' " "had $2 remaining values but expected $3", filename(), col_reader->schema_element().name, last_num_tuples, scratch_batch_->num_tuples)); + parse_status_.MergeStatus(err); } return Status::OK(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0065d37/be/src/runtime/coordinator.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/coordinator.cc b/be/src/runtime/coordinator.cc index 5b86690..3c5ffbe 100644 --- a/be/src/runtime/coordinator.cc +++ b/be/src/runtime/coordinator.cc @@ -651,7 +651,7 @@ Status Coordinator::FinalizeSuccessfulInsert() { // when the directory is empty(HDFS-8407). Need to check errno to make sure // the call fails. if (existing_files == nullptr && errno != 0) { - return GetHdfsErrorMsg("Could not list directory: ", part_path); + return Status(GetHdfsErrorMsg("Could not list directory: ", part_path)); } for (int i = 0; i < num_files; ++i) { const string filename = path(existing_files[i].mName).filename().string(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0065d37/be/src/util/codec.cc ---------------------------------------------------------------------- diff --git a/be/src/util/codec.cc b/be/src/util/codec.cc index c4d1d8b..8c5cc90 100644 --- a/be/src/util/codec.cc +++ b/be/src/util/codec.cc @@ -156,7 +156,7 @@ Status Codec::CreateDecompressor(MemPool* mem_pool, bool reuse, break; default: { if (format == THdfsCompression::LZO) return Status(NO_LZO_MSG); - return Substitute("Unsupported codec: $0", format); + return Status(Substitute("Unsupported codec: $0", format)); } }
