fix: HyperLogLogCollector returns zero cardinality when a single element overflows into sparse mode#19678
Conversation
…ent overflows into sparse mode When an element's positionOf1 exceeds the 4-bit nibble range, its value is kept only in the overflow register and no sparse entry is written. If that register is the only data the collector holds, estimateSparse() never visits it, zeroCount stays at NUM_BUCKETS, and linear counting estimates a cardinality of 0. Account for a non-zero overflow register that was not folded into a visited entry.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 2 of 2 changed files.
This is an automated review by Codex GPT-5.6-Sol
| } | ||
| 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; |
There was a problem hiding this comment.
[P2] Normalize sparse positions before marking overflow applied
position is the serialized byte offset, which toByteBuffer() writes as the payload index plus getNumHeaderBytes() (7 for a V1 sketch), while overflowPosition is only overflowRegister >>> 1. Consequently this flag can remain false after visiting the overflow bucket's actual sparse byte, or become true for an unrelated entry. For example, if a bucket first receives an in-range value and then an overflow value, a sparse round-trip stores its tuple at overflowPosition + 7; the new fallback then treats the overflow as a second previously empty register and decrements zeroCount again, making one populated HLL register estimate approximately 2 instead of 1. Compare positions in the same coordinate system before deciding whether standalone compensation is needed.
There was a problem hiding this comment.
Confirmed. In the sparse buffer position is the serialized offset (payload byte index + getNumHeaderBytes()), whereas overflowPosition is overflowRegister >>> 1, so the in-loop check never matched the overflow bucket's own byte and overflowRegisterApplied was unreliable. In the in-range-then-overflow case the register was then counted twice — its sparse entry plus the fallback — estimating 2 for a single populated register.
Fixed by normalizing position to the payload byte index (position - numHeaderBytes) before comparing with overflowPosition, so the in-loop overflow handling matches the correct entry and the fallback only fires when the overflow bucket has no sparse entry. Added regression tests for the single-register round-trip (now estimates 1, was 2) and for an overflow register sitting alongside other sparse entries.
There was a problem hiding this 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.
There was a problem hiding this comment.
Confirmed. Both checks in that line operate on decoded scalars, so the nibble masks were wrong on both sides: (lowerNibble & 0x0f) == 0 marks decoded 16/32/48 as empty (the odd-bucket case you describe, which estimated 0), and (upperNibble & 0xf0) == 0 marks decoded 1-15 as empty, so a populated neighbor register sharing the overflow byte was miscounted as well. Replaced both with direct == 0 tests on the decoded values.
The same pattern exists in estimateDense, where the position comparison has always matched the overflow byte. A dense collector holding a single odd-bucket overflow register (for example add((short) 5, (byte) 3) followed by add((short) 5, (byte) 16), with no serialization round-trip) also estimated 0, independently of this PR's earlier changes. Fixed that occurrence the same way.
Added regression tests: odd-bucket sparse round-trip (estimates 1), odd bucket with a populated neighbor in the same byte (estimates 2), and dense even/odd overflow without a round-trip (1 each).
The serialized sparse position is the payload byte index plus the header size, while overflowPosition is overflowRegister >>> 1, so the in-loop comparison never matched the overflow bucket's own entry. When a bucket held both an in-range and an overflow value, the register was counted twice (its sparse entry plus the standalone compensation), estimating 2 for a single populated register. Normalize the position to the payload byte index before comparing, and cover both cases with regression tests.
… register The zeroCount updates in estimateSparse and estimateDense applied nibble masks to decoded scalar values. An overflow value such as 16 folded into the lower nibble of an odd bucket satisfied (lowerNibble & 0x0f) == 0 and was counted as an empty register, collapsing the estimate of a single populated register to 0. Decoded in-range values 1-15 in the upper position were miscounted by (upperNibble & 0xf0) == 0 the same way. Test the decoded values against zero directly, and add regression coverage for odd buckets, a populated neighbor sharing the overflow byte, and dense collectors of both parities.
Fixes #19649.
Description
HyperLogLogCollector.estimateSparse()could return a cardinality estimate of0even after the collector had received data. In Hadoop-based ingestion this surfaces asDetermineHashedPartitionsJobreportingFound approximately [0] rows in dataand the job failing withNo buckets?? seems there is no data to index., even though the input records were read correctly.Root cause
When an element's
positionOf1exceeds the 4-bit nibble range (registerOffset + RANGE, i.e. 16 on a fresh collector), its value is kept only in the global overflow register and no entry is written to the sparse storage buffer. When that overflow register is the only information the collector holds — the typical case for a single such element — the sparse buffer is empty, so the scan inestimateSparse()never visits the overflow register:zeroCountstays equal toNUM_BUCKETSand the linear-counting correction evaluatesm * ln(m / m) = 0.The pre-existing in-loop handling for the overflow register (the
position == overflowPositionbranch) only runs when the overflow bucket shares a byte with a populated sparse entry, so it does not cover the empty-buffer case.Fix
estimateSparse()now records whether the overflow register was already folded into a visited entry. If a non-zero overflow register was not accounted for during the scan, it is added explicitly:zeroCountis decremented by one (the register is not empty) and1 / 2^overflowValueis added to the harmonic sum, mirroring how the dense path and the in-loop sparse path already handle the overflow register. Every other case — no overflow, an overflow register folded into an entry, and the dense path — is unchanged.Release note
Fixed a bug where a
hyperUnique/HLL sketch built from a very small number of values could estimate a cardinality of0when the only populated register was the overflow register. This could also cause Hadoop-based ingestion to fail withNo buckets?? seems there is no data to index.Key changed/added classes in this PR
HyperLogLogCollectorThis PR has: