[Fix](status)Fix leaky abstraction and shield the status code END_OF_FILE from upper layers
#24165
Conversation
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
bobhan1
changed the title
END_OF_FILE from upper layers
|
clang-tidy review says "All clean, LGTM! 👍" |
bobhan1
changed the title
END_OF_FILE from upper layersEND_OF_FILE from upper layers
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
| } | ||
| if (!st.ok()) { | ||
| return st; | ||
| } | ||
| } |
There was a problem hiding this comment.
should return a Status::OK() at the end of this method?
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. |
| @@ -202,7 +202,13 @@ Status BinaryPrefixPageDecoder::seek_at_or_after_value(const void* value, bool* | |||
| return Status::OK(); | |||
| } | |||
| _cur_pos++; | |||
| RETURN_IF_ERROR(_read_next_value()); | |||
| auto st = _read_next_value(); | |||
| if (st.is<ErrorCode::END_OF_FILE>()) { | |||
There was a problem hiding this comment.
Does it have an impact on other users of this code？
There was a problem hiding this comment.
only IndexedColumnIterator::seek_at_or_after may call BinaryPrefixPageDecoder::seek_at_or_after_value
…_FILE` from upper layers (#24165)
…_FILE` from upper layers (#24165)
Proposed changes
In #23855 and #24139, the low level status code
END_OF_FILEis leaked to upper layers and causes some problems. This PR shields the status codeEND_OF_FILEfrom upper layers and transforms it to the status code with appropriate semantics.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...