Skip to content

Commit

Permalink
[refactor](mempool) remove mempool parameter from key decoder methods (…
Browse files Browse the repository at this point in the history 
…apache#17137)


decode method is only used for big int and other decode method is only used in unit test.

I remove the useless method and we can remove mempool parameter from decode method.
---------

Co-authored-by: yiguolei <yiguolei@gmail.com>
  • Loading branch information
@yiguolei @Doris-Extras
yiguolei and Doris-Extras authored Feb 27, 2023
1 parent aab8dad commit 33acaa0
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 65 deletions.
4 changes: 0 additions & 4 deletions be/src/olap/field.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,6 @@ class Field {
void full_encode_ascending(const void* value, std::string* buf) const {
_key_coder->full_encode_ascending(value, buf);
}

Status decode_ascending(Slice* encoded_key, uint8_t* cell_ptr, MemPool* pool) const {
return _key_coder->decode_ascending(encoded_key, _index_size, cell_ptr, pool);
}
void add_sub_field(std::unique_ptr<Field> sub_field) {
_sub_fields.emplace_back(std::move(sub_field));
}
Expand Down
71 changes: 20 additions & 51 deletions be/src/olap/key_coder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ using strings::Substitute;

using FullEncodeAscendingFunc = void (*)(const void* value, std::string* buf);
using EncodeAscendingFunc = void (*)(const void* value, size_t index_size, std::string* buf);
using DecodeAscendingFunc = Status (*)(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool);
using DecodeAscendingFunc = Status (*)(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr);

// Order-preserving binary encoding for values of a particular type so that
// those values can be compared by memcpy their encoded bytes.
Expand All @@ -55,9 +54,9 @@ class KeyCoder {
_encode_ascending(value, index_size, buf);
}

Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) const {
return _decode_ascending(encoded_key, index_size, cell_ptr, pool);
// Only used for test, should delete it in the future
Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) const {
return _decode_ascending(encoded_key, index_size, cell_ptr);
}

private:
Expand Down Expand Up @@ -117,8 +116,9 @@ class KeyCoderTraits<field_type, typename std::enable_if<std::is_integral<typena
full_encode_ascending(value, buf);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
// decode_ascending only used in orinal index page, maybe should remove it in the future.
// currently, we reduce the usage of this method.
if (encoded_key->size < sizeof(UnsignedCppType)) {
return Status::InvalidArgument("Key too short, need={} vs real={}",
sizeof(UnsignedCppType), encoded_key->size);
Expand Down Expand Up @@ -155,8 +155,7 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_DATE> {
full_encode_ascending(value, buf);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
if (encoded_key->size < sizeof(UnsignedCppType)) {
return Status::InvalidArgument("Key too short, need={} vs real={}",
sizeof(UnsignedCppType), encoded_key->size);
Expand Down Expand Up @@ -189,8 +188,7 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_DATEV2> {
full_encode_ascending(value, buf);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
if (encoded_key->size < sizeof(UnsignedCppType)) {
return Status::InvalidArgument(Substitute("Key too short, need=$0 vs real=$1",
sizeof(UnsignedCppType), encoded_key->size));
Expand Down Expand Up @@ -223,8 +221,7 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_DATETIMEV2> {
full_encode_ascending(value, buf);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
if (encoded_key->size < sizeof(UnsignedCppType)) {
return Status::InvalidArgument(Substitute("Key too short, need=$0 vs real=$1",
sizeof(UnsignedCppType), encoded_key->size));
Expand All @@ -246,19 +243,18 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_DECIMAL> {
memcpy(&decimal_val, value, sizeof(decimal12_t));
KeyCoderTraits<OLAP_FIELD_TYPE_BIGINT>::full_encode_ascending(&decimal_val.integer, buf);
KeyCoderTraits<OLAP_FIELD_TYPE_INT>::full_encode_ascending(&decimal_val.fraction, buf);
}
} // namespace doris

static void encode_ascending(const void* value, size_t index_size, std::string* buf) {
full_encode_ascending(value, buf);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
decimal12_t decimal_val = {0, 0};
RETURN_IF_ERROR(KeyCoderTraits<OLAP_FIELD_TYPE_BIGINT>::decode_ascending(
encoded_key, sizeof(decimal_val.integer), (uint8_t*)&decimal_val.integer, pool));
encoded_key, sizeof(decimal_val.integer), (uint8_t*)&decimal_val.integer));
RETURN_IF_ERROR(KeyCoderTraits<OLAP_FIELD_TYPE_INT>::decode_ascending(
encoded_key, sizeof(decimal_val.fraction), (uint8_t*)&decimal_val.fraction, pool));
encoded_key, sizeof(decimal_val.fraction), (uint8_t*)&decimal_val.fraction));
memcpy(cell_ptr, &decimal_val, sizeof(decimal12_t));
return Status::OK();
}
Expand All @@ -280,17 +276,8 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_CHAR> {
buf->append(slice->data, index_size);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
if (encoded_key->size < index_size) {
return Status::InvalidArgument("Key too short, need={} vs real={}", index_size,
encoded_key->size);
}
Slice* slice = (Slice*)cell_ptr;
slice->data = (char*)pool->allocate(index_size);
slice->size = index_size;
memcpy(slice->data, encoded_key->data, index_size);
encoded_key->remove_prefix(index_size);
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
LOG(FATAL) << "decode_ascending is not implemented";
return Status::OK();
}
};
Expand All @@ -309,17 +296,8 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_VARCHAR> {
buf->append(slice->data, copy_size);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
CHECK(encoded_key->size <= index_size)
<< "encoded_key size is larger than index_size, key_size=" << encoded_key->size
<< ", index_size=" << index_size;
auto copy_size = encoded_key->size;
Slice* slice = (Slice*)cell_ptr;
slice->data = (char*)pool->allocate(copy_size);
slice->size = copy_size;
memcpy(slice->data, encoded_key->data, copy_size);
encoded_key->remove_prefix(copy_size);
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
LOG(FATAL) << "decode_ascending is not implemented";
return Status::OK();
}
};
Expand All @@ -338,17 +316,8 @@ class KeyCoderTraits<OLAP_FIELD_TYPE_STRING> {
buf->append(slice->data, copy_size);
}

static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr,
MemPool* pool) {
CHECK(encoded_key->size <= index_size)
<< "encoded_key size is larger than index_size, key_size=" << encoded_key->size
<< ", index_size=" << index_size;
auto copy_size = encoded_key->size;
Slice* slice = (Slice*)cell_ptr;
slice->data = (char*)pool->allocate(copy_size);
slice->size = copy_size;
memcpy(slice->data, encoded_key->data, copy_size);
encoded_key->remove_prefix(copy_size);
static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
LOG(FATAL) << "decode_ascending is not implemented";
return Status::OK();
}
};
Expand Down
2 changes: 1 addition & 1 deletion be/src/olap/rowset/segment_v2/ordinal_page_index.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Status OrdinalIndexReader::load(bool use_page_cache, bool kept_in_memory) {
Slice key = reader.get_key(i);
ordinal_t ordinal = 0;
RETURN_IF_ERROR(KeyCoderTraits<OLAP_FIELD_TYPE_UNSIGNED_BIGINT>::decode_ascending(
&key, sizeof(ordinal_t), (uint8_t*)&ordinal, nullptr));
&key, sizeof(ordinal_t), (uint8_t*)&ordinal));

