siddharthteotia commented on PR #8917: URL: https://github.com/apache/pinot/pull/8917#issuecomment-1162534423
> 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. Sounds good. When we were iterating upon it internally, it was more of what all is needed to get this done correctly throughout the code base with all testing so there was some back n forth. Now that we agree with the overall work, I am ok with the above suggestion to breakdown. cc @somandal -- 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-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
