drcrallen edited a comment on issue #6865: Densify swapped hll buffer URL: https://github.com/apache/incubator-druid/pull/6865#issuecomment-470253897 If the solutions were presented which fixed up items related to the core HLL aggregator itself. This part of the code still would have the assumption that the buffer is densified. I do not have full coverage for all test cases where it may or may not have dense buffers at this code point. So there may in the future be other cases where it is "normal". The workflow assumes dense so I added a check to rectify when that is not the case. Fixes to the overall HLL algorithm itself are beyond the scope of this PR, and presenting HLL results which "don't make good sense" because of bad input values is not solved by this PR. Meaning if someone puts in values that do not encounter this error case, but still violate some of the (poorly documented) design constraints of the HLL aggregator, then it will STILL present nonsense values even IF this particular code path fix is not hit. In other words NOT hitting this error case does NOT mean the HLL approximation is GOOD, just that it didn't crash. Therefore I argue for keeping the code from crashing, thereby making ALL poorly constructed input values simply return whatever the algorithm can calculate.
---------------------------------------------------------------- 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. 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]
