nooneuse commented on code in PR #67469:
URL: https://github.com/apache/doris/pull/67469#discussion_r4002108026
##########
be/src/exprs/aggregate/aggregate_function_datasketches_hll_union_agg.h:
##########
@@ -91,12 +84,32 @@ struct AggregateFunctionHllSketchData {
"Internal error happened when update HLL sketch:
unknown exception.");
}
}
- void reset() {
- if (hll_union_data.has_value()) {
- hll_union_data->reset();
+ void merge(const Sketch& sketch_data) {
+ if (sketch_data.is_empty()) {
+ return;
}
- hll_union_data.reset();
+ const auto lg_max_k = std::max<uint8_t>(sketch_data.get_lg_config_k(),
MIN_UNION_LOG_K);
Review Comment:
Thanks, this issue is valid.
The missing validation is in DataSketches: its normal constructor rejects
lgK outside [4, 21], but deserialization can construct a sparse sketch with
lgK=22.
I also traced the exception handling. So yes, Normal pipeline execution
catches the resulting std::invalid_argument and fails the query with
INTERNAL_ERROR.
I will address the validation gap upstream in DataSketches, then update the
pinned dependency and add Doris coverage for the expected CORRUPTION error.
Once DataSketches rejects the invalid header inside deserialize(), Doris's
existing deserialization catch provides the required conversion. This fixes the
library invariant at its source rather than duplicating sketch-format
validation in Doris.
--
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]
