github-actions[bot] commented on code in PR #33584:
URL: https://github.com/apache/doris/pull/33584#discussion_r1564634773
##########
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,
```
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -308,6 +357,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:606:
```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_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/columns/column_string.cpp:
##########
@@ -284,6 +319,20 @@
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:601:
```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:322:
```diff
- char
const*& begin) const {
+ char
const*& begin) {
```
##########
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:143:
```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_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:484:
```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_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:487:
```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,
```
--
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
