FrankChen021 commented on code in PR #19678:
URL: https://github.com/apache/druid/pull/19678#discussion_r3578877479
##########
processing/src/main/java/org/apache/druid/hll/HyperLogLogCollector.java:
##########
@@ -190,12 +191,22 @@ private static double estimateSparse(
}
e += 1.0d / Math.pow(2, upperNibble) + 1.0d / Math.pow(2, lowerNibble);
zeroCount += (((upperNibble & 0xf0) == 0) ? 1 : 0) + (((lowerNibble &
0x0f) == 0) ? 1 : 0);
+ overflowRegisterApplied = true;
Review Comment:
Thanks—the serialized-position normalization fixes the original double-count
for upper-nibble overflow buckets. One parity case remains: for an odd overflow
bucket, `lowerNibble` is already decoded to a scalar (for example, 16), so
`((lowerNibble & 0x0f) == 0)` incorrectly marks it as zero. `add((short) 5,
(byte) 3)`, then `add((short) 5, (byte) 16)`, followed by a sparse round-trip
therefore reaches `zeroCount == NUM_BUCKETS` and estimates 0. Please test
decoded values with `upperNibble == 0` / `lowerNibble == 0` and add odd-bucket
regression coverage.
Reviewed 2 of 2 changed files.
--
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]