_ordinals[i] = ordinal;
_pages[i] = reader.get_value(i);
Expand Down
22 changes: 13 additions & 9 deletions be/test/olap/key_coder_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void test_integer_encode() {
{
Slice slice(buf);
CppType check_val;
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val, nullptr);
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val);
EXPECT_EQ(val, check_val);
}
}
Expand All @@ -76,7 +76,7 @@ void test_integer_encode() {
{
Slice slice(buf);
CppType check_val;
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val, nullptr);
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val);
EXPECT_EQ(val, check_val);
}
}
Expand Down Expand Up @@ -132,7 +132,7 @@ TEST_F(KeyCoderTest, test_date) {
{
Slice slice(buf);
CppType check_val;
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val, nullptr);
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val);
EXPECT_EQ(val, check_val);
}
}
Expand All @@ -148,7 +148,7 @@ TEST_F(KeyCoderTest, test_date) {
{
Slice slice(buf);
CppType check_val;
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val, nullptr);
key_coder->decode_ascending(&slice, sizeof(CppType), (uint8_t*)&check_val);
EXPECT_EQ(val, check_val);
}
}
Expand Down Expand Up @@ -183,7 +183,7 @@ TEST_F(KeyCoderTest, test_decimal) {

decimal12_t check_val;
Slice slice1(buf1);
key_coder->decode_ascending(&slice1, sizeof(decimal12_t), (uint8_t*)&check_val, nullptr);
key_coder->decode_ascending(&slice1, sizeof(decimal12_t), (uint8_t*)&check_val);
EXPECT_EQ(check_val, val1);

{
Expand Down Expand Up @@ -227,26 +227,28 @@ TEST_F(KeyCoderTest, test_char) {
std::string key;
key_coder->encode_ascending(&slice, 10, &key);
Slice encoded_key(key);

/*
Slice check_slice;
auto st = key_coder->decode_ascending(&encoded_key, 10, (uint8_t*)&check_slice, &_pool);
EXPECT_TRUE(st.ok());
EXPECT_EQ(10, check_slice.size);
EXPECT_EQ(strncmp("1234567890", check_slice.data, 10), 0);
*/
}

{
std::string key;
key_coder->encode_ascending(&slice, 5, &key);
Slice encoded_key(key);

/*
Slice check_slice;
auto st = key_coder->decode_ascending(&encoded_key, 5, (uint8_t*)&check_slice, &_pool);
EXPECT_TRUE(st.ok());
EXPECT_EQ(5, check_slice.size);
EXPECT_EQ(strncmp("12345", check_slice.data, 5), 0);
*/
}
}

Expand All @@ -260,26 +262,28 @@ TEST_F(KeyCoderTest, test_varchar) {
std::string key;
key_coder->encode_ascending(&slice, 15, &key);
Slice encoded_key(key);

/*
Slice check_slice;
auto st = key_coder->decode_ascending(&encoded_key, 15, (uint8_t*)&check_slice, &_pool);
EXPECT_TRUE(st.ok());
EXPECT_EQ(10, check_slice.size);
EXPECT_EQ(strncmp("1234567890", check_slice.data, 10), 0);
*/
}

{
std::string key;
key_coder->encode_ascending(&slice, 5, &key);
Slice encoded_key(key);

/*
Slice check_slice;
auto st = key_coder->decode_ascending(&encoded_key, 5, (uint8_t*)&check_slice, &_pool);
EXPECT_TRUE(st.ok());
EXPECT_EQ(5, check_slice.size);
EXPECT_EQ(strncmp("12345", check_slice.data, 5), 0);
*/
}
}

Expand Down

0 comments on commit 33acaa0

Please sign in to comment.