Skip to content

Commit

Permalink
[Fix](Status) Handle status code correctly and add a new error code `…
Browse files Browse the repository at this point in the history
…ENTRY_NOT_FOUND` (apache#24139)
  • Loading branch information
bobhan1 authored and xiaokang committed Sep 11, 2023
1 parent e5dac51 commit 78a48df
Show file tree
Hide file tree
Showing 21 changed files with 45 additions and 40 deletions.
2 changes: 2 additions & 0 deletions be/src/common/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ E(INVERTED_INDEX_EVALUATE_SKIPPED, -6007);
E(INVERTED_INDEX_BUILD_WAITTING, -6008);
E(KEY_NOT_FOUND, -6009);
E(KEY_ALREADY_EXISTS, -6010);
E(ENTRY_NOT_FOUND, -6011);
#undef E
} // namespace ErrorCode

Expand Down Expand Up @@ -311,6 +312,7 @@ constexpr bool capture_stacktrace(int code) {
&& code != ErrorCode::TRANSACTION_ALREADY_VISIBLE
&& code != ErrorCode::TOO_MANY_TRANSACTIONS
&& code != ErrorCode::TRANSACTION_ALREADY_COMMITTED
&& code != ErrorCode::ENTRY_NOT_FOUND
&& code != ErrorCode::KEY_NOT_FOUND
&& code != ErrorCode::KEY_ALREADY_EXISTS
&& code != ErrorCode::CANCELLED
Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/delete_bitmap_calculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Status MergeIndexDeleteBitmapCalculatorContext::advance() {

Status MergeIndexDeleteBitmapCalculatorContext::seek_at_or_after(Slice const& key) {
auto st = _iter->seek_at_or_after(&key, &_excat_match);
if (st.is<ErrorCode::NOT_FOUND>()) {
if (st.is<ErrorCode::ENTRY_NOT_FOUND>()) {
return Status::EndOfFile("Reach the end of file");
}
RETURN_IF_ERROR(st);
Expand Down
5 changes: 3 additions & 2 deletions be/src/olap/rowset/segment_v2/binary_dict_page.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
// IWYU pragma: no_include <opentelemetry/common/threadlocal.h>
#include "common/compiler_util.h" // IWYU pragma: keep
#include "common/logging.h"
#include "common/status.h"
#include "gutil/casts.h"
#include "gutil/port.h"
#include "gutil/strings/substitute.h" // for Substitute
Expand Down Expand Up @@ -179,7 +180,7 @@ Status BinaryDictPageBuilder::get_dictionary_page(OwnedSlice* dictionary_page) {
Status BinaryDictPageBuilder::get_first_value(void* value) const {
DCHECK(_finished);
if (_data_page_builder->count() == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
if (_encoding_type != DICT_ENCODING) {
return _data_page_builder->get_first_value(value);
Expand All @@ -191,7 +192,7 @@ Status BinaryDictPageBuilder::get_first_value(void* value) const {
Status BinaryDictPageBuilder::get_last_value(void* value) const {
DCHECK(_finished);
if (_data_page_builder->count() == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
if (_encoding_type != DICT_ENCODING) {
return _data_page_builder->get_last_value(value);
Expand Down
4 changes: 2 additions & 2 deletions be/src/olap/rowset/segment_v2/binary_plain_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,15 @@ class BinaryPlainPageBuilder : public PageBuilder {
Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_offsets.size() == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_first_value);
return Status::OK();
}
Status get_last_value(void* value) const override {
DCHECK(_finished);
if (_offsets.size() == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_last_value);
return Status::OK();
Expand Down
6 changes: 3 additions & 3 deletions be/src/olap/rowset/segment_v2/binary_prefix_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class BinaryPrefixPageBuilder : public PageBuilder {
Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_first_entry);
return Status::OK();
Expand All @@ -81,7 +81,7 @@ class BinaryPrefixPageBuilder : public PageBuilder {
Status get_last_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_last_entry);
return Status::OK();
Expand Down Expand Up @@ -137,7 +137,7 @@ class BinaryPrefixPageDecoder : public PageDecoder {

// read next value at `_cur_pos` and `_next_ptr` into `_current_value`.
// return OK and advance `_next_ptr` on success. `_cur_pos` is not modified.
// return NotFound when no more entry can be read.
// return EndOfFile when no more entry can be read.
// return other error status otherwise.
Status _read_next_value();

Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/rowset/segment_v2/bitmap_index_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class BitmapIndexIterator {
// by `current_ordinal()`, *exact_match is set to indicate whether the
// seeked value exactly matches `value` or not
//
// Returns NotFound when no such value exists (all values in dictionary < `value`).
// Returns Status::Error<ENTRY_NOT_FOUND> when no such value exists (all values in dictionary < `value`).
// Returns other error status otherwise.
Status seek_dictionary(const void* value, bool* exact_match);

Expand Down
8 changes: 4 additions & 4 deletions be/src/olap/rowset/segment_v2/bitshuffle_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ class BitshufflePageBuilder : public PageBuilder {
Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_first_value, SIZE_OF_TYPE);
return Status::OK();
}
Status get_last_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_last_value, SIZE_OF_TYPE);
return Status::OK();
Expand Down Expand Up @@ -332,7 +332,7 @@ class BitShufflePageDecoder : public PageDecoder {
DCHECK(_parsed) << "Must call init() firstly";

if (_num_elements == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}

size_t left = 0;
Expand All @@ -353,7 +353,7 @@ class BitShufflePageDecoder : public PageDecoder {
}
}
if (left >= _num_elements) {
return Status::NotFound("all value small than the value");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("all value small than the value");
}
void* find_value = get_data(left);
if (TypeTraits<Type>::cmp(find_value, value) == 0) {
Expand Down
6 changes: 3 additions & 3 deletions be/src/olap/rowset/segment_v2/frame_of_reference_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,15 @@ class FrameOfReferencePageBuilder : public PageBuilder {

Status get_first_value(void* value) const override {
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_first_val, sizeof(CppType));
return Status::OK();
}

Status get_last_value(void* value) const override {
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_last_val, sizeof(CppType));
return Status::OK();
Expand Down Expand Up @@ -135,7 +135,7 @@ class FrameOfReferencePageDecoder : public PageDecoder {
DCHECK(_parsed) << "Must call init() firstly";
bool found = _decoder->seek_at_or_after_value(value, exact_match);
if (!found) {
return Status::NotFound("not found");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("not found");
}
_cur_index = _decoder->current_index();
return Status::OK();
Expand Down
5 changes: 3 additions & 2 deletions be/src/olap/rowset/segment_v2/index_page.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void IndexPageBuilder::finish(OwnedSlice* body, PageFooterPB* footer) {

Status IndexPageBuilder::get_first_key(Slice* key) const {
if (_count == 0) {
return Status::NotFound("index page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("index page is empty");
}
Slice input(_buffer);
if (get_length_prefixed_slice(&input, key)) {
Expand Down Expand Up @@ -105,7 +105,8 @@ Status IndexPageIterator::seek_at_or_before(const Slice& search_key) {
// no exact match, the insertion point is `left`
if (left == 0) {
// search key is smaller than all keys
return Status::NotFound("given key is smaller than all keys in page");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>(
"given key is smaller than all keys in page");
}
// index entry records the first key of the indexed page,
// therefore the first page with keys >= searched key is the one before the insertion point
Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/rowset/segment_v2/index_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class IndexPageIterator {

// Find the largest index entry whose key is <= search_key.
// Return OK status when such entry exists.
// Return NotFound when no such entry is found (all keys > search_key).
// Return ENTRY_NOT_FOUND when no such entry is found (all keys > search_key).
// Return other error status otherwise.
Status seek_at_or_before(const Slice& search_key);

Expand Down
9 changes: 5 additions & 4 deletions be/src/olap/rowset/segment_v2/indexed_column_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include <algorithm>

#include "common/status.h"
#include "gutil/strings/substitute.h" // for Substitute
#include "olap/key_coder.h"
#include "olap/olap_common.h"
Expand Down Expand Up @@ -200,7 +201,7 @@ Status IndexedColumnIterator::seek_at_or_after(const void* key, bool* exact_matc
}

if (_reader->num_values() == 0) {
return Status::NotFound("value index is empty ");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("value index is empty ");
}

g_index_reader_seek_count << 1;
Expand All @@ -212,11 +213,11 @@ Status IndexedColumnIterator::seek_at_or_after(const void* key, bool* exact_matc
std::string encoded_key;
_reader->_value_key_coder->full_encode_ascending(key, &encoded_key);
Status st = _value_iter.seek_at_or_before(encoded_key);
if (st.is<NOT_FOUND>()) {
if (st.is<ENTRY_NOT_FOUND>()) {
// all keys in page is greater than `encoded_key`, point to the first page.
// otherwise, we may missing some pages.
// For example, the predicate is `col1 > 2`, and the index page is [3,5,7].
// so the `seek_at_or_before(2)` will return Status::NotFound().
// so the `seek_at_or_before(2)` will return Status::Error<ENTRY_NOT_FOUND>().
// But actually, we expect it to point to page `3`.
_value_iter.seek_to_first();
} else if (!st.ok()) {
Expand All @@ -241,7 +242,7 @@ Status IndexedColumnIterator::seek_at_or_after(const void* key, bool* exact_matc
// seek inside data page
Status st = _data_page.data_decoder->seek_at_or_after_value(key, exact_match);
// return the first row of next page when not found
if (st.is<NOT_FOUND>() && _reader->_has_index_page) {
if (st.is<ENTRY_NOT_FOUND>() && _reader->_has_index_page) {
if (_value_iter.has_next()) {
_seeked = true;
*exact_match = false;
Expand Down
4 changes: 2 additions & 2 deletions be/src/olap/rowset/segment_v2/indexed_column_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class IndexedColumnIterator {
_value_iter(&reader->_value_index_reader) {}

// Seek to the given ordinal entry. Entry 0 is the first entry.
// Return NotFound if provided seek point is past the end.
// Return Status::Error<ENTRY_NOT_FOUND> if provided seek point is past the end.
// Return NotSupported for column without ordinal index.
Status seek_to_ordinal(ordinal_t idx);

Expand All @@ -114,7 +114,7 @@ class IndexedColumnIterator {
// Sets *exact_match to indicate whether the seek found the exact
// key requested.
//
// Return NotFound if the given key is greater than all keys in this column.
// Return Status::Error<ENTRY_NOT_FOUND> if the given key is greater than all keys in this column.
// Return NotSupported for column without value index.
Status seek_at_or_after(const void* key, bool* exact_match);
Status seek_at_or_after(const std::string* key, bool* exact_match) {
Expand Down
4 changes: 2 additions & 2 deletions be/src/olap/rowset/segment_v2/page_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ class PageBuilder {

// Return the first value in this page.
// This method could only be called between finish() and reset().
// Status::NotFound if no values have been added.
// Status::Error<ENTRY_NOT_FOUND> if no values have been added.
virtual Status get_first_value(void* value) const = 0;

// Return the last value in this page.
// This method could only be called between finish() and reset().
// Status::NotFound if no values have been added.
// Status::Error<ENTRY_NOT_FOUND> if no values have been added.
virtual Status get_last_value(void* value) const = 0;

private:
Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/rowset/segment_v2/page_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class PageDecoder {
//
// If the given value is less than the lowest value in the page,
// seeks to the start of the page. If it is higher than the highest
// value in the page, then returns Status::NotFound
// value in the page, then returns Status::Error<ENTRY_NOT_FOUND>
//
// This will only return valid results when the data page
// consists of values in sorted order.
Expand Down
8 changes: 4 additions & 4 deletions be/src/olap/rowset/segment_v2/plain_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ class PlainPageBuilder : public PageBuilder {

Status get_first_value(void* value) const override {
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, _first_value.data(), SIZE_OF_TYPE);
return Status::OK();
}

Status get_last_value(void* value) const override {
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, _last_value.data(), SIZE_OF_TYPE);
return Status::OK();
Expand Down Expand Up @@ -147,7 +147,7 @@ class PlainPageDecoder : public PageDecoder {
DCHECK(_parsed) << "Must call init() firstly";

if (_num_elems == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}

size_t left = 0;
Expand All @@ -168,7 +168,7 @@ class PlainPageDecoder : public PageDecoder {
}
}
if (left >= _num_elems) {
return Status::NotFound("all value small than the value");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("all value small than the value");
}
const void* find_value = &_data[PLAIN_PAGE_HEADER_SIZE + left * SIZE_OF_TYPE];
if (TypeTraits<Type>::cmp(find_value, value) == 0) {
Expand Down
4 changes: 2 additions & 2 deletions be/src/olap/rowset/segment_v2/rle_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class RlePageBuilder : public PageBuilder {
Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_first_value, SIZE_OF_TYPE);
return Status::OK();
Expand All @@ -125,7 +125,7 @@ class RlePageBuilder : public PageBuilder {
Status get_last_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::NotFound("page is empty");
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_last_value, SIZE_OF_TYPE);
return Status::OK();
Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/rowset/segment_v2/segment_iterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ Status SegmentIterator::_lookup_ordinal_from_pk_index(const RowCursor& key, bool
Status status = index_iterator->seek_at_or_after(&index_key, &exact_match);
if (UNLIKELY(!status.ok())) {
*rowid = num_rows();
if (status.is<NOT_FOUND>()) {
if (status.is<ENTRY_NOT_FOUND>()) {
return Status::OK();
}
return status;
Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/tablet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2832,7 +2832,7 @@ Status Tablet::lookup_row_key(const Slice& encoded_key, bool with_seq_col,

for (auto id : picked_segments) {
Status s = segments[id]->lookup_row_key(encoded_key, with_seq_col, &loc);
if (s.is<KEY_NOT_FOUND>()) {
if (s.is<ENTRY_NOT_FOUND>() || s.is<KEY_NOT_FOUND>()) {
continue;
}
if (!s.ok() && !s.is<KEY_ALREADY_EXISTS>()) {
Expand Down
4 changes: 2 additions & 2 deletions be/test/olap/rowset/segment_v2/binary_prefix_page_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class BinaryPrefixPageTest : public testing::Test {
Slice v1 = Slice("1039");
bool exact_match;
ret = page_decoder->seek_at_or_after_value(&v1, &exact_match);
EXPECT_TRUE(ret.is<NOT_FOUND>());
EXPECT_TRUE(ret.is<ENTRY_NOT_FOUND>());

Slice v2 = Slice("1000");
ret = page_decoder->seek_at_or_after_value(&v2, &exact_match);
Expand Down Expand Up @@ -243,7 +243,7 @@ class BinaryPrefixPageTest : public testing::Test {
Slice v1 = Slice("1039");
bool exact_match;
ret = page_decoder->seek_at_or_after_value(&v1, &exact_match);
EXPECT_TRUE(ret.is<NOT_FOUND>());
EXPECT_TRUE(ret.is<ENTRY_NOT_FOUND>());

Slice v2 = Slice("1000");
ret = page_decoder->seek_at_or_after_value(&v2, &exact_match);
Expand Down
2 changes: 1 addition & 1 deletion be/test/olap/rowset/segment_v2/bitshuffle_page_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class BitShufflePageTest : public testing::Test {
EXPECT_FALSE(exact_match);

status = page_decoder.seek_at_or_after_value(bigger_than_biggest, &exact_match);
EXPECT_EQ(status.code(), NOT_FOUND);
EXPECT_EQ(status.code(), ENTRY_NOT_FOUND);
}
};

Expand Down
2 changes: 1 addition & 1 deletion be/test/olap/rowset/segment_v2/plain_page_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class PlainPageTest : public testing::Test {

if (bigger_than_biggest != nullptr) {
status = page_decoder.seek_at_or_after_value(bigger_than_biggest, &exact_match);
EXPECT_EQ(status.code(), NOT_FOUND);
EXPECT_EQ(status.code(), ENTRY_NOT_FOUND);
}
}
};
Expand Down

0 comments on commit 78a48df

Please sign in to comment.