abstractdog edited a comment on pull request #1824: URL: https://github.com/apache/hive/pull/1824#issuecomment-756195387
ran through the patches, I'll comment on particular code sections, but first a question in general: is there a specific reason for doing this method transformation? ``` compute_bit_vector(x, 'hll') -> compute_bit_vector_hll(x) ``` while I was reading the qout changes I realized that this rewrite makes this patch so huge, and I'm not sure if it's really worth, as I cannot see the added value, and I personally don't have any problem with the algorithm as a parameter, also what's the default behavior? this would make sense to me: ``` compute_bit_vector(x) -- could be translated to --> compute_bit_vector(x, 'hll') ``` without added value, e.g. the backport of this patch will be painful if we keep it this way, and the patch is about vectorization coverage, and has nothing to do with compiler basically, what do you think? ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
