gianm commented on issue #12022:
URL: https://github.com/apache/druid/issues/12022#issuecomment-985596977


   > Workaround (Rejected):
   > To retain compatibilty, we could add a new default long 
aggregateAndEstimateMemory() method and leave the existing aggregate() method 
as is. The default implementation would return a negative value (say -1) in 
which case the OnHeapIncrementalIndex or any other estimator would use the max 
estimated size for that invocation.
   
   This approach actually seems like a good idea to me for a few reasons:
   
   - Aggregation is hot code, and there will be some overhead to size 
estimation, and query-time callers don't necessarily need it. So having two 
methods allows the caller to decide if it actually needs size estimates or not.
   - We want to avoid incompatible changes to aggregation interfaces whenever 
possible. 


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

Reply via email to