SavicStefan opened a new pull request, #512:
URL: https://github.com/apache/datasketches-cpp/pull/512
Since the lazy KxQ/curMin rebuild was introduced ("skip updating kxq in HLL
merge"), Hll8Array::mergeHll only sets rebuild_kxq_curmin_ and defers the
recompute. Several consumers read that deferred state without honoring the
flag, so unioning HLL sketches with different lgConfigK could:
- discard accumulated data: isEmpty() reads curMin_/numAtCurMin_, which stay
at empty-sketch defaults after a downsampling merge, so a populated gadget
looks empty and the next union update overwrites it;
- corrupt the estimate: internalCouponUpdate does incremental KxQ/HIP
updates against the stale base;
- serialize non-deterministically: copyAs() rebuilds via register replay
(convertToHll8) while estimate/bounds rebuild via the direct sum, so equivalent
merges could serialize to different bytes.
The effect was estimates collapsing to ~one input's cardinality (merge-order
dependent) and merge-order-dependent serialization; 4.0.1 had neither.
Fix, keeping the lazy-merge optimization:
- isEmpty(): a pending rebuild means the array is non-empty.
- internalCouponUpdate(): rebuild before an incremental update reads KxQ,
only when the coupon changes a register (duplicate coupons stay lazy).
- copyAs(): make every same-type result use the direct-sum rebuild so all
paths agree, keeping serialization merge-order independent.
Adds a regression test covering estimate order-independence, is_empty()
after a downsampling merge, scalar-after-merge accumulation, and
merge-order-independent serialization.
--
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]
