ctsk commented on PR #16083: URL: https://github.com/apache/datafusion/pull/16083#issuecomment-2927498848
Alrighty! Regarding the lack of support for RightMark joins in some join operators, I believe it would be best to return an error in the constructor of those operators if they do not support the JoinType. I believe the PR currently would just return a wrong result. I don't (yet) know how much work it would be to adjust physical optimizer rules so that no bad JoinType x OperatorType combination gets constructed. Algorithmically, what I found hardest to understand is the switcheroo between build and probe side - You've modified `adjust_indices_by_join_type` for then `RightMark` join so that `left_indices` holds the indices for the probe batch and `right_indices` holds the mark. Then you also switch `probe_batch`, `build_input_buffer and the `build_side` in the call to `build_batch_from_indices` so that it all works out..... It's kind of brilliant, but would be best to **at least** document that this is going on. I think a less confusing alternative would be to just add a `mark_side: bool` parameter to `build_batch_from_indices` that lets it distinguish between LeftMark and RightMark join for the ColumnSide::None case. -- 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
