[refactor](datalake) return the error status instead of static_cast<void> #34873
Conversation
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
TeamCity be ut coverage result: |
| LOG(WARNING) << "fail to close wal " << _file_name << " st= " << st.to_string(); | ||
| } | ||
| if (file_reader) { | ||
| return file_reader->close(); |
There was a problem hiding this comment.
@w41ter PTAL, I not sure why we ignore the error before
There was a problem hiding this comment.
It should have been returned but was left out. This change is LGTM.
| Status init_reader( | ||
| std::unordered_map<std::string, ColumnValueRangeType>* colname_to_value_range); | ||
|
|
||
| Status close() override { |
There was a problem hiding this comment.
Same as parent, no need to override
|
|
||
| TypeDescriptor convert_complex_type(const rapidjson::Document::ConstObject child_schema); | ||
|
|
||
| Status close() override { |
There was a problem hiding this comment.
Same as parent, no need to override
| Status init_reader( | ||
| std::unordered_map<std::string, ColumnValueRangeType>* colname_to_value_range); | ||
|
|
||
| Status close() override { |
There was a problem hiding this comment.
Same as parent, no need to override
| Status init_reader( | ||
| std::unordered_map<std::string, ColumnValueRangeType>* colname_to_value_range); | ||
|
|
||
| Status close() override { |
There was a problem hiding this comment.
Same as parent, no need to override
| Status init_reader( | ||
| std::unordered_map<std::string, ColumnValueRangeType>* colname_to_value_range); | ||
|
|
||
| Status close() override { |
| _wal_id = state->wal_id(); | ||
| } | ||
|
|
||
| WalReader::~WalReader() { |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
TeamCity be ut coverage result: |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
…oid> (#34873) Followup #34797 `static_cast<void>` has ignored the wrong status, some of them should make the query finished with error status, so replace `static_cast<void>` with `RETURN_IF_ERROR`. The following three scenarios need to be handled separately and cannot be simply replaced: 1. The outer function returns void; 2. Call status function inner constructors or destructors; 3. Call status function with best effort, and should ignore the wrong status.
…oid> (#34873) Followup #34797 `static_cast<void>` has ignored the wrong status, some of them should make the query finished with error status, so replace `static_cast<void>` with `RETURN_IF_ERROR`. ### Remaining Works The following three scenarios need to be handled separately and cannot be simply replaced: 1. The outer function returns void; 2. Call status function inner constructors or destructors; 3. Call status function with best effort, and should ignore the wrong status.
5 participants
Proposed changes
Followup #34797
static_cast<void>has ignored the wrong status, some of them should make the query finished with error status, so replacestatic_cast<void>withRETURN_IF_ERROR.Remaining Works
The following three scenarios need to be handled separately and cannot be simply replaced:
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...