parthchandra commented on code in PR #1862: URL: https://github.com/apache/datafusion-comet/pull/1862#discussion_r2146236444
########## spark/src/test/scala/org/apache/comet/exec/CometNativeShuffleSuite.scala: ########## @@ -120,29 +120,51 @@ class CometNativeShuffleSuite extends CometTestBase with AdaptiveSparkPlanHelper } } - test("native operator after native shuffle") { - withParquetTable((0 until 5).map(i => (i, (i + 1).toLong)), "tbl") { - val df = sql("SELECT * FROM tbl") - - val shuffled1 = df - .repartition(10, $"_2") - .select($"_1", $"_1" + 1, $"_2" + 2) - .repartition(10, $"_1") - .filter($"_1" > 1) - - // 2 Comet shuffle exchanges are expected - checkShuffleAnswer(shuffled1, 2) - - val shuffled2 = df - .repartitionByRange(10, $"_2") - .select($"_1", $"_1" + 1, $"_2" + 2) - .repartition(10, $"_1") - .filter($"_1" > 1) - - // Because the first exchange from the bottom is range exchange which native shuffle - // doesn't support. So Comet exec operators stop before the first exchange and thus - // there is no Comet exchange. - checkShuffleAnswer(shuffled2, 0) + test("native operator after native shuffle with hash partitioning") { + Seq("true", "false").foreach { hashPartitioningEnabled => + withSQLConf( + CometConf.COMET_EXEC_SHUFFLE_WITH_HASH_PARTITIONING_ENABLED.key -> hashPartitioningEnabled) { Review Comment: We could probably merge these two tests ? ``` Seq("true", "false").foreach { partitioningEnabled => Seq(CometConf.COMET_EXEC_SHUFFLE_WITH_HASH_PARTITIONING_ENABLED, CometConf.COMET_EXEC_SHUFFLE_WITH_RANGE_PARTITIONING_ENABLED) { partitioningType => withSQLConf( partittioningType.key -> partitioningEnabled) ``` ########## common/src/main/scala/org/apache/comet/CometConf.scala: ########## @@ -307,6 +307,18 @@ object CometConf extends ShimCometConf { .booleanConf .createWithDefault(false) + val COMET_EXEC_SHUFFLE_WITH_HASH_PARTITIONING_ENABLED: ConfigEntry[Boolean] = Review Comment: Can a user have both configs enabled? What happens? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org