github-actions[bot] commented on code in PR #33584:
URL: https://github.com/apache/doris/pull/33584#discussion_r1566022359
##########
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 final { return true; }
+ const char* get_family_name() const 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() final { return "String"; }
```
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -126,36 +125,72 @@ void ColumnString::insert_range_from(const IColumn& src,
size_t start, size_t le
}
}
-void ColumnString::insert_indices_from(const IColumn& src, const uint32_t*
indices_begin,
- const uint32_t* indices_end) {
- const auto& src_str = assert_cast<const ColumnString&>(src);
- const auto* src_offset_data = src_str.offsets.data();
+void ColumnString::insert_range_from_ignore_overflow(const
doris::vectorized::IColumn& src,
Review Comment:
warning: method 'insert_range_from_ignore_overflow' can be made static
[readability-convert-member-functions-to-static]
be/src/vec/columns/column_string.h:481:
```diff
- void insert_range_from_ignore_overflow(const IColumn& src, size_t
start, size_t length) final;
+ static void insert_range_from_ignore_overflow(const IColumn& src,
size_t start, size_t length) 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 final { return true; }
+ const char* get_family_name() const final { return "String"; }
- size_t size() const override { return offsets.size(); }
+ size_t size() const final { return offsets.size(); }
- size_t byte_size() const override { return chars.size() + offsets.size() *
sizeof(offsets[0]); }
+ size_t byte_size() const final { return chars.size() + offsets.size() *
sizeof(offsets[0]); }
- size_t allocated_bytes() const override {
+ size_t allocated_bytes() const 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 final;
- MutableColumnPtr get_shrinked_column() override;
- bool could_shrinked_column() override { return true; }
+ MutableColumnPtr get_shrinked_column() final;
+ bool could_shrinked_column() 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() final { return true; }
```
##########
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 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) 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 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() final { return true; }
```
##########
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 final {
assert(n < size());
return StringRef(&chars[offset_at(n)], size_at(n));
}
- void insert(const Field& x) override {
+ void insert(const Field& x) final {
Review Comment:
warning: method 'insert' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void insert(const Field& x) 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 final { return true; }
+ const char* get_family_name() const final { return "String"; }
- size_t size() const override { return offsets.size(); }
+ size_t size() const final { return offsets.size(); }
- size_t byte_size() const override { return chars.size() + offsets.size() *
sizeof(offsets[0]); }
+ size_t byte_size() const final { return chars.size() + offsets.size() *
sizeof(offsets[0]); }
- size_t allocated_bytes() const override {
+ size_t allocated_bytes() const 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 final;
- MutableColumnPtr get_shrinked_column() override;
- bool could_shrinked_column() override { return true; }
+ MutableColumnPtr get_shrinked_column() final;
+ bool could_shrinked_column() final { return true; }
- Field operator[](size_t n) const override {
+ Field operator[](size_t n) const 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 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) final {
```
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -126,36 +125,72 @@
}
}
-void ColumnString::insert_indices_from(const IColumn& src, const uint32_t*
indices_begin,
- const uint32_t* indices_end) {
- const auto& src_str = assert_cast<const ColumnString&>(src);
- const auto* src_offset_data = src_str.offsets.data();
+void ColumnString::insert_range_from_ignore_overflow(const
doris::vectorized::IColumn& src,
+ size_t start, size_t
length) {
+ const ColumnString& src_concrete = assert_cast<const ColumnString&>(src);
+ if (start + length > src_concrete.offsets.size()) {
+ throw doris::Exception(
+ doris::ErrorCode::INTERNAL_ERROR,
+ "Parameter out of bound in IColumnString::insert_range_from
method.");
+ }
+
+ size_t nested_offset = src_concrete.offset_at(start);
+ size_t nested_length = src_concrete.offsets[start + length - 1] -
nested_offset;
- auto old_char_size = chars.size();
- size_t total_chars_size = old_char_size;
+ size_t old_chars_size = chars.size();
+ chars.resize(old_chars_size + nested_length);
+ memcpy(&chars[old_chars_size], &src_concrete.chars[nested_offset],
nested_length);
- auto dst_offsets_pos = offsets.size();
- offsets.resize(offsets.size() + indices_end - indices_begin);
- auto* dst_offsets_data = offsets.data();
+ if (start == 0 && offsets.empty()) {
+ offsets.assign(src_concrete.offsets.begin(),
src_concrete.offsets.begin() + length);
+ } else {
+ size_t old_size = offsets.size();
+ size_t prev_max_offset = offsets.back(); /// -1th index is Ok, see
PaddedPODArray
+ offsets.resize(old_size + length);
- for (const auto* x = indices_begin; x != indices_end; ++x) {
- total_chars_size += src_offset_data[*x] - src_offset_data[int(*x) - 1];
- dst_offsets_data[dst_offsets_pos++] = total_chars_size;
+ for (size_t i = 0; i < length; ++i) {
+ offsets[old_size + i] =
+ src_concrete.offsets[start + i] - nested_offset +
prev_max_offset;
+ }
}
- check_chars_length(total_chars_size, offsets.size());
+}
+
+void ColumnString::insert_indices_from(const IColumn& src, const uint32_t*
indices_begin,
Review Comment:
warning: method 'insert_indices_from' can be made static
[readability-convert-member-functions-to-static]
be/src/vec/columns/column_string.h:483:
```diff
- void insert_indices_from(const IColumn& src, const uint32_t*
indices_begin,
+ static void insert_indices_from(const IColumn& src, const uint32_t*
indices_begin,
```
##########
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)
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) final {
```
##########
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) 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) 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) 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) 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) 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)
final {
```
##########
be/src/vec/columns/column_string.h:
##########
@@ -571,15 +572,42 @@
}
void get_indices_of_non_default_rows(Offsets64& indices, size_t from,
- size_t limit) const override {
+ size_t limit) const 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 final;
- double get_ratio_of_default_rows(double sample_ratio) const override {
+ double get_ratio_of_default_rows(double sample_ratio) const final {
return get_ratio_of_default_rows_impl<ColumnString>(sample_ratio);
}
+
+ ColumnPtr convert_column_if_overflow() final;
+
+ virtual bool is_large_string() const { return false; }
+
+ virtual void* get_offsets_ptr() const { return (void*)offsets.data(); }
};
+// The column only use in Join case in build side. the column iterface use in
+// build side must overrided the function and use `large_offsets`
+// * 1. `get_max_row_byte_size`
+// * 2. `serialize_vec`
+// * 3. `serialize_value_in_into_arena`
+// * 4. `is_large_string`
+class ColumnLargeStringForJoin final : public ColumnString {
+public:
+ PaddedPODArray<UInt64> large_offsets;
+
+ StringRef serialize_value_into_arena(size_t n, Arena& arena, char const*&
begin) const override;
+
+ void serialize_vec(std::vector<StringRef>& keys, size_t num_rows,
+ size_t max_row_byte_size) const override;
+
+ size_t get_max_row_byte_size() const override;
+
+ bool is_large_string() const override { return true; }
+
+ virtual void* get_offsets_ptr() const override { return
(void*)large_offsets.data(); }
Review Comment:
warning: 'virtual' is redundant since the function is already declared
'override' [modernize-use-override]
```suggestion
void* get_offsets_ptr() const override { return
(void*)large_offsets.data(); }
```
##########
be/src/vec/columns/column_string.h:
##########
@@ -571,15 +572,42 @@
}
void get_indices_of_non_default_rows(Offsets64& indices, size_t from,
- size_t limit) const override {
+ size_t limit) const 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 final;
- double get_ratio_of_default_rows(double sample_ratio) const override {
+ double get_ratio_of_default_rows(double sample_ratio) const 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) final {
```
##########
be/src/vec/columns/column_string.h:
##########
@@ -478,62 +477,63 @@
}
}
- 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)
final;
+
+ void insert_range_from_ignore_overflow(const IColumn& src, size_t start,
size_t length) final;
void insert_indices_from(const IColumn& src, const uint32_t* indices_begin,
- const uint32_t* indices_end) override;
+ const uint32_t* indices_end) 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 final;
+ size_t filter(const Filter& filter) 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) final;
- ColumnPtr permute(const Permutation& perm, size_t limit) const override;
+ ColumnPtr permute(const Permutation& perm, size_t limit) const 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 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() final { offsets.push_back(chars.size()); }
- void insert_many_defaults(size_t length) override {
+ void insert_many_defaults(size_t length) 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 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 final;
- ColumnPtr replicate(const Offsets& replicate_offsets) const override;
+ ColumnPtr replicate(const Offsets& replicate_offsets) const final;
void append_data_by_selector(MutableColumnPtr& res,
- const IColumn::Selector& selector) const
override {
+ const IColumn::Selector& selector) const
final {
append_data_by_selector_impl<ColumnString>(res, selector);
}
void append_data_by_selector(MutableColumnPtr& res, const
IColumn::Selector& selector,
- size_t begin, size_t end) const override {
+ size_t begin, size_t end) const final {
append_data_by_selector_impl<ColumnString>(res, selector, begin, end);
}
- // void gather(ColumnGathererStream & gatherer_stream) override;
- void reserve(size_t n) override;
+ void reserve(size_t n) final;
- void resize(size_t n) override;
+ void resize(size_t n) final;
- bool is_column_string() const override { return true; }
+ bool is_column_string() const 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() final { return true; }
```
##########
be/src/vec/columns/column_string.h:
##########
@@ -478,62 +477,63 @@
}
}
- 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)
final;
+
+ void insert_range_from_ignore_overflow(const IColumn& src, size_t start,
size_t length) final;
void insert_indices_from(const IColumn& src, const uint32_t* indices_begin,
- const uint32_t* indices_end) override;
+ const uint32_t* indices_end) 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 final;
+ size_t filter(const Filter& filter) 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) final;
- ColumnPtr permute(const Permutation& perm, size_t limit) const override;
+ ColumnPtr permute(const Permutation& perm, size_t limit) const 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 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() final { offsets.push_back(chars.size()); }
- void insert_many_defaults(size_t length) override {
+ void insert_many_defaults(size_t length) 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 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 final;
- ColumnPtr replicate(const Offsets& replicate_offsets) const override;
+ ColumnPtr replicate(const Offsets& replicate_offsets) const final;
void append_data_by_selector(MutableColumnPtr& res,
- const IColumn::Selector& selector) const
override {
+ const IColumn::Selector& selector) const
final {
append_data_by_selector_impl<ColumnString>(res, selector);
}
void append_data_by_selector(MutableColumnPtr& res, const
IColumn::Selector& selector,
- size_t begin, size_t end) const override {
+ size_t begin, size_t end) const final {
append_data_by_selector_impl<ColumnString>(res, selector, begin, end);
}
- // void gather(ColumnGathererStream & gatherer_stream) override;
- void reserve(size_t n) override;
+ void reserve(size_t n) final;
- void resize(size_t n) override;
+ void resize(size_t n) final;
- bool is_column_string() const override { return true; }
+ bool is_column_string() const final { return true; }
- bool structure_equals(const IColumn& rhs) const override {
+ bool structure_equals(const IColumn& rhs) const 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) 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) 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) 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
