leerho commented on issue #6865: Densify swapped hll buffer
URL: https://github.com/apache/incubator-druid/pull/6865#issuecomment-465466750
 
 
   Thank you for your thoughtful comments.  I looked again at the “noop” line 
... and you are right ... I missed it!  But it is a convoluted statement that 
would have benefited from a code comment!
   
   I also agree that throwing an exception would make a lot of sense.  That may 
cause some of the tests that take advantage of the ability to fill the Sketch 
with illegal sequences to fail.  They would have to be rewritten.  I’m not 
sure, you would have to try it and see.  
   
   An ISE would be fine, as long as the message indicates that the user of the 
Sketch has done something seriously wrong be feeding the Sketch with an illegal 
(astronomically unlikely) sequence.  
   
   This Sketch is highly vulnerable to accidental corruption because the 
default hash seed used by the aggregator is zero.  And most people when using 
hash functions never bother with a seed.  So anybody using the 
MurmurHash3_64_128() in other parts of their code may inadvertently create 
correlation corruption effects with this Sketch.  And there is no warning!  
Unfortunately, I don’t know of any way to detect this or fix it in a compatible 
way WRT historically stored sketches.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to