realno commented on pull request #1525: URL: https://github.com/apache/arrow-datafusion/pull/1525#issuecomment-1008174462
@alamb thanks for the through review! I will open a followup PR to add `batch_update` and `batch_merge` as discussed. I want to discuss the aggregator interface a bit further. The `update` function doesn't seem to provide much value with the current architecture, it seems under all circumstances `batch_update` will perform better. Would it be a reasonable choice to just keep `batch_update` (and remove `update`)? This way we can get rid a lot of code dealing with record level ScalarValue arithmetic. IMO it only makes sense to differentiate the two if `update` can provide additional value such as lower latency in streaming context. With that said I am still new to the code base and could have missed something here. It would be nice to hear more people's opinion. Also please let me know if there is a more appropriate forum to discuss this. -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
