[GitHub] [incubator-pinot] buchireddy commented on issue #4321: #4317 Feature/variable length bytes offline dictionary for indexing bytes and string dicts.

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

buchireddy commented on issue #4321: #4317 Feature/variable length bytes offline dictionary for indexing bytes and string dicts.
URL: https://github.com/apache/incubator-pinot/pull/4321#issuecomment-507135532
 
 
   FTR, I was waiting to address review comments on this PR until now because @mcvsubbu was working on pulling out `Buffer` as a common class (PR #4348) which can be reused in this PR as well. I personally think that's a nice-to-have change and will help to reuse the byte array store code for both mutable and immutable dictionaries.
   However, @Jackie-Jiang is working on refactoring/cleaning the mutable byte array store related code so #4348 isn't going to be merged soon. Also, @Jackie-Jiang and I discussed that the byte array store used for VariableLength dictionary is different enough from the regular mutable store that it's okay for VarLength dictionary to have it's own immutable bytes store. We can definitely find better optimizations for immutable byte store that may not be applicable for mutable store.
   Hence, I am now going to work on addressing the review comments on this PR and will get it ready to be merged. @mcvsubbu @Jackie-Jiang Please reply if you guys think otherwise.
   cc @kishoreg 

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org