build: Add spark-4.0 profile and shims #407
Conversation
|
@viirya @andygrove Please approve to start CI |
|
Triggered. |
|
@viirya @andygrove Is there a way to start CI without bothering you? |
|
I remember only the first-time contributors need approval to trigger CI. |
kazuyukitanimura
force-pushed
the
spark-4.0
branch
from
ae59e5b
to
d629df1
Compare
|
@viirya @andygrove passed all the tests on my personal github actions |
|
Triggered CI pipelines. |
common/src/main/spark-4.0/org/apache/comet/shims/ShimFileFormat.scala
Outdated
Show resolved
Hide resolved
common/src/main/spark-4.0/org/apache/spark/sql/comet/shims/ShimCometParquetUtils.scala
Outdated
Show resolved
Hide resolved
spark/src/main/scala/org/apache/spark/sql/comet/DecimalPrecision.scala
Outdated
Show resolved
Hide resolved
...k/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala
Outdated
Show resolved
Hide resolved
| override def write( | ||
| rdd: RDD[_], | ||
| inputs: Iterator[_], |
There was a problem hiding this comment.
If the input iterator has partitionId, we still need to get rid of it. I don't run this but looks like this will cause failures.
There was a problem hiding this comment.
We have ShimCometShuffleWriteProcessor.write() for Spark 3.x
There was a problem hiding this comment.
Yea, I mean for Spark 4.0 case, we also need to get rid of the partition. I saw you replied my other comment: #407 (comment)
There was a problem hiding this comment.
I implemented the partitionId removal.
Thanks @viirya
...k/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala
Outdated
Show resolved
Hide resolved
|
@viirya Please take another look |
.github/workflows/pr_build.yml
Outdated
| matrix: | ||
| os: [ubuntu-latest] | ||
| java_version: [17] | ||
| test-target: [rust, java] |
There was a problem hiding this comment.
Is it necessary to run rust test for Spark 4.0 separately? It should be no difference with 3.4 or 3.3/3.2.
We also don't run rust test for Spark 3.3/3.2 but only for Spark 3.4.
.github/workflows/pr_build.yml
Outdated
| - if: matrix.test-target == 'java' | ||
| name: Install Spark | ||
| shell: bash | ||
| working-directory: ./apache-spark | ||
| run: build/mvn install -Phive -Phadoop-cloud -DskipTests |
There was a problem hiding this comment.
This is only needed for Spark 4.0? I don't see we install it for other Spark versions.
There was a problem hiding this comment.
Yes, This is only needed for Spark 4.0 because there is no 4.0.0-SNAPSHOT jar publicly available
| /** | ||
| * Returns a tuple of expressions for the `unhex` function. | ||
| */ |
There was a problem hiding this comment.
| /** | |
| * Returns a tuple of expressions for the `unhex` function. | |
| */ | |
| /** | |
| * Returns a tuple of expressions for the `unhex` function. | |
| */ |
| @@ -29,7 +29,7 @@ import org.apache.comet.CometConf | |||
| /** | |||
| * This test suite contains tests for only Spark 3.4. | |||
There was a problem hiding this comment.
nit:
| * This test suite contains tests for only Spark 3.4. | |
| * This test suite contains tests for only Spark 3.4+. |
|
@kazuyukitanimura I think you can try to merge this. |
|
Thank you @viirya merged |
This PR adds the spark-4.0 profile and shims This is an initial commit. Tests with the spark-4.0 profile do not pass yet. Tests for spark-3.x should pass. (cherry picked from commit 9b3e87b)
Which issue does this PR close?
Part of #372
Rationale for this change
To be ready for Spark 4.0
What changes are included in this PR?
This PR adds the spark-4.0 profile and shims
How are these changes tested?
This is an initial commit. Tests with the spark-4.0 profile do not pass yet. Tests for spark-3.x should pass.