saadtajwar commented on issue #23974: URL: https://github.com/apache/datafusion/issues/23974#issuecomment-5363480995
> > [@rluvaton](https://github.com/rluvaton) / [@2010YOUY01](https://github.com/2010YOUY01) - going to take `NestedLoopJoinStream` next if that's OK! > > Thank you! I have a few thoughts you could consider. I’m only around 70% confident in them, so please only use them as suggestion. > > * For the complexity in NLJ, we might want to keep explicit state management, with the current state represented as an `enum`, and let that coexist with the generator pattern. My reasoning is: > a) We probably need a state-transition diagram to fully understand the implementation anyway, and structuring the code similarly makes it easier to reason about. > b) Explicit states make the entry and exit conditions for each state visible, which may make the implementation safer. > * We could split the regular joins (inner, left, right, full) and the semi/anti joins into separate streams. They are really different relational operations, and different optimizations tend to apply to each. The current approach combines them and therefore needs several flags/configurations to route the internal logic, which adds complexity. We have already made a similar split in sort-merge join. > > This second point may be slightly outside the scope of this project, but since we are already doing a fairly large refactor, I wanted to mention it briefly. Thanks for the detailed thoughts @2010YOUY01 ! This definitely makes sense to me - it seems like it might be a bit easier to split this into separate PRs of: - First separating regular joins from semi/anti/mark joins (preserving the existing polling behavior/spilling/state machines) - Then converting the streams to generators independently If that makes sense to you then I'll go ahead and start on implementation :) -- 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]
