github-actions[bot] commented on code in PR #64152:
URL: https://github.com/apache/doris/pull/64152#discussion_r3386184224
##########
be/src/core/data_type_serde/data_type_quantilestate_serde.h:
##########
@@ -78,17 +78,26 @@ class DataTypeQuantileStateSerDe : public DataTypeSerDe {
Status write_column_to_pb(const IColumn& column, PValues& result, int64_t
start,
int64_t end) const override {
+ auto ptype = result.mutable_type();
+ ptype->set_id(PGenericType::QUANTILE_STATE);
+ const auto& col = assert_cast<const ColumnQuantileState&>(column);
result.mutable_bytes_value()->Reserve(cast_set<int>(end - start));
for (size_t row_num = start; row_num < end; ++row_num) {
- StringRef data = column.get_data_at(row_num);
- result.add_bytes_value(data.to_string());
+ const auto& val = col.get_element(row_num);
+ size_t size = val.get_serialized_size();
+ std::string buf(size, '\0');
+ val.serialize(reinterpret_cast<uint8_t*>(buf.data()));
+ result.add_bytes_value(std::move(buf));
}
return Status::OK();
}
Status read_column_from_pb(IColumn& column, const PValues& arg) const
override {
- column.reserve(arg.bytes_value_size());
+ auto& col = assert_cast<ColumnQuantileState&>(column);
+ col.reserve(arg.bytes_value_size());
for (int i = 0; i < arg.bytes_value_size(); ++i) {
- column.insert_data(arg.bytes_value(i).c_str(),
arg.bytes_value(i).size());
+ QuantileState value;
+ value.deserialize(Slice(arg.bytes_value(i)));
Review Comment:
`PValues` is not only read back from this same writer:
`RPCFnImpl::_convert_to_block()` reads `PFunctionCallResponse.result(0)` from
the RPC function server, and constant folding also moves `PValues` across
process boundaries. If that payload is malformed, truncated, or produced by an
implementation that still sends the old raw `QuantileState` object bytes,
`QuantileState::deserialize()` returns `false` and leaves `value` as the
default `EMPTY` state, but this code still inserts it and returns
`Status::OK()`. That makes the query silently lose the quantile value instead
of failing. Please propagate a non-OK `Status` here, and add a negative test
with an invalid `bytes_value`.
```suggestion
if (!value.deserialize(Slice(arg.bytes_value(i)))) {
return Status::InternalError(
"deserialize QuantileState from protobuf fail at
index {}", i);
}
```
--
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]
