Skip to content

branch-4.1: [cleanup](segment)Remove dead get_first_value/get_last_value APIs from PageBuilder #62493 #64433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
github-actions wants to merge 1 commit into
base: branch-4.1
Choose a base branch
from
Open

branch-4.1: [cleanup](segment)Remove dead get_first_value/get_last_value APIs from PageBuilder #62493 #64433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions be/src/storage/segment/binary_dict_page.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ Status BinaryDictPageBuilder::add(const uint8_t* vals, size_t* count) {
auto* actual_builder = dynamic_cast<BitshufflePageBuilder<FieldType::OLAP_FIELD_TYPE_INT>*>(
_data_page_builder.get());

if (_data_page_builder->count() == 0) {
_first_value.assign_copy(reinterpret_cast<const uint8_t*>(src->get_data()),
src->get_size());
}

for (int i = 0; i < *count; ++i, ++src) {
if (is_page_full()) {
break;
Expand Down Expand Up @@ -219,32 +214,6 @@ Status BinaryDictPageBuilder::get_dictionary_page_encoding(EncodingTypePB* encod
return Status::OK();
}

Status BinaryDictPageBuilder::get_first_value(void* value) const {
DCHECK(_finished);
if (_data_page_builder->count() == 0) {
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
if (_encoding_type != DICT_ENCODING) {
return _data_page_builder->get_first_value(value);
}
*reinterpret_cast<Slice*>(value) = Slice(_first_value);
return Status::OK();
}

Status BinaryDictPageBuilder::get_last_value(void* value) const {
DCHECK(_finished);
if (_data_page_builder->count() == 0) {
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
if (_encoding_type != DICT_ENCODING) {
return _data_page_builder->get_last_value(value);
}
uint32_t value_code;
RETURN_IF_ERROR(_data_page_builder->get_last_value(&value_code));
RETURN_IF_ERROR(_dict_builder->get_dict_word(value_code, reinterpret_cast<Slice*>(value)));
return Status::OK();
}

uint64_t BinaryDictPageBuilder::get_raw_data_size() const {
return _raw_data_size;
}
Expand Down
5 changes: 0 additions & 5 deletions be/src/storage/segment/binary_dict_page.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,6 @@ class BinaryDictPageBuilder : public PageBuilderHelper<BinaryDictPageBuilder> {

Status get_dictionary_page_encoding(EncodingTypePB* encoding) const override;

Status get_first_value(void* value) const override;

Status get_last_value(void* value) const override;

uint64_t get_raw_data_size() const override;

private:
Expand Down Expand Up @@ -123,7 +119,6 @@ class BinaryDictPageBuilder : public PageBuilderHelper<BinaryDictPageBuilder> {
// TODO(zc): rethink about this arena
Arena _arena;
faststring _buffer;
faststring _first_value;
uint64_t _raw_data_size = 0;

bool _has_empty = false;
Expand Down
48 changes: 0 additions & 48 deletions be/src/storage/segment/binary_plain_page.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ class BinaryPlainPageBuilder : public PageBuilderHelper<BinaryPlainPageBuilder<T
put_fixed32_le(&_buffer, _offset);
}
put_fixed32_le(&_buffer, cast_set<uint32_t>(_offsets.size()));
if (_offsets.size() > 0) {
_copy_value_at(0, &_first_value);
_copy_value_at(_offsets.size() - 1, &_last_value);
}
*slice = _buffer.build();
});
return Status::OK();
Expand All @@ -133,46 +129,10 @@ class BinaryPlainPageBuilder : public PageBuilderHelper<BinaryPlainPageBuilder<T

uint64_t get_raw_data_size() const override { return _raw_data_size; }

Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_offsets.size() == 0) {
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::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_last_value);
return Status::OK();
}

inline Slice operator[](size_t idx) const {
DCHECK(!_finished);
DCHECK_LT(idx, _offsets.size());
size_t value_size =
(idx < _offsets.size() - 1) ? _offsets[idx + 1] - _offsets[idx] : _last_value_size;
return Slice(&_buffer[_offsets[idx]], value_size);
}

Status get_dict_word(uint32_t value_code, Slice* word) override {
*word = (*this)[value_code];
return Status::OK();
}

private:
BinaryPlainPageBuilder(const PageBuilderOptions& options)
: _size_estimate(0), _options(options) {}

void _copy_value_at(size_t idx, faststring* value) const {
size_t value_size =
(idx < _offsets.size() - 1) ? _offsets[idx + 1] - _offsets[idx] : _last_value_size;
value->assign_copy(&_buffer[_offsets[idx]], value_size);
}

faststring _buffer;
size_t _size_estimate;
// Offsets of each entry, relative to the start of the page
Expand All @@ -182,8 +142,6 @@ class BinaryPlainPageBuilder : public PageBuilderHelper<BinaryPlainPageBuilder<T
// size of last added value
uint32_t _last_value_size = 0;
uint64_t _raw_data_size = 0;
faststring _first_value;
faststring _last_value;
};

