berkaysynnada commented on PR #16196: URL: https://github.com/apache/datafusion/pull/16196#issuecomment-2922380421
@pepijnve thanks for the example. However, the same example can also consume the pending return generated by Aggregate, so the guarantee cannot be upheld in our model. I still believe consolidating this at the source is a better approach. It would also be easily configurable during planning (as the patches spread, one could choose to disable it entirely). Even if I couldn't get consensus on that direction, this version could still be improved. For example, by inserting the YieldStream only when it’s truly needed, such as in the empty GROUP BY case. I know having GROUP BY can still cause such situations, but if we’re going to fix things reactively based on complaints, then the fixes should consistently address similar cases. Lastly, adding also a cancellation timeout test will be good -- 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...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
