github-actions[bot] commented on code in PR #33584: URL: https://github.com/apache/doris/pull/33584#discussion_r1562139012
########## be/src/vec/columns/column_string.h: ########## @@ -106,28 +106,28 @@ public: void sanity_check() const; - bool is_variable_length() const override { return true; } - const char* get_family_name() const override { return "String"; } + bool is_variable_length() const override final { return true; } + const char* get_family_name() const override final { return "String"; } - size_t size() const override { return offsets.size(); } + size_t size() const override final { return offsets.size(); } - size_t byte_size() const override { return chars.size() + offsets.size() * sizeof(offsets[0]); } + size_t byte_size() const override final { return chars.size() + offsets.size() * sizeof(offsets[0]); } - size_t allocated_bytes() const override { + size_t allocated_bytes() const override final { return chars.allocated_bytes() + offsets.allocated_bytes(); } - MutableColumnPtr clone_resized(size_t to_size) const override; + MutableColumnPtr clone_resized(size_t to_size) const override final; - MutableColumnPtr get_shrinked_column() override; - bool could_shrinked_column() override { return true; } + MutableColumnPtr get_shrinked_column() override final ; + bool could_shrinked_column() override final { return true; } - Field operator[](size_t n) const override { + Field operator[](size_t n) const override final { assert(n < size()); return Field(&chars[offset_at(n)], size_at(n)); } - void get(size_t n, Field& res) const override { + void get(size_t n, Field& res) const override final { Review Comment: warning: method 'get' can be made static [readability-convert-member-functions-to-static] ```suggestion static void get(size_t n, Field& res) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -106,28 +106,28 @@ class ColumnString final : public COWHelper<IColumn, ColumnString> { public: void sanity_check() const; - bool is_variable_length() const override { return true; } - const char* get_family_name() const override { return "String"; } + bool is_variable_length() const override final { return true; } Review Comment: warning: method 'is_variable_length' can be made static [readability-convert-member-functions-to-static] ```suggestion static bool is_variable_length() override final { return true; } ``` ########## be/src/vec/columns/column_string.h: ########## @@ -269,7 +269,7 @@ } void insert_many_binary_data(char* data_array, uint32_t* len_array, - uint32_t* start_offset_array, size_t num) override { + uint32_t* start_offset_array, size_t num) override final { Review Comment: warning: pointer parameter 'start_offset_array' can be pointer to const [readability-non-const-parameter] ```suggestion const uint32_t* start_offset_array, size_t num) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -185,7 +185,7 @@ } } - void insert_data(const char* pos, size_t length) override { + void insert_data(const char* pos, size_t length) override final { Review Comment: warning: method 'insert_data' can be made static [readability-convert-member-functions-to-static] ```suggestion static void insert_data(const char* pos, size_t length) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -478,58 +478,58 @@ } } - void insert_range_from(const IColumn& src, size_t start, size_t length) override; + void insert_range_from(const IColumn& src, size_t start, size_t length) override final; void insert_indices_from(const IColumn& src, const uint32_t* indices_begin, - const uint32_t* indices_end) override; + const uint32_t* indices_end) override final; - ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override; - size_t filter(const Filter& filter) override; + ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override final; + size_t filter(const Filter& filter) override final; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion size_t filter(const Filter& filter) final; ``` ########## be/src/vec/columns/column_string.h: ########## @@ -478,58 +478,58 @@ } } - void insert_range_from(const IColumn& src, size_t start, size_t length) override; + void insert_range_from(const IColumn& src, size_t start, size_t length) override final; void insert_indices_from(const IColumn& src, const uint32_t* indices_begin, - const uint32_t* indices_end) override; + const uint32_t* indices_end) override final; - ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override; - size_t filter(const Filter& filter) override; + ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override final; + size_t filter(const Filter& filter) override final; - Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override; + Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override final; - ColumnPtr permute(const Permutation& perm, size_t limit) const override; + ColumnPtr permute(const Permutation& perm, size_t limit) const override final; void sort_column(const ColumnSorter* sorter, EqualFlags& flags, IColumn::Permutation& perms, - EqualRange& range, bool last_column) const override; + EqualRange& range, bool last_column) const override final; // ColumnPtr index(const IColumn & indexes, size_t limit) const override; template <typename Type> ColumnPtr index_impl(const PaddedPODArray<Type>& indexes, size_t limit) const; - void insert_default() override { offsets.push_back(chars.size()); } + void insert_default() override final { offsets.push_back(chars.size()); } - void insert_many_defaults(size_t length) override { + void insert_many_defaults(size_t length) override final { offsets.resize_fill(offsets.size() + length, chars.size()); } int compare_at(size_t n, size_t m, const IColumn& rhs_, - int /*nan_direction_hint*/) const override { + int /*nan_direction_hint*/) const override final { const ColumnString& rhs = assert_cast<const ColumnString&>(rhs_); return memcmp_small_allow_overflow15(chars.data() + offset_at(n), size_at(n), rhs.chars.data() + rhs.offset_at(m), rhs.size_at(m)); } void get_permutation(bool reverse, size_t limit, int nan_direction_hint, - Permutation& res) const override; + Permutation& res) const override final; - ColumnPtr replicate(const Offsets& replicate_offsets) const override; + ColumnPtr replicate(const Offsets& replicate_offsets) const override final; void append_data_by_selector(MutableColumnPtr& res, - const IColumn::Selector& selector) const override { + const IColumn::Selector& selector) const override final { append_data_by_selector_impl<ColumnString>(res, selector); } // void gather(ColumnGathererStream & gatherer_stream) override; - void reserve(size_t n) override; + void reserve(size_t n) override final; - void resize(size_t n) override; + void resize(size_t n) override final; - bool is_column_string() const override { return true; } + bool is_column_string() const override final { return true; } - bool structure_equals(const IColumn& rhs) const override { + bool structure_equals(const IColumn& rhs) const override final { Review Comment: warning: method 'structure_equals' can be made static [readability-convert-member-functions-to-static] ```suggestion static bool structure_equals(const IColumn& rhs) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -106,28 +106,28 @@ public: void sanity_check() const; - bool is_variable_length() const override { return true; } - const char* get_family_name() const override { return "String"; } + bool is_variable_length() const override final { return true; } + const char* get_family_name() const override final { return "String"; } - size_t size() const override { return offsets.size(); } + size_t size() const override final { return offsets.size(); } - size_t byte_size() const override { return chars.size() + offsets.size() * sizeof(offsets[0]); } + size_t byte_size() const override final { return chars.size() + offsets.size() * sizeof(offsets[0]); } - size_t allocated_bytes() const override { + size_t allocated_bytes() const override final { return chars.allocated_bytes() + offsets.allocated_bytes(); } - MutableColumnPtr clone_resized(size_t to_size) const override; + MutableColumnPtr clone_resized(size_t to_size) const override final; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion MutableColumnPtr clone_resized(size_t to_size) const final; ``` ########## be/src/vec/columns/column_string.h: ########## @@ -106,28 +106,28 @@ public: void sanity_check() const; - bool is_variable_length() const override { return true; } - const char* get_family_name() const override { return "String"; } + bool is_variable_length() const override final { return true; } + const char* get_family_name() const override final { return "String"; } Review Comment: warning: method 'get_family_name' can be made static [readability-convert-member-functions-to-static] ```suggestion static const char* get_family_name() override final { return "String"; } ``` ########## be/src/vec/columns/column_string.h: ########## @@ -106,28 +106,28 @@ public: void sanity_check() const; - bool is_variable_length() const override { return true; } - const char* get_family_name() const override { return "String"; } + bool is_variable_length() const override final { return true; } + const char* get_family_name() const override final { return "String"; } - size_t size() const override { return offsets.size(); } + size_t size() const override final { return offsets.size(); } - size_t byte_size() const override { return chars.size() + offsets.size() * sizeof(offsets[0]); } + size_t byte_size() const override final { return chars.size() + offsets.size() * sizeof(offsets[0]); } - size_t allocated_bytes() const override { + size_t allocated_bytes() const override final { return chars.allocated_bytes() + offsets.allocated_bytes(); } - MutableColumnPtr clone_resized(size_t to_size) const override; + MutableColumnPtr clone_resized(size_t to_size) const override final; - MutableColumnPtr get_shrinked_column() override; - bool could_shrinked_column() override { return true; } + MutableColumnPtr get_shrinked_column() override final ; + bool could_shrinked_column() override final { return true; } Review Comment: warning: method 'could_shrinked_column' can be made static [readability-convert-member-functions-to-static] ```suggestion static bool could_shrinked_column() override final { return true; } ``` ########## be/src/vec/columns/column_string.h: ########## @@ -106,28 +106,28 @@ public: void sanity_check() const; - bool is_variable_length() const override { return true; } - const char* get_family_name() const override { return "String"; } + bool is_variable_length() const override final { return true; } + const char* get_family_name() const override final { return "String"; } - size_t size() const override { return offsets.size(); } + size_t size() const override final { return offsets.size(); } - size_t byte_size() const override { return chars.size() + offsets.size() * sizeof(offsets[0]); } + size_t byte_size() const override final { return chars.size() + offsets.size() * sizeof(offsets[0]); } - size_t allocated_bytes() const override { + size_t allocated_bytes() const override final { return chars.allocated_bytes() + offsets.allocated_bytes(); } - MutableColumnPtr clone_resized(size_t to_size) const override; + MutableColumnPtr clone_resized(size_t to_size) const override final; - MutableColumnPtr get_shrinked_column() override; - bool could_shrinked_column() override { return true; } + MutableColumnPtr get_shrinked_column() override final ; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion MutableColumnPtr get_shrinked_column() final ; ``` ########## be/src/vec/columns/column_string.cpp: ########## @@ -284,6 +286,20 @@ StringRef ColumnString::serialize_value_into_arena(size_t n, Arena& arena, return res; } +StringRef ColumnLargeStringForJoin::serialize_value_into_arena(size_t n, Arena& arena, Review Comment: warning: method 'serialize_value_into_arena' can be made static [readability-convert-member-functions-to-static] be/src/vec/columns/column_string.h:622: ```diff - StringRef serialize_value_into_arena(size_t n, Arena& arena, char const*& begin) const override; + static StringRef serialize_value_into_arena(size_t n, Arena& arena, char const*& begin) override; ``` be/src/vec/columns/column_string.cpp:289: ```diff - char const*& begin) const { + char const*& begin) { ``` ########## be/src/vec/columns/column_string.cpp: ########## @@ -308,6 +324,16 @@ return max_size + sizeof(uint32_t); } +size_t ColumnLargeStringForJoin::get_max_row_byte_size() const { Review Comment: warning: method 'get_max_row_byte_size' can be made static [readability-convert-member-functions-to-static] be/src/vec/columns/column_string.h:626: ```diff - size_t get_max_row_byte_size() const override; + static size_t get_max_row_byte_size() override; ``` ```suggestion size_t ColumnLargeStringForJoin::get_max_row_byte_size() { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -163,7 +163,7 @@ offsets.push_back(new_size); } - void insert_from(const IColumn& src_, size_t n) override { + void insert_from(const IColumn& src_, size_t n) override final { Review Comment: warning: method 'insert_from' can be made static [readability-convert-member-functions-to-static] ```suggestion static void insert_from(const IColumn& src_, size_t n) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -137,12 +137,12 @@ res.assign_string(&chars[offset_at(n)], size_at(n)); } - StringRef get_data_at(size_t n) const override { + StringRef get_data_at(size_t n) const override final { Review Comment: warning: method 'get_data_at' can be made static [readability-convert-member-functions-to-static] ```suggestion static StringRef get_data_at(size_t n) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -137,12 +137,12 @@ res.assign_string(&chars[offset_at(n)], size_at(n)); } - StringRef get_data_at(size_t n) const override { + StringRef get_data_at(size_t n) const override final { assert(n < size()); return StringRef(&chars[offset_at(n)], size_at(n)); } - void insert(const Field& x) override { + void insert(const Field& x) override final { Review Comment: warning: method 'insert' can be made static [readability-convert-member-functions-to-static] ```suggestion static void insert(const Field& x) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -382,15 +382,15 @@ } } - void pop_back(size_t n) override { + void pop_back(size_t n) override final { Review Comment: warning: method 'pop_back' can be made static [readability-convert-member-functions-to-static] ```suggestion static void pop_back(size_t n) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -478,58 +478,58 @@ } } - void insert_range_from(const IColumn& src, size_t start, size_t length) override; + void insert_range_from(const IColumn& src, size_t start, size_t length) override final; void insert_indices_from(const IColumn& src, const uint32_t* indices_begin, - const uint32_t* indices_end) override; + const uint32_t* indices_end) override final; - ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override; - size_t filter(const Filter& filter) override; + ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override final; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const final; ``` ########## be/src/vec/columns/column_string.h: ########## @@ -291,7 +291,7 @@ } } - void insert_many_strings(const StringRef* strings, size_t num) override { + void insert_many_strings(const StringRef* strings, size_t num) override final { Review Comment: warning: method 'insert_many_strings' can be made static [readability-convert-member-functions-to-static] ```suggestion static void insert_many_strings(const StringRef* strings, size_t num) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -478,58 +478,58 @@ } } - void insert_range_from(const IColumn& src, size_t start, size_t length) override; + void insert_range_from(const IColumn& src, size_t start, size_t length) override final; void insert_indices_from(const IColumn& src, const uint32_t* indices_begin, - const uint32_t* indices_end) override; + const uint32_t* indices_end) override final; - ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override; - size_t filter(const Filter& filter) override; + ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override final; + size_t filter(const Filter& filter) override final; - Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override; + Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override final; - ColumnPtr permute(const Permutation& perm, size_t limit) const override; + ColumnPtr permute(const Permutation& perm, size_t limit) const override final; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion ColumnPtr permute(const Permutation& perm, size_t limit) const final; ``` ########## be/src/vec/columns/column_string.h: ########## @@ -478,58 +478,58 @@ } } - void insert_range_from(const IColumn& src, size_t start, size_t length) override; + void insert_range_from(const IColumn& src, size_t start, size_t length) override final; void insert_indices_from(const IColumn& src, const uint32_t* indices_begin, - const uint32_t* indices_end) override; + const uint32_t* indices_end) override final; - ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override; - size_t filter(const Filter& filter) override; + ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override final; + size_t filter(const Filter& filter) override final; - Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override; + Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override final; - ColumnPtr permute(const Permutation& perm, size_t limit) const override; + ColumnPtr permute(const Permutation& perm, size_t limit) const override final; void sort_column(const ColumnSorter* sorter, EqualFlags& flags, IColumn::Permutation& perms, - EqualRange& range, bool last_column) const override; + EqualRange& range, bool last_column) const override final; // ColumnPtr index(const IColumn & indexes, size_t limit) const override; template <typename Type> ColumnPtr index_impl(const PaddedPODArray<Type>& indexes, size_t limit) const; - void insert_default() override { offsets.push_back(chars.size()); } + void insert_default() override final { offsets.push_back(chars.size()); } - void insert_many_defaults(size_t length) override { + void insert_many_defaults(size_t length) override final { offsets.resize_fill(offsets.size() + length, chars.size()); } int compare_at(size_t n, size_t m, const IColumn& rhs_, - int /*nan_direction_hint*/) const override { + int /*nan_direction_hint*/) const override final { const ColumnString& rhs = assert_cast<const ColumnString&>(rhs_); return memcmp_small_allow_overflow15(chars.data() + offset_at(n), size_at(n), rhs.chars.data() + rhs.offset_at(m), rhs.size_at(m)); } void get_permutation(bool reverse, size_t limit, int nan_direction_hint, - Permutation& res) const override; + Permutation& res) const override final; - ColumnPtr replicate(const Offsets& replicate_offsets) const override; + ColumnPtr replicate(const Offsets& replicate_offsets) const override final; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion ColumnPtr replicate(const Offsets& replicate_offsets) const final; ``` ########## be/src/vec/columns/column_string.h: ########## @@ -592,15 +593,41 @@ } void get_indices_of_non_default_rows(Offsets64& indices, size_t from, - size_t limit) const override { + size_t limit) const override final { return get_indices_of_non_default_rows_impl<ColumnString>(indices, from, limit); } - ColumnPtr index(const IColumn& indexes, size_t limit) const override; + ColumnPtr index(const IColumn& indexes, size_t limit) const override final; - double get_ratio_of_default_rows(double sample_ratio) const override { + double get_ratio_of_default_rows(double sample_ratio) const override final { Review Comment: warning: method 'get_ratio_of_default_rows' can be made static [readability-convert-member-functions-to-static] ```suggestion static double get_ratio_of_default_rows(double sample_ratio) override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -539,12 +539,12 @@ Offsets& get_offsets() { return offsets; } const Offsets& get_offsets() const { return offsets; } - void clear() override { + void clear() override final { chars.clear(); offsets.clear(); } - void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override { + void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) override final { Review Comment: warning: method 'replace_column_data' can be made const [readability-make-member-function-const] ```suggestion void replace_column_data(const IColumn& rhs, size_t row, size_t self_row = 0) const override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -567,7 +567,7 @@ } // should replace according to 0,1,2... ,size,0,1,2... - void replace_column_data_default(size_t self_row = 0) override { + void replace_column_data_default(size_t self_row = 0) override final { Review Comment: warning: method 'replace_column_data_default' can be made const [readability-make-member-function-const] ```suggestion void replace_column_data_default(size_t self_row = 0) const override final { ``` ########## be/src/vec/columns/column_string.h: ########## @@ -478,58 +478,58 @@ } } - void insert_range_from(const IColumn& src, size_t start, size_t length) override; + void insert_range_from(const IColumn& src, size_t start, size_t length) override final; void insert_indices_from(const IColumn& src, const uint32_t* indices_begin, - const uint32_t* indices_end) override; + const uint32_t* indices_end) override final; - ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override; - size_t filter(const Filter& filter) override; + ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override final; + size_t filter(const Filter& filter) override final; - Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override; + Status filter_by_selector(const uint16_t* sel, size_t sel_size, IColumn* col_ptr) override final; - ColumnPtr permute(const Permutation& perm, size_t limit) const override; + ColumnPtr permute(const Permutation& perm, size_t limit) const override final; void sort_column(const ColumnSorter* sorter, EqualFlags& flags, IColumn::Permutation& perms, - EqualRange& range, bool last_column) const override; + EqualRange& range, bool last_column) const override final; // ColumnPtr index(const IColumn & indexes, size_t limit) const override; template <typename Type> ColumnPtr index_impl(const PaddedPODArray<Type>& indexes, size_t limit) const; - void insert_default() override { offsets.push_back(chars.size()); } + void insert_default() override final { offsets.push_back(chars.size()); } - void insert_many_defaults(size_t length) override { + void insert_many_defaults(size_t length) override final { offsets.resize_fill(offsets.size() + length, chars.size()); } int compare_at(size_t n, size_t m, const IColumn& rhs_, - int /*nan_direction_hint*/) const override { + int /*nan_direction_hint*/) const override final { const ColumnString& rhs = assert_cast<const ColumnString&>(rhs_); return memcmp_small_allow_overflow15(chars.data() + offset_at(n), size_at(n), rhs.chars.data() + rhs.offset_at(m), rhs.size_at(m)); } void get_permutation(bool reverse, size_t limit, int nan_direction_hint, - Permutation& res) const override; + Permutation& res) const override final; - ColumnPtr replicate(const Offsets& replicate_offsets) const override; + ColumnPtr replicate(const Offsets& replicate_offsets) const override final; void append_data_by_selector(MutableColumnPtr& res, - const IColumn::Selector& selector) const override { + const IColumn::Selector& selector) const override final { append_data_by_selector_impl<ColumnString>(res, selector); } // void gather(ColumnGathererStream & gatherer_stream) override; - void reserve(size_t n) override; + void reserve(size_t n) override final; - void resize(size_t n) override; + void resize(size_t n) override final; - bool is_column_string() const override { return true; } + bool is_column_string() const override final { return true; } Review Comment: warning: method 'is_column_string' can be made static [readability-convert-member-functions-to-static] ```suggestion static bool is_column_string() override final { return true; } ``` ########## be/src/vec/columns/column_string.h: ########## @@ -592,15 +593,41 @@ } void get_indices_of_non_default_rows(Offsets64& indices, size_t from, - size_t limit) const override { + size_t limit) const override final { return get_indices_of_non_default_rows_impl<ColumnString>(indices, from, limit); } - ColumnPtr index(const IColumn& indexes, size_t limit) const override; + ColumnPtr index(const IColumn& indexes, size_t limit) const override final; Review Comment: warning: 'override' is redundant since the function is already declared 'final' [modernize-use-override] ```suggestion ColumnPtr index(const IColumn& indexes, size_t limit) const final; ``` -- 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