andygrove commented on PR #6835: URL: https://github.com/apache/datafusion-comet/pull/6835#issuecomment-6124584423
Thanks, both. This is all in 2f95eefb36. @viirya, you're right that `ineligible_reason` keeps the outer joins away from the dynamic filter, so that sentence is gone from the description. The walk is in, with one change I explained in that thread. @dwsmith1983, the aggregate test now pins each join's class and build side, and asserts that both aggregates sit above the join with no exchange in between. The comment in `CometHashJoin.doConvert` names the null-aware anti join as the exception, and the planner test asserts that both shapes plan to a `Right` join. Your NOT IN test is in already, since the walk stops at the null-aware anti join, and it keeps the default broadcast threshold with the hint on the outer join, as you noted. The fallback was deliberate, since nothing at that point can tell whether an operator above relies on the order, but it was broader than it needed to be. Declaring any order makes DataFusion keep the probe rows in place, so `doConvert` now sends the longest prefix of the order that Comet can express, and only a join whose first key it can't express stays in Spark. The fallback test covers both cases. With the walk, this branch no longer splits groups above a NOT IN, but the order there is still lost until #6810 sorts it, so the plan stays the same: I'll rebase onto #6810 once it's in and move `streamed_sort_orders` to field 12. -- 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]
