build: Add scala-version to matrix (#396)

* build: Add scala-version to matrix Add testing of Scala 2.13 along with 2.12 by adding `scala-version` to the matrix. Explicitly exclude combinations of Spark `3.2` and Scala `2.13`. * Update pr_build.yml * chore: Fixes for scalafix --------- Co-authored-by: Steve Vaughan <[email protected]>
apache · May 8, 2024 · ee86b0e · ee86b0e
1 parent 9df0236
commit ee86b0e
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 32 deletions.
diff --git a/.github/workflows/pr_build.yml b/.github/workflows/pr_build.yml
@@ -47,13 +47,14 @@ jobs:
         java_version: [8, 11, 17]
         test-target: [rust, java]
         spark-version: ['3.4']
+        scala-version: ['2.12', '2.13']
         is_push_event:
           - ${{ github.event_name == 'push' }}
         exclude: # exclude java 11 for pull_request event
           - java_version: 11
             is_push_event: false
       fail-fast: false
-    name: ${{ matrix.os }}/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}/${{ matrix.test-target }}
+    name: ${{ matrix.os }}/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}-scala-${{matrix.scala-version}}/${{ matrix.test-target }}
     runs-on: ${{ matrix.os }}
     container:
       image: amd64/rust
@@ -71,7 +72,7 @@ jobs:
         name: Java test steps
         uses: ./.github/actions/java-test
         with:
-          maven_opts: -Pspark-${{ matrix.spark-version }}
+          maven_opts: -Pspark-${{ matrix.spark-version }},scala-${{ matrix.scala-version }}
           # upload test reports only for java 17
           upload-test-reports: ${{ matrix.java_version == '17' }}
 
@@ -82,13 +83,16 @@ jobs:
         java_version: [8, 11, 17]
         test-target: [java]
         spark-version: ['3.2', '3.3']
+        scala-version: ['2.12', '2.13']
         exclude:
           - java_version: 17
             spark-version: '3.2'
           - java_version: 11
             spark-version: '3.2'
+          - spark-version: '3.2'
+            scala-version: '2.13'
       fail-fast: false
-    name: ${{ matrix.os }}/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}/${{ matrix.test-target }}
+    name: ${{ matrix.os }}/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}-scala-${{matrix.scala-version}}/${{ matrix.test-target }}
     runs-on: ${{ matrix.os }}
     container:
       image: amd64/rust
@@ -102,7 +106,7 @@ jobs:
       - name: Java test steps
         uses: ./.github/actions/java-test
         with:
-          maven_opts: -Pspark-${{ matrix.spark-version }}
+          maven_opts: -Pspark-${{ matrix.spark-version }},scala-${{ matrix.scala-version }}
 
   macos-test:
     strategy:
@@ -111,9 +115,10 @@ jobs:
         java_version: [8, 11, 17]
         test-target: [rust, java]
         spark-version: ['3.4']
+        scala-version: ['2.12', '2.13']
       fail-fast: false
     if: github.event_name == 'push'
-    name: ${{ matrix.os }}/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}/${{ matrix.test-target }}
+    name: ${{ matrix.os }}/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}-scala-${{matrix.scala-version}}/${{ matrix.test-target }}
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
@@ -129,21 +134,22 @@ jobs:
         name: Java test steps
         uses: ./.github/actions/java-test
         with:
-          maven_opts: -Pspark-${{ matrix.spark-version }}
+          maven_opts: -Pspark-${{ matrix.spark-version }},scala-${{ matrix.scala-version }}
 
   macos-aarch64-test:
     strategy:
       matrix:
         java_version: [8, 11, 17]
         test-target: [rust, java]
         spark-version: ['3.4']
+        scala-version: ['2.12', '2.13']
         is_push_event:
           - ${{ github.event_name == 'push' }}
         exclude: # exclude java 11 for pull_request event
           - java_version: 11
             is_push_event: false
       fail-fast: false
-    name: macos-14(Silicon)/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}/${{ matrix.test-target }}
+    name: macos-14(Silicon)/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}-scala-${{matrix.scala-version}}/${{ matrix.test-target }}
     runs-on: macos-14
     steps:
       - uses: actions/checkout@v4
@@ -161,21 +167,24 @@ jobs:
         name: Java test steps
         uses: ./.github/actions/java-test
         with:
-          maven_opts: -Pspark-${{ matrix.spark-version }}
+          maven_opts: -Pspark-${{ matrix.spark-version }},scala-${{ matrix.scala-version }}
 
   macos-aarch64-test-with-old-spark:
     strategy:
       matrix:
         java_version: [8, 17]
         test-target: [java]
         spark-version: ['3.2', '3.3']