template <FieldType Type>
Expand Down Expand Up @@ -311,12 +269,6 @@ class BinaryPlainPageDecoder : public PageDecoder {
return _cur_idx;
}

Slice string_at_index(size_t idx) const {
const uint32_t start_offset = offset(idx);
uint32_t len = offset(idx + 1) - start_offset;
return Slice(&_data[start_offset], len);
}

Status get_dict_word_info(StringRef* dict_word_info) override {
if (UNLIKELY(_num_elems <= 0)) {
return Status::OK();
Expand Down
49 changes: 0 additions & 49 deletions be/src/storage/segment/binary_plain_page_v2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,6 @@ class BinaryPlainPageV2Builder : public PageBuilderHelper<BinaryPlainPageV2Build
DCHECK(!_finished);
_finished = true;
RETURN_IF_CATCH_EXCEPTION({
// Store first and last values for statistics
if (_positions.size() > 0) {
_copy_value_at(0, &_first_value);
_copy_value_at(_positions.size() - 1, &_last_value);
}

// Write trailer: number of elements
put_fixed32_le(&_buffer, cast_set<uint32_t>(_positions.size()));

Expand Down Expand Up @@ -137,51 +131,10 @@ class BinaryPlainPageV2Builder : public PageBuilderHelper<BinaryPlainPageV2Build

uint64_t get_raw_data_size() const override { return _raw_data_size; }

Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_positions.size() == 0) {
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 (_positions.size() == 0) {
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_last_value);
return Status::OK();
}

Status get_dict_word(uint32_t value_code, Slice* word) override {
if (value_code >= _positions.size()) {
return Status::Error<ErrorCode::INVALID_ARGUMENT>(
"value_code {} is out of range [0, {})", value_code, _positions.size());
}

const uint8_t* ptr = reinterpret_cast<const uint8_t*>(&_buffer[_positions[value_code]]);
uint32_t length;
const uint8_t* data_ptr = decode_varint32_ptr(ptr, ptr + 5, &length);

word->data = const_cast<char*>(reinterpret_cast<const char*>(data_ptr));
word->size = length;

return Status::OK();
}

private:
BinaryPlainPageV2Builder(const PageBuilderOptions& options)
: _size_estimate(0), _options(options) {}

void _copy_value_at(size_t idx, faststring* value) const {
const uint8_t* ptr = &_buffer[_positions[idx]];
uint32_t length;
const uint8_t* data_ptr = decode_varint32_ptr(ptr, ptr + 5, &length);
value->assign_copy(data_ptr, length);
}

faststring _buffer;
size_t _size_estimate = 0;
// Positions of each entry in the buffer (pointing to the varuint length)
Expand All @@ -190,8 +143,6 @@ class BinaryPlainPageV2Builder : public PageBuilderHelper<BinaryPlainPageV2Build
PageBuilderOptions _options;
uint32_t _last_value_size = 0;
uint64_t _raw_data_size = 0;
faststring _first_value;
faststring _last_value;
};

// BinaryPlainPageV2Decoder now inherits from BinaryPlainPageDecoder.
Expand Down
4 changes: 0 additions & 4 deletions be/src/storage/segment/binary_prefix_page.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ Status BinaryPrefixPageBuilder::add(const uint8_t* vals, size_t* add_count) {
}

const Slice* src = reinterpret_cast<const Slice*>(vals);
if (_count == 0) {
_first_entry.assign_copy(reinterpret_cast<const uint8_t*>(src->get_data()),
src->get_size());
}

int i = 0;
for (; i < *add_count; ++i, ++src) {
Expand Down
21 changes: 2 additions & 19 deletions be/src/storage/segment/binary_prefix_page.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,30 +76,13 @@ class BinaryPrefixPageBuilder : public PageBuilderHelper<BinaryPrefixPageBuilder

uint64_t get_raw_data_size() const override { return _raw_data_size; }

Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_first_entry);
return Status::OK();
}

Status get_last_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
return Status::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
*reinterpret_cast<Slice*>(value) = Slice(_last_entry);
return Status::OK();
}

