andygrove commented on code in PR #6607:
URL: https://github.com/apache/datafusion-comet/pull/6607#discussion_r4197877768
##########
spark/src/test/scala/org/apache/spark/sql/comet/execution/shuffle/CometCelebornShufflePlanningSuite.scala:
##########
@@ -265,6 +265,21 @@ class CometCelebornShufflePlanningSuite extends
CometTestBase {
}
}
+ test("the shuffle input conversion leaves a Spark child to Spark's shuffle")
{
+ // The conversion only takes over from the JVM columnar shuffle, which
Celeborn does not
+ // have, and Celeborn's native shuffle needs a native child.
+ withSQLConf(
+ CometConf.COMET_SHUFFLE_MODE.key -> "native",
Review Comment:
Confirmed: with the Celeborn block in `shuffleSupported` removed, the test
still passed in `native`. Running it in `auto` alone wouldn't catch that
either, though. Under Celeborn, `shuffleSupported` first calls
`cometCelebornShuffleFallbackReason` through `isCometShuffleEnabledReason`, and
that already turns down `auto` and `jvm` with "Celeborn-backed Comet shuffle
requires spark.comet.shuffle.mode=native". The `auto` version passed with the
block removed, and failed only once both checks were gone.
c90dcfca5 runs the test in `auto`, `jvm` and `native`, and checks that the
fallback reason names Celeborn, which in `native` only the block gives; without
it the reason is "Comet columnar shuffle not enabled". The test now fails if
the block is removed, if both Celeborn checks are removed, since `auto` then
converts, or if the gate stops requiring `CometColumnarShuffle`. Removing only
the mode checks in `cometCelebornShuffleFallbackReason` fails eight other tests
in the suite.
--
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]
