sandeshkr419 commented on code in PR #23816:
URL: https://github.com/apache/datafusion/pull/23816#discussion_r3677586866
##########
datafusion/functions-aggregate/src/approx_distinct.rs:
##########
@@ -336,13 +367,14 @@ impl GroupHll {
/// Merge a serialized state (produced by [`Self::serialize`] or by the
/// per-group [`Accumulator`]) into this sketch.
- fn merge_serialized(&mut self, bytes: &[u8]) -> Result<isize> {
+ fn merge_serialized(&mut self, bytes: &[u8], p: usize) -> Result<isize> {
if bytes.is_empty() {
return Ok(0);
}
- if bytes.len() == NUM_REGISTERS {
+ let num_registers = 1_usize << p;
+ if bytes.len() == num_registers {
Review Comment:
Fixed now, serialize now promotes to dense whenever the sparse byte length
would equal `1 << p`, avoiding the ambiguity; added roundtrip regression tests
at both collision points (p=4 with 2 hashes, p=10 with 128 hashes).
--
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]