zhuqi-lucas commented on PR #16196: URL: https://github.com/apache/datafusion/pull/16196#issuecomment-2930649676
> > My thinking was that we could use EmissionType to insert the yield wrapper closer to where it's needed rather than at the leaves. > > I don't think this is a good idea. We can have many operators in a plan that break the pipeline, especially in plans with joins and windowing (cascades of windowing operators are quite common). Simply adding a `YieldExec` as a parent to each pipeline-breaking operator would be bad -- prompting the question of where to insert the `YieldExec` arises in such plans. > > Inserting it as a parent of leaf nodes, and only when necessary (first item in my message above), gives us a system where the least number of necessary `YieldExec`s are inserted, and at a non-arbitrary place. I agree with this now. I actually tried using EmissionType to insert the YieldExec wrapper closer to where it's needed instead of just at the leaves. However, during SQL logic test runs with complex plans, it caused many errors, including stack overflows. I believe this approach introduces more edge cases and makes the behavior harder to reason about. -- 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
