andygrove commented on PR #5004: URL: https://github.com/apache/datafusion-comet/pull/5004#issuecomment-5070215551
On the design question: this is the intended tradeoff, and as you noted it is a property of #5003 rather than this PR (`main` already routes oversized batches through the coalescer). A single-partition shuffle feeds one reducer, so passing an already-materialized `>= batch_size` batch through as one block just means a single larger IPC message instead of re-splitting it, which is exactly what #5003's bypass is for. Rather than guard it, I rebased this PR onto #5003 and made it agree with that behavior: - The doc comment and `native_shuffle.md` drop the uniform-`batch_size` / chunk-independence wording and now describe the passthrough and the resulting variable block sizes. - The single-partition test leads with an oversized batch on an empty buffer so it exercises the passthrough and asserts `[250, 100, 30]`. I left #5003's merged description as-is. Happy to open a quick follow-up to drop its "identical bytes" wording if you think it is worth it. -- 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]
