andygrove opened a new pull request, #6907: URL: https://github.com/apache/datafusion-comet/pull/6907
## Which issue does this PR close? Closes #6903. ## Rationale for this change Spark gives a join the type `Cross` when the query asks for a cross join explicitly, with `CROSS JOIN` in SQL or `Dataset.crossJoin`, and Spark's join operators run `Cross` exactly as they run `Inner`. Comet's join serdes matched only `Inner`, so a cross join that Spark planned as a sort-merge, shuffled hash, broadcast hash or broadcast nested loop join fell back with `Unsupported join type Cross`, while the same join written as `JOIN ... ON` ran natively. A production job on Spark 3.5 hit this. A cross join with an equi-key filter became a `SortMergeJoin` of type `Cross` that produced 110 billion rows in Spark, and its stage took two thirds of the job's task time. ## What changes are included in this PR? - The sort-merge and hash join serdes map `InnerLike` (`Inner` and `Cross`) to the native `Inner` join type. The keys and condition carry over unchanged, so the native plan is the one the equivalent inner join gets. - The broadcast nested loop join accepts `InnerLike` with either build side, in its support check and its conversion, as it already did for `Inner`. - Removes the "Spark doesn't support other join types" comment from the three fallback cases. Spark does support `Cross`, and `LeftSingle` from Spark 4.0. - Marks Spark's `SPARK-26138 Pushdown limit through InnerLike when condition is empty` test `IgnoreComet` in all five Spark diffs, regenerated from the Spark tags after checking that each committed diff round-trips unchanged. The test takes `LIMIT 3` of `t1 CROSS JOIN t2` with no `ORDER BY` and expects the rows Spark returns. The join now runs as a native nested loop join, which emits rows in a different order, so Comet returns `(0,0), (1,0), (2,0)` where Spark returns `(0,0), (0,1), (0,2)`. Both are valid answers. `main` already returns Comet's rows for `FROM t1, t2 LIMIT 3`, an `Inner` join; the test passed only because the `Cross` join fell back. Native cross nested loop joins now also have the problem described in #6898, which inner ones already have: the join reports the streamed side's order without keeping it. `CartesianProductExec`, a cross join with no broadcast side, still falls back (#199). ## How are these changes tested? New tests in `CometJoinSuite`: - `CROSS JOIN ... WHERE tbl_a._2 = tbl_b._1`, with and without a residual condition, under `SHUFFLE_HASH`, `MERGE` and `BROADCAST` hints. - `CROSS JOIN` with a broadcast hint on either side, with no condition and with a non-equi condition, which Spark plans as a broadcast nested loop join. Each test checks the answer against Spark, checks that the plan is fully native, and checks that the native join has the type `Cross`, so a plan in which Spark chose `Inner` can't make it pass. With the serde change reverted, all four fail with `Unsupported join type Cross` or `Join type Cross is not supported`. Locally, `CometJoinSuite` passes on Spark 3.4, 3.5 and 4.1. The other Comet tests that use a cross join pass on Spark 3.4 and 4.1: `CometFloatSemanticsSuite`, the AQE tests in `CometExecSuite`, a `CometCodegenSuite` test, and two SQL file tests. For Spark's SQL tests, I went through every cross join in the `sql/core` and `sql/hive` tests of Spark 3.5.9 and 4.1.3 with the diffs applied, and compared the cross joins in 3.4.3, 4.0.4 and 4.2.0 against 4.1.3. `SPARK-26138` is the only test whose outcome changes. The others already accept Comet's join classes, are in suites ignored under Comet, or check only results. I haven't run Spark's SQL suites locally. -- 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]
