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

Reply via email to