github-actions[bot] commented on code in PR #33850: URL: https://github.com/apache/doris/pull/33850#discussion_r1570564566
########## be/src/vec/columns/column_struct.cpp: ########## @@ -251,6 +251,15 @@ void ColumnStruct::insert_range_from(const IColumn& src, size_t start, size_t le } } +void ColumnStruct::insert_range_from_ignore_overflow(const IColumn& src, size_t start, Review Comment: warning: method 'insert_range_from_ignore_overflow' can be made static [readability-convert-member-functions-to-static] be/src/vec/columns/column_struct.h:147: ```diff - void insert_range_from_ignore_overflow(const IColumn& src, size_t start, + static void insert_range_from_ignore_overflow(const IColumn& src, size_t start, ``` ########## be/src/vec/columns/column_array.cpp: ########## @@ -514,6 +514,40 @@ void ColumnArray::insert_range_from(const IColumn& src, size_t start, size_t len } } +void ColumnArray::insert_range_from_ignore_overflow(const IColumn& src, size_t start, + size_t length) { + const ColumnArray& src_concrete = assert_cast<const ColumnArray&>(src); + + if (start + length > src_concrete.get_offsets().size()) { + throw doris::Exception(doris::ErrorCode::INTERNAL_ERROR, + "Parameter out of bound in ColumnArray::insert_range_from method. " + "[start({}) + length({}) > offsets.size({})]", + std::to_string(start), std::to_string(length), + std::to_string(src_concrete.get_offsets().size())); + } + + size_t nested_offset = src_concrete.offset_at(start); + size_t nested_length = src_concrete.get_offsets()[start + length - 1] - nested_offset; + + get_data().insert_range_from_ignore_overflow(src_concrete.get_data(), nested_offset, + nested_length); + + auto& cur_offsets = get_offsets(); + const auto& src_offsets = src_concrete.get_offsets(); + + if (start == 0 && cur_offsets.empty()) { + cur_offsets.assign(src_offsets.begin(), src_offsets.begin() + length); + } else { + size_t old_size = cur_offsets.size(); + // -1 is ok, because PaddedPODArray pads zeros on the left. + size_t prev_max_offset = cur_offsets.back(); + cur_offsets.resize(old_size + length); + + for (size_t i = 0; i < length; ++i) + cur_offsets[old_size + i] = src_offsets[start + i] - nested_offset + prev_max_offset; Review Comment: warning: statement should be inside braces [readability-braces-around-statements] ```suggestion for (size_t i = 0; i < length; ++i) { cur_offsets[old_size + i] = src_offsets[start + i] - nested_offset + prev_max_offset; } ``` ########## be/src/vec/core/sort_block.h: ########## @@ -223,6 +223,15 @@ class ColumnSorter { } } + void sort_column(const ColumnString64& column, EqualFlags& flags, IColumn::Permutation& perms, + EqualRange& range, bool last_column) const { Review Comment: warning: method 'sort_column' can be made static [readability-convert-member-functions-to-static] ```suggestion static void sort_column(const ColumnString64& column, EqualFlags& flags, IColumn::Permutation& perms, EqualRange& range, bool last_column) { ``` ########## be/src/vec/columns/column_map.cpp: ########## @@ -407,6 +407,42 @@ void ColumnMap::insert_range_from(const IColumn& src, size_t start, size_t lengt } } +void ColumnMap::insert_range_from_ignore_overflow(const IColumn& src, size_t start, size_t length) { Review Comment: warning: method 'insert_range_from_ignore_overflow' can be made static [readability-convert-member-functions-to-static] be/src/vec/columns/column_map.h:104: ```diff - void insert_range_from_ignore_overflow(const IColumn& src, size_t start, + static void insert_range_from_ignore_overflow(const IColumn& src, size_t start, ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org