viirya commented on code in PR #109:
URL: 
https://github.com/apache/arrow-datafusion-comet/pull/109#discussion_r1505429531


##########
spark/src/test/scala/org/apache/comet/exec/CometColumnarShuffleSuite.scala:
##########
@@ -925,193 +831,64 @@ abstract class CometShuffleSuiteBase extends 
CometTestBase with AdaptiveSparkPla
     }
   }
 
-  test("hash shuffle: Comet shuffle") {
-    // Disable CometExec to explicit test Comet Arrow shuffle path
-    Seq(true, false).foreach { execEnabled =>
-      withSQLConf(
-        CometConf.COMET_EXEC_ENABLED.key -> execEnabled.toString,
-        CometConf.COMET_EXEC_SHUFFLE_ENABLED.key -> "true",
-        CometConf.COMET_COLUMNAR_SHUFFLE_ENABLED.key -> 
(!execEnabled).toString) {
-        withParquetTable((0 until 5).map(i => (i, (i + 1).toLong)), "tbl") {
-          val df = sql("SELECT * FROM tbl").sortWithinPartitions($"_1".desc)
-          val shuffled1 = df.repartition(10, $"_1")
-
-          // If Comet execution is disabled, `Sort` operator is Spark operator
-          // and jvm arrow shuffle is applied.
-          checkCometExchange(shuffled1, 1, execEnabled)
-          checkSparkAnswer(shuffled1)
-
-          val shuffled2 = df.repartition(10, $"_1", $"_2")
-
-          checkCometExchange(shuffled2, 1, execEnabled)
-          checkSparkAnswer(shuffled2)
-
-          val shuffled3 = df.repartition(10, $"_2", $"_1")
-
-          checkCometExchange(shuffled3, 1, execEnabled)
-          checkSparkAnswer(shuffled3)
-        }
-      }
-    }
-  }
-
-  test("Comet shuffle: different data type") {

Review Comment:
   > We have this which tests both columnar and native shuffle
   
   I looked at this test now. I think it is not to test both columnar and 
native shuffle, but only native shuffle. You can see it never enabled columnar 
shuffle, and the feature is disable by default.
   
   When `CometExec` is disabled in this test, I think it goes to test if we can 
still run Spark shuffle without issue, i.e, if we can fallback to original 
Spark shuffle correctly.



-- 
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]

Reply via email to