private:
BinaryPrefixPageBuilder(const PageBuilderOptions& options) : _options(options) {}

PageBuilderOptions _options;
std::vector<uint32_t> _restart_points_offset;
faststring _first_entry;
// Holds the most recently added entry; used by add() to compute the
// shared-prefix length against the next entry.
faststring _last_entry;
size_t _count = 0;
bool _finished = false;
Expand Down
23 changes: 0 additions & 23 deletions be/src/storage/segment/bitshuffle_page.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,6 @@ class BitshufflePageBuilder : public PageBuilderHelper<BitshufflePageBuilder<Typ
}

Status finish(OwnedSlice* slice) override {
if (_count > 0) {
_first_value = cell(0);
_last_value = cell(_count - 1);
}
RETURN_IF_CATCH_EXCEPTION({ *slice = _finish(SIZE_OF_TYPE); });
return Status::OK();
}
Expand Down Expand Up @@ -191,23 +187,6 @@ class BitshufflePageBuilder : public PageBuilderHelper<BitshufflePageBuilder<Typ

uint64_t get_raw_data_size() const override { return _raw_data_size; }

Status get_first_value(void* value) const override {
DCHECK(_finished);
if (_count == 0) {
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::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_last_value, SIZE_OF_TYPE);
return Status::OK();
}

private:
BitshufflePageBuilder(const PageBuilderOptions& options)
: _options(options), _count(0), _remain_element_capacity(0), _finished(false) {}
Expand Down Expand Up @@ -266,8 +245,6 @@ class BitshufflePageBuilder : public PageBuilderHelper<BitshufflePageBuilder<Typ
bool _finished;
faststring _data;
faststring _buffer;
CppType _first_value;
CppType _last_value;
uint64_t _raw_data_size = 0;
};

Expand Down
22 changes: 0 additions & 22 deletions be/src/storage/segment/frame_of_reference_page.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,9 @@ class FrameOfReferencePageBuilder : public PageBuilderHelper<FrameOfReferencePag
return Status::OK();
}
auto new_vals = reinterpret_cast<const CppType*>(vals);
if (_count == 0) {
_first_val = *new_vals;
}
_encoder->put_batch(new_vals, *count);
_count += *count;
_raw_data_size += *count * sizeof(CppType);
_last_val = new_vals[*count - 1];
return Status::OK();
}

Expand All @@ -78,22 +74,6 @@ class FrameOfReferencePageBuilder : public PageBuilderHelper<FrameOfReferencePag

uint64_t get_raw_data_size() const override { return _raw_data_size; }

Status get_first_value(void* value) const override {
if (_count == 0) {
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::Error<ErrorCode::ENTRY_NOT_FOUND>("page is empty");
}
memcpy(value, &_last_val, sizeof(CppType));
return Status::OK();
}

private:
explicit FrameOfReferencePageBuilder(const PageBuilderOptions& options)
: _options(options), _count(0), _finished(false) {}
Expand All @@ -104,8 +84,6 @@ class FrameOfReferencePageBuilder : public PageBuilderHelper<FrameOfReferencePag
bool _finished;
std::unique_ptr<ForEncoder<CppType>> _encoder;
faststring _buf;
CppType _first_val;
CppType _last_val;
uint64_t _raw_data_size = 0;
};

Expand Down
14 changes: 0 additions & 14 deletions be/src/storage/segment/page_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ class PageBuilder {
return Status::NotSupported("get_dictionary_page_encoding not implemented");
}

virtual Status get_dict_word(uint32_t value_code, Slice* word) {
return Status::NotSupported("get_dict_word not implemented");
}

// Reset the internal state of the page builder.
//
// Any data previously returned by finish may be invalidated by this call.
Expand All @@ -95,16 +91,6 @@ class PageBuilder {
// Return the uncompressed data size in bytes (raw data added via add() method).
// This is used to track the original data size before compression.
virtual uint64_t get_raw_data_size() const = 0;

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

template <typename Derived>
Expand Down
Loading