+        scala-version: ['2.12', '2.13']
         exclude:
           - java_version: 17
             spark-version: '3.2'
           - java_version: 8
             spark-version: '3.3'
+          - spark-version: '3.2'
+            scala-version: '2.13'
       fail-fast: false
-    name: macos-14(Silicon)/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}/${{ matrix.test-target }}
+    name: macos-14(Silicon)/java ${{ matrix.java_version }}-spark-${{matrix.spark-version}}-scala-${{matrix.scala-version}}/${{ matrix.test-target }}
     runs-on: macos-14
     steps:
       - uses: actions/checkout@v4
@@ -190,5 +199,5 @@ jobs:
         name: Java test steps
         uses: ./.github/actions/java-test
         with:
-          maven_opts: -Pspark-${{ matrix.spark-version }}
+          maven_opts: -Pspark-${{ matrix.spark-version }},scala-${{ matrix.scala-version }}
 
diff --git a/...rc/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala b/...rc/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala
@@ -334,7 +334,7 @@ object CometShuffleExchangeExec extends ShimCometShuffleExchangeExec {
         // seed by hashing will help. Refer to SPARK-21782 for more details.
         val partitionId = TaskContext.get().partitionId()
         var position = new XORShiftRandom(partitionId).nextInt(numPartitions)
-        (row: InternalRow) => {
+        (_: InternalRow) => {
           // The HashPartitioner will handle the `mod` by the number of partitions
           position += 1
           position

diff --git a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala b/spark/src/test/scala/org/apache/comet/CometCastSuite.scala
@@ -867,26 +867,27 @@ class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper {
     }
   }
 
-  private def castFallbackTestTimezone(
-      input: DataFrame,
-      toType: DataType,
-      expectedMessage: String): Unit = {
-    withTempPath { dir =>
-      val data = roundtripParquet(input, dir).coalesce(1)
-      data.createOrReplaceTempView("t")
-
-      withSQLConf(
-        (SQLConf.ANSI_ENABLED.key, "false"),
-        (CometConf.COMET_CAST_ALLOW_INCOMPATIBLE.key, "true"),
-        (SQLConf.SESSION_LOCAL_TIMEZONE.key, "America/Los_Angeles")) {
-        val df = data.withColumn("converted", col("a").cast(toType))
-        df.collect()
-        val str =
-          new ExtendedExplainInfo().generateExtendedInfo(df.queryExecution.executedPlan)
-        assert(str.contains(expectedMessage))
-      }
-    }
-  }
+  // TODO Commented out to work around scalafix since this is currently unused.
+  // private def castFallbackTestTimezone(
+  //     input: DataFrame,
+  //     toType: DataType,
+  //     expectedMessage: String): Unit = {
+  //   withTempPath { dir =>
+  //     val data = roundtripParquet(input, dir).coalesce(1)
+  //     data.createOrReplaceTempView("t")
+  //
+  //     withSQLConf(
+  //       (SQLConf.ANSI_ENABLED.key, "false"),
+  //       (CometConf.COMET_CAST_ALLOW_INCOMPATIBLE.key, "true"),
+  //       (SQLConf.SESSION_LOCAL_TIMEZONE.key, "America/Los_Angeles")) {
+  //       val df = data.withColumn("converted", col("a").cast(toType))
+  //       df.collect()
+  //       val str =
+  //         new ExtendedExplainInfo().generateExtendedInfo(df.queryExecution.executedPlan)
+  //       assert(str.contains(expectedMessage))
+  //     }
+  //   }
+  // }
 
   private def castTimestampTest(input: DataFrame, toType: DataType) = {
     withTempPath { dir =>

diff --git a/spark/src/test/scala/org/apache/comet/exec/CometColumnarShuffleSuite.scala b/spark/src/test/scala/org/apache/comet/exec/CometColumnarShuffleSuite.scala
@@ -587,7 +587,7 @@ abstract class CometColumnarShuffleSuite extends CometTestBase with AdaptiveSpar
   }
 
   test("columnar shuffle on nested array") {
-    Seq("false", "true").foreach { execEnabled =>
+    Seq("false", "true").foreach { _ =>
       Seq(10, 201).foreach { numPartitions =>
         Seq("1.0", "10.0").foreach { ratio =>
           withSQLConf(CometConf.COMET_SHUFFLE_PREFER_DICTIONARY_RATIO.key -> ratio) {