[GitHub] [pinot] somandal commented on pull request #8917: Add support for querying noDict MV columns for offline (all data types) and realtime (fixed width) segments

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

somandal commented on PR #8917:
URL: https://github.com/apache/pinot/pull/8917#issuecomment-1163194506

   > This PR adds support for querying multi-value columns in RAW format (without dictionary). Support to create segments with multi-value columns with dictionary disabled already existed. On trying to create a multi-value RAW column and querying it, we ran into this issue: #8875
   > 
   > This PR adds MV RAW support for the following types of columns:
   > 
   > * Offline segments with MV columns of types INT, LONG, FLOAT, DOUBLE, and STRING
   > * Mutable (realtime) segments with MV columns of INT, LONG, FLOAT, and DOUBLE
   > 
   > TODO - Support for variable length MV RAW columns for Mutable segments will be added in the future as it requires a mutable variable length writer.
   > 
   > Test cases for various types of queries have been added to ensure that MV RAW columns can be supported.
   > 
   > cc @siddharthteotia
   
   Thanks for taking a look and for the suggestion to break this down. I've got the first part out for review in this PR: https://github.com/apache/pinot/pull/8953
   
   
   > This is great, thanks for adding all the tests!
   > 
   > This PR is a little bit too large, and contains changes to lots of modules. I'd suggest breaking them into smaller ones so that each PR only focus on one thing. That can make both review and future tracking easier. We can break the PR into the following smaller scopes:
   > 
   > * Change `getValueType` to `getStoredType`
   > * New APIs to ForwardIndexReader and the implementations
   > * Filter support / group by support etc.
   
   Thanks for the suggestion @Jackie-Jiang! I've opened a new PR for the second one on your list "New APIs to ForwardIndexReader and the implementations" in this PR: https://github.com/apache/pinot/pull/8953. Can you take a look when you get a chance? (I've also tried to address your comments on that PR directly)
   
   I'll be working on the `getStoredType` after the other two.


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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