[GitHub] [hive] abstractdog commented on pull request #1824: HIVE-24510: Vectorize compute_bit_vector

[GitBox <gi...@apache.org>]

abstractdog commented 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 problems with the algorithm as a parameter, not the mention the possible fallback to a default maybe?
   ```
   compute_bit_vector(x) -- could be translated to --> compute_bit_vector_hll(x, 'hll') 
   ```
   I would consider this, as, without added value, e.g. the backport of this patch will be painful, and this 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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org