stantheman0128 opened a new issue, #6233: URL: https://github.com/apache/datafusion-comet/issues/6233
### What is the problem the feature request solves? `CometCollationSuite` holds the collation fallback tests (#1947, #4051, #4646), but it only runs on some profiles. There are two copies, one under `spark/src/test/spark-4.0` and one under `spark/src/test/spark-4.1`, and there is no `spark/src/test/spark-4.2`, so the 4.2 profile runs neither. None of its tests run on 4.2, including the DISTINCT, GROUP BY and ORDER BY fallbacks and the #4646 datetime checks. The two copies have drifted in one place. The 4.1 copy has no #4051 join-key tests (the block at lines 81-247 of the 4.0 copy, plus its imports and the `joinKeyCollationReason` constant); everything else is the same. Spark 4.1 looks like the reason. `BroadcastHashJoinExec` and `ShuffledHashJoinExec` build their keys through `HashJoin.normalizeJoinKeys` there, which wraps a collated key in `CollationKey`, whose type is `BinaryType`. So the exec never receives a collated key and the converter has nothing to reject. `SortMergeJoinExec` does not normalize its keys, so the sort-merge join tests in that block should still hold on 4.1 and later. Meanwhile new collation tests are going into separate suites under `spark-4.x` (`CometCastCollatedStringSuite` in #5302, `CometSortCollationSuite` in #6206). With `CometCollationSuite` in `spark-4.x`, they would have one place to go. ### Describe the potential solution 1. Move the shared part of the 4.0 copy to `spark/src/test/spark-4.x/org/apache/spark/sql/CometCollationSuite.scala` and delete the 4.1 copy. Both would otherwise be test roots on the 4.1 profile (`spark/pom.xml:726-728`), and two classes would share one fully qualified name. 2. Keep the broadcast and shuffled hash join tests where only 4.0 compiles and runs them, with a comment saying why they do not apply on 4.1 and later. Keep the sort-merge join tests on every 4.x profile if they pass there. 3. Run the moved suite on 4.0, 4.1 and 4.2, and fix or document whatever the first 4.2 run turns up. A related question for later: if Spark 4.1 already turns collated hash join keys into binary before Comet sees them, the #4051 guard may not be needed for those joins on 4.1 and later, and Comet could run them natively. That deserves its own look and is not part of this move. ### Additional context Came out of the #5302 review: https://github.com/apache/datafusion-comet/pull/5302#pullrequestreview-5294205378 Line numbers are from `main` at `646ff181`. -- 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]
