dwsmith1983 commented on PR #6384: URL: https://github.com/apache/datafusion-comet/pull/6384#issuecomment-5997677138
> Could the rule put back the local sorts the rewrite took away? Done in 6f7b639f1, as you suggested: after the `transformUp`, `RewriteJoin.restoreRequiredOrdering` wraps any child whose `outputOrdering` does not satisfy its parent's `requiredChildOrdering` in `SortExec(required, global = false, child)`, and the normal conversion turns it into a `CometSortExec`. It only runs when `replaceSortMergeJoin` is on, since that rewrite is the only place sorts are removed. > Could we also add the chained join as a test with `checkSparkAnswer` in both AQE modes? Added, with your tables and the `spark.sql.adaptive.autoBroadcastJoinThreshold` override for the AQE run, plus a check that a sort sits between the kept join and the hash join. I also added `ExistenceJoin` and `LeftSemi` versions with no size limit. Those fail on main without the sort (1000 rows against 10 or 5), so the kept semi and existence joins were already losing rows; I filed #6673 for that and noted the backport in the description. -- 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]
