clintropolis commented on PR #14866: URL: https://github.com/apache/druid/pull/14866#issuecomment-1684488529
>longLast, longFirst, doubleFirst, doubleLast could stop lying about their intermediate types. IIRC they only had to lie about this to make groupBy v1 work. I actually did this initially, but then noticed that #14462 was also making this change, so reverted for now. That said, it would only be a minor conflict with the other PR, so Im willing to add it back. >OnheapIncrementalIndex no longer needs to support multithreaded writers, so addToFacts could be simplified Yeah, totally, i actually imagine a lot of other parts of `IncrementalIndex` could also be improved. Should I do that in this PR or save it for a follow-up? In the very least i can remove the group by v1 comment in `addToFacts` and add another comment to replace it indicating that the condition should now be impossible and we should work to simplify the code area in the future -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
