Yicong-Huang commented on PR #58990: URL: https://github.com/apache/spark/pull/58990#issuecomment-5876098855
> 1. The PR goes a bit beyond a pure move. `ArrowBatchTransformer.concat_batches` now returns `None` for empty input instead of asserting, and the existing `ArrowGroupedAggUDFHandler` and `ArrowWindowAggUDFHandler` switch to passing the group iterator straight in. That's behavior-neutral: the other caller in `worker.py`'s `emit_ready` only calls it when `needed > 0`, so its input is never empty. Still, a line in the description would help anyone who later bisects a change to `concat_batches`. > 2. The new `concat_batches` docstring says a caller can pass a group iterator "without materializing it to a list first", but the function still does `batches = tuple(batches)`. The actual saving is avoiding the extra `list()` before that `tuple()`. Rewording along those lines would avoid suggesting it streams. Thanks @HyukjinKwon! I think both are valid but would be ideal to resolve out of this PR. @Spenserrrr do you want to fix the `concat_batches` transformer separately? thanks! -- 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]
