Re: [PR] support string column not overflow in join merge [doris]

[via GitHub]

github-actions[bot] commented on code in PR #33584:
URL: https://github.com/apache/doris/pull/33584#discussion_r1562160373


##########
be/src/vec/columns/column_string.cpp:
##########
@@ -284,6 +288,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:598:
   ```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:291:
   ```diff
   -                                                                char 
const*& begin) const {
   +                                                                char 
const*& begin) {
   ```
   



##########
be/src/vec/columns/column_string.h:
##########
@@ -106,28 +106,30 @@ 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; }
+    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.h:
##########
@@ -567,15 +571,38 @@
     }
 
     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 
{
         return get_ratio_of_default_rows_impl<ColumnString>(sample_ratio);
     }
+
+    ColumnPtr convert_column_if_overflow() override final;

Review Comment:
   warning: 'override' is redundant since the function is already declared 
'final' [modernize-use-override]
   
   ```suggestion
       ColumnPtr convert_column_if_overflow() final;
   ```
   



##########
be/src/vec/columns/column_string.cpp:
##########
@@ -308,6 +326,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:603:
   ```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() {
   ```
   



-- 
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