Jackie-Jiang commented on code in PR #20154:
URL: https://github.com/apache/pinot/pull/20154#discussion_r4474218295
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/DistinctCountHLLPlusValueAggregator.java:
##########
@@ -77,7 +78,7 @@ public HyperLogLogPlus getInitialAggregatedValue(@Nullable
Object rawValue) {
} else {
initialValue = new HyperLogLogPlus(_p, _sp);
offerValue(initialValue, rawValue);
- _maxByteSize = HyperLogLogPlusUtils.byteSize(_p, _sp);
+ _maxByteSize = HyperLogLogPlusUtils.maxByteSize(_p, _sp);
Review Comment:
Fixed in 5e9b49a714. The `byte[]` branch now uses
`HyperLogLogPlusUtils.maxByteSize(bytes)`, which reads `p` and `sp` from the
input's header, and keeps the max over the initial values instead of recording
an input's length. `DistinctCountHLLPlusValueAggregatorTest` covers your `p =
12`, `sp = 20` case: a 10-value input merged with a 1990-value input, followed
by a 5-value input.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/DistinctCountHLLPlusValueAggregator.java:
##########
@@ -132,7 +133,7 @@ public boolean isAggregatedValueFixedSize() {
public int getMaxAggregatedValueByteSize() {
// NOTE: For aggregated metrics, initial aggregated value might have not
been generated. Returns the byte size
// based on p and sp.
- return _maxByteSize > 0 ? _maxByteSize : HyperLogLogPlusUtils.byteSize(_p,
_sp);
+ return _maxByteSize > 0 ? _maxByteSize :
HyperLogLogPlusUtils.maxByteSize(_p, _sp);
Review Comment:
Done in 5e9b49a714. `isAggregatedValueFixedSize()` now returns `_sp == 0`,
so `distinctCountHLLPlus(metric, 12, 20)` fails ingestion config validation
with "must have fixed size aggregated value" (covered in `ValueAggregatorTest`
and `TableConfigUtilsTest`). One case it can't catch: pre-serialized HLL++
inputs with `sp > 0` ingested into a column configured with `sp = 0`, since the
configured `sp` doesn't describe the data.
--
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]
