Merge branch 'explain-fallback-reasons' of github.com:andygrove/arrow…

…-datafusion-comet into explain-fallback-reasons
apache · May 13, 2024 · 0dee9c7 · 0dee9c7
2 parents bf87ff7 + b8566fe
commit 0dee9c7
Show file tree

Hide file tree

Showing 31 changed files with 2,407 additions and 294 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/common/pom.xml b/common/pom.xml
@@ -179,7 +179,8 @@ under the License.
             </goals>
             <configuration>
               <sources>
-                <source>src/main/${shims.source}</source>
+                <source>src/main/${shims.majorVerSrc}</source>
+                <source>src/main/${shims.minorVerSrc}</source>
               </sources>
             </configuration>
           </execution>

diff --git a/common/src/main/scala/org/apache/comet/CometConf.scala b/common/src/main/scala/org/apache/comet/CometConf.scala
@@ -140,14 +140,14 @@ object CometConf {
       .booleanConf
       .createWithDefault(false)
 
-  val COMET_COLUMNAR_SHUFFLE_ENABLED: ConfigEntry[Boolean] = conf(
-    "spark.comet.columnar.shuffle.enabled")
-    .doc(
-      "Force Comet to only use columnar shuffle for CometScan and Spark regular operators. " +
-        "If this is enabled, Comet native shuffle will not be enabled but only Arrow shuffle. " +
-        "By default, this config is false.")
-    .booleanConf
-    .createWithDefault(false)
+  val COMET_COLUMNAR_SHUFFLE_ENABLED: ConfigEntry[Boolean] =
+    conf("spark.comet.columnar.shuffle.enabled")
+      .doc(
+        "Whether to enable Arrow-based columnar shuffle for Comet and Spark regular operators. " +
+          "If this is enabled, Comet prefers columnar shuffle than native shuffle. " +
+          "By default, this config is true.")
+      .booleanConf
+      .createWithDefault(true)
 
   val COMET_SHUFFLE_ENFORCE_MODE_ENABLED: ConfigEntry[Boolean] =
     conf("spark.comet.shuffle.enforceMode.enabled")

diff --git a/common/src/main/scala/org/apache/comet/vector/NativeUtil.scala b/common/src/main/scala/org/apache/comet/vector/NativeUtil.scala
@@ -66,7 +66,7 @@ class NativeUtil {
           val arrowArray = ArrowArray.allocateNew(allocator)
           Data.exportVector(
             allocator,
-            getFieldVector(valueVector),
+            getFieldVector(valueVector, "export"),
             provider,
             arrowArray,
             arrowSchema)

diff --git a/common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala b/common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala
@@ -242,7 +242,7 @@ object Utils {
             }
           }
 
-          getFieldVector(valueVector)
+          getFieldVector(valueVector, "serialize")
 
         case c =>
           throw new SparkException(
@@ -253,14 +253,15 @@ object Utils {
     (fieldVectors, provider)
   }
 
-  def getFieldVector(valueVector: ValueVector): FieldVector = {
+  def getFieldVector(valueVector: ValueVector, reason: String): FieldVector = {
     valueVector match {
       case v @ (_: BitVector | _: TinyIntVector | _: SmallIntVector | _: IntVector |
           _: BigIntVector | _: Float4Vector | _: Float8Vector | _: VarCharVector |
           _: DecimalVector | _: DateDayVector | _: TimeStampMicroTZVector | _: VarBinaryVector |
           _: FixedSizeBinaryVector | _: TimeStampMicroVector) =>
         v.asInstanceOf[FieldVector]
-      case _ => throw new SparkException(s"Unsupported Arrow Vector: ${valueVector.getClass}")
+      case _ =>
+        throw new SparkException(s"Unsupported Arrow Vector for $reason: ${valueVector.getClass}")
     }
   }
 }