dwsmith1983 opened a new pull request, #7126: URL: https://github.com/apache/datafusion-comet/pull/7126
## Which issue does this PR close? Closes #6922. Found again in #6785 ([comment](https://github.com/apache/datafusion-comet/pull/6785#discussion_r4321400980)), where `forceShuffledHashJoin` reaches the same shape with no hint. ## Rationale for this change #6835 declares the streamed side's order on the probe input of a native outer hash join that runs as DataFusion's right outer join (`LeftOuter` with `BuildRight`, `RightOuter` with `BuildLeft`), so the unmatched probe rows stay in place. When `mayReportFalseOrder` cannot vouch for that order, the serde sends none, but it still converts the join. Spark has already dropped any sort above the join that the reported order satisfies, so the native join appends the unmatched rows after the matched ones and the order is lost. Both shapes in #6922 do this on main: a `localCheckpoint` of a sorted relation, and a cache written by an outer hash join whose probe input was a native filter. A `SHUFFLE_HASH` or `BROADCAST` hint over the checkpoint is enough to reproduce it, for both outer shapes. Falling back is the fix, not declaring the order anyway. An order that may not hold, once declared, lets an aggregate above run in sorted mode over unsorted rows and split groups. ## What changes are included in this PR? - `CometHashJoin.doConvert` falls back to Spark, with a reason, when an outer join of the two shapes reports an order that `mayReportFalseOrder` cannot vouch for. The check applies to both `CometHashJoinExec` and `CometBroadcastHashJoinExec`. - The fallback counts only an order that reaches the join through an input from the JVM (a cache, a checkpoint, a Spark leaf) or through a nested loop join. Inside one native plan, DataFusion carries a probe order through a join that kept it, so a chain of outer joins over a window, a filter or a range stays native as on main. `mayReportFalseOrder` takes a `countInPlan` flag for this. The order the serde sends is decided exactly as before. - An outer join over a nested loop join falls back as well. Over a one-row build side the nested loop join keeps its rows in order, so a native join above would move them. Over two or more build rows the rows are already out of order (#6898), and the fallback does not restore them, so this costs the native join on that shape without a gain until #6898 is fixed. - The compatibility guide and the contributor guide describe when the join falls back. ## How are these changes tested? New tests in `CometJoinSuite` and `CometInMemoryCacheSuite`: - A shuffled and a broadcast outer hash join over a sorted `localCheckpoint`, both outer shapes, AQE on and off: the join runs in Spark, no sort is added, and every partition stays in order. - #6922's cache repro, over Comet's cache and Spark's, AQE on and off: the second join falls back and keeps the order. - An outer join over a native inner join over a checkpoint falls back, while the inner join stays native. - Chained outer joins over a native window, and over a native filter over a sorted cache, stay native, with no fallback and the order kept. - An outer join over a nested loop join falls back. Its rows match Spark, and with a one-row build they stay in order. The checkpoint, cache and inner-join tests fail on main with rows out of order. The chained tests pass on main and failed on a broader version of this fallback that also counted joins in the same plan. Removing the fallback, letting in-plan joins count, passing the flag through a cache or wrapper, exempting the nested loop join, or dropping the join-shape check each fails a named test. `CometJoinSuite` and `CometInMemoryCacheSuite` pass on Spark 3.5, 4.0 and 4.1. -- 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]
