Michael Ho created IMPALA-6309:
----------------------------------
Summary: Clean up usage of Status(TErrorCode::type error, const
ArgType& arg0, ...)
Key: IMPALA-6309
URL: https://issues.apache.org/jira/browse/IMPALA-6309
Project: IMPALA
Issue Type: Task
Components: Backend
Affects Versions: Impala 2.11.0
Reporter: Michael Ho
Priority: Minor
As shown in IMPALA-6308, the following idiom of not using hard coded
TErrorCode::type with {{Status(TErrorCode::type error, const ArgType& arg0,
...)}} is pretty error prone as the format string changes. We should consider
replacing {{error_code}} with constant {{TErrorCode::type}} like most other
places in the code.
{noformat}
void HdfsAvroScanner::SetStatusCorruptData(TErrorCode::type error_code) {
DCHECK(parse_status_.ok());
if (TestInfo::is_test()) {
parse_status_ = Status(error_code, "test file", 123);
} else {
parse_status_ = Status(error_code, stream_->filename(),
stream_->file_offset());
}
}
void HdfsAvroScanner::SetStatusInvalidValue(TErrorCode::type error_code,
int64_t len) {
DCHECK(parse_status_.ok());
if (TestInfo::is_test()) {
parse_status_ = Status(error_code, "test file", len, 123);
} else {
parse_status_ = Status(error_code, stream_->filename(), len,
stream_->file_offset());
}
}
void HdfsAvroScanner::SetStatusValueOverflow(TErrorCode::type error_code,
int64_t len,
int64_t limit) {
DCHECK(parse_status_.ok());
if (TestInfo::is_test()) {
parse_status_ = Status(error_code, "test file", len, limit, 123);
} else {
parse_status_ = Status(error_code, stream_->filename(), len, limit,
stream_->file_offset());
}
}
{noformat}
In general, {{Status(TErrorCode::type error, const ArgType& arg0,..)}} relies
on the caller to pass the right number of arguments to match the number of
substitution argument. In theory, if our test coverage is comprehensive, we
should catch cases in which there is any mismatch. However, it's unclear if all
usages of {{Status(TErrorCode::type error, const ArgType& arg0,..)}} are
exercised. While the code coverage is the bigger issue here, it'd be nice to
implement some compilation check to catch any mismatch between the number of
arguments passed to {{Status()}} and the number of substitution arguments in
the format string. This may be a follow on change after the above clean up is
done.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
