github-actions[bot] commented on code in PR #31769:
URL: https://github.com/apache/doris/pull/31769#discussion_r1531443159
##########
be/src/vec/data_types/serde/data_type_array_serde.cpp:
##########
@@ -224,33 +226,40 @@ void DataTypeArraySerDe::write_one_cell_to_jsonb(const
IColumn& column, JsonbWri
result.writeEndBinary();
}
-void DataTypeArraySerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
-
rapidjson::Document::AllocatorType& allocator,
- int row_num) const {
- // vectorized::Field array = column[row_num];
+Status DataTypeArraySerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
Review Comment:
warning: method 'write_one_cell_to_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status DataTypeArraySerDe::write_one_cell_to_json(const IColumn&
column, rapidjson::Value& result,
```
be/src/vec/data_types/serde/data_type_array_serde.cpp:230:
```diff
- int row_num) const {
+ int row_num) {
```
##########
be/src/vec/data_types/serde/data_type_serde.cpp:
##########
@@ -82,14 +82,15 @@
}
}
-void DataTypeSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
- rapidjson::Document::AllocatorType&
allocator,
- int row_num) const {
- LOG(FATAL) << fmt::format("Not support write {} to rapidjson",
column.get_name());
+Status DataTypeSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
+
rapidjson::Document::AllocatorType& allocator,
+ int row_num) const {
+ return Status::InternalError("Not support write {} to rapidjson",
column.get_name());
}
-void DataTypeSerDe::read_one_cell_from_json(IColumn& column, const
rapidjson::Value& result) const {
- LOG(FATAL) << fmt::format("Not support read {} from rapidjson",
column.get_name());
+Status DataTypeSerDe::read_one_cell_from_json(IColumn& column,
+ const rapidjson::Value& result)
const {
Review Comment:
warning: method 'read_one_cell_from_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status DataTypeSerDe::read_one_cell_from_json(IColumn& column,
const rapidjson::Value&
result) {
```
##########
be/src/vec/data_types/serde/data_type_nullable_serde.cpp:
##########
@@ -334,29 +334,33 @@
return Status::OK();
}
-void DataTypeNullableSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
-
rapidjson::Document::AllocatorType& allocator,
- int row_num) const {
+Status DataTypeNullableSerDe::write_one_cell_to_json(const IColumn& column,
+ rapidjson::Value& result,
+
rapidjson::Document::AllocatorType& allocator,
+ int row_num) const {
auto& col = static_cast<const ColumnNullable&>(column);
auto& nested_col = col.get_nested_column();
if (col.is_null_at(row_num)) {
result.SetNull();
} else {
- nested_serde->write_one_cell_to_json(nested_col, result, allocator,
row_num);
+ RETURN_IF_ERROR(
+ nested_serde->write_one_cell_to_json(nested_col, result,
allocator, row_num));
}
+ return Status::OK();
}
-void DataTypeNullableSerDe::read_one_cell_from_json(IColumn& column,
- const rapidjson::Value&
result) const {
+Status DataTypeNullableSerDe::read_one_cell_from_json(IColumn& column,
+ const rapidjson::Value&
result) const {
auto& col = static_cast<ColumnNullable&>(column);
Review Comment:
warning: method 'read_one_cell_from_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
,static
{
```
##########
be/src/vec/data_types/serde/data_type_nullable_serde.cpp:
##########
@@ -334,29 +334,33 @@ Status DataTypeNullableSerDe::write_column_to_orc(const
std::string& timezone,
return Status::OK();
}
-void DataTypeNullableSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
-
rapidjson::Document::AllocatorType& allocator,
- int row_num) const {
+Status DataTypeNullableSerDe::write_one_cell_to_json(const IColumn& column,
Review Comment:
warning: method 'write_one_cell_to_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static
```
be/src/vec/data_types/serde/data_type_nullable_serde.cpp:339:
```diff
- ,
- {
+ ,
+ {
```
##########
be/src/vec/data_types/serde/data_type_jsonb_serde.cpp:
##########
@@ -203,17 +203,17 @@ static void convert_jsonb_to_rapidjson(const JsonbValue&
val, rapidjson::Value&
}
}
-void DataTypeJsonbSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
-
rapidjson::Document::AllocatorType& allocator,
- int row_num) const {
- auto& data = assert_cast<const ColumnString&>(column);
+Status DataTypeJsonbSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
Review Comment:
warning: method 'write_one_cell_to_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status DataTypeJsonbSerDe::write_one_cell_to_json(const IColumn&
column, rapidjson::Value& result,
```
be/src/vec/data_types/serde/data_type_jsonb_serde.cpp:207:
```diff
- int row_num) const {
+ int row_num) {
```
##########
be/src/vec/data_types/serde/data_type_jsonb_serde.cpp:
##########
@@ -222,10 +222,11 @@
} else {
result = std::move(value);
}
+ return Status::OK();
}
-void DataTypeJsonbSerDe::read_one_cell_from_json(IColumn& column,
- const rapidjson::Value&
result) const {
+Status DataTypeJsonbSerDe::read_one_cell_from_json(IColumn& column,
+ const rapidjson::Value&
result) const {
Review Comment:
warning: method 'read_one_cell_from_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status DataTypeJsonbSerDe::read_one_cell_from_json(IColumn& column,
const rapidjson::Value&
result) {
```
##########
be/src/vec/columns/column_object.cpp:
##########
@@ -1106,15 +1110,17 @@ bool ColumnObject::serialize_one_row_to_json_format(int
row, rapidjson::StringBu
} else {
output->Clear();
rapidjson::Writer<rapidjson::StringBuffer> writer(*output);
- return root.Accept(writer);
+ if (!root.Accept(writer)) {
+ return Status::InternalError("Failed to serialize json value");
+ }
}
- return true;
+ return Status::OK();
}
-void ColumnObject::merge_sparse_to_root_column() {
+Status ColumnObject::merge_sparse_to_root_column() {
Review Comment:
warning: function 'merge_sparse_to_root_column' exceeds recommended
size/complexity thresholds [readability-function-size]
```cpp
Status ColumnObject::merge_sparse_to_root_column() {
^
```
<details>
<summary>Additional context</summary>
**be/src/vec/columns/column_object.cpp:1119:** 88 lines including whitespace
and comments (threshold 80)
```cpp
Status ColumnObject::merge_sparse_to_root_column() {
^
```
</details>
##########
be/src/vec/columns/column_object.cpp:
##########
@@ -1106,15 +1110,17 @@
} else {
output->Clear();
rapidjson::Writer<rapidjson::StringBuffer> writer(*output);
- return root.Accept(writer);
+ if (!root.Accept(writer)) {
+ return Status::InternalError("Failed to serialize json value");
+ }
}
- return true;
+ return Status::OK();
}
-void ColumnObject::merge_sparse_to_root_column() {
+Status ColumnObject::merge_sparse_to_root_column() {
Review Comment:
warning: method 'merge_sparse_to_root_column' can be made const
[readability-make-member-function-const]
be/src/vec/columns/column_object.h:263:
```diff
- Status merge_sparse_to_root_column();
+ Status merge_sparse_to_root_column() const;
```
```suggestion
Status ColumnObject::merge_sparse_to_root_column() const {
```
##########
be/src/vec/data_types/serde/data_type_serde.cpp:
##########
@@ -82,14 +82,15 @@ void DataTypeSerDe::convert_field_to_rapidjson(const
vectorized::Field& field,
}
}
-void DataTypeSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
- rapidjson::Document::AllocatorType&
allocator,
- int row_num) const {
- LOG(FATAL) << fmt::format("Not support write {} to rapidjson",
column.get_name());
+Status DataTypeSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
Review Comment:
warning: method 'write_one_cell_to_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status DataTypeSerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
```
be/src/vec/data_types/serde/data_type_serde.cpp:86:
```diff
- int row_num) const {
+ int row_num) {
```
##########
be/src/vec/data_types/serde/data_type_array_serde.cpp:
##########
@@ -224,33 +226,40 @@
result.writeEndBinary();
}
-void DataTypeArraySerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
-
rapidjson::Document::AllocatorType& allocator,
- int row_num) const {
- // vectorized::Field array = column[row_num];
+Status DataTypeArraySerDe::write_one_cell_to_json(const IColumn& column,
rapidjson::Value& result,
+
rapidjson::Document::AllocatorType& allocator,
+ int row_num) const {
// Use allocator instead of stack memory, since rapidjson hold the
reference of String value
// otherwise causes stack use after free
auto& column_array = static_cast<const ColumnArray&>(column);
+ if (row_num > column_array.size()) {
+ return Status::InternalError("row num {} out of range {}!", row_num,
column_array.size());
+ }
void* mem = allocator.Malloc(sizeof(vectorized::Field));
+ if (!mem) {
+ return Status::InternalError("Malloc failed");
+ }
vectorized::Field* array = new (mem)
vectorized::Field(column_array[row_num]);
convert_field_to_rapidjson(*array, result, allocator);
+ return Status::OK();
}
-void DataTypeArraySerDe::read_one_cell_from_json(IColumn& column,
- const rapidjson::Value&
result) const {
+Status DataTypeArraySerDe::read_one_cell_from_json(IColumn& column,
+ const rapidjson::Value&
result) const {
Review Comment:
warning: method 'read_one_cell_from_json' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status DataTypeArraySerDe::read_one_cell_from_json(IColumn& column,
const rapidjson::Value&
result) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
