upgraded spark 3.5.4 to 3.5.5 #1565
Conversation
parthchandra
left a comment
There was a problem hiding this comment.
lgtm.
Did we really need to regenerate the diff file?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1565 +/- ##
============================================
+ Coverage 56.12% 58.40% +2.27%
- Complexity 976 985 +9
============================================
Files 119 122 +3
Lines 11743 12214 +471
Branches 2251 2280 +29
============================================
+ Hits 6591 7133 +542
+ Misses 4012 3949 -63
+ Partials 1140 1132 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@parthchandra I regenerated the diff file per the instructions in https://github.com/apache/datafusion-comet/blob/main/docs/source/contributor-guide/spark-sql-tests.md - |
| maxSplitBytes: Long, | ||
| partitionValues: InternalRow): Seq[PartitionedFile] = | ||
| PartitionedFileUtil.splitFiles(sparkSession, file, isSplitable, maxSplitBytes, partitionValues) | ||
| PartitionedFileUtil.splitFiles(sparkSession, file, filePath, isSplitable, maxSplitBytes, partitionValues) |
There was a problem hiding this comment.
This change seems incompatible with versions below 3.5.4 of 3.5.X. Should we try to maintain compatibility among minor versions?
There was a problem hiding this comment.
That would be possible using minor-version-specific shims, but it will change the release structure -
which assumed a single release for each spark major version.
What do the maintainers think?
If not, I think the latest minor should be preferred.
There was a problem hiding this comment.
We may be able to simply make them compatible through reflection
There was a problem hiding this comment.
That would be certainly much easier from a release-engineering perspective (it's also not big of a development effort) -
but the question is whether this is required.
There was a problem hiding this comment.
I've tried using reflection in #1573 (also adding 3.5.4 to the testing matrix) -
Let's see how it fares.
## Which issue does this PR close? Closes apache#1461 ## Rationale for this change Spark 3.5.5 is the latest stable 3.5.x version, and should be supported. ## What changes are included in this PR? Just the upgrade of spark 3.5.4 to 3.5.5 and the only code change required in `ShimCometScanExec`. ## How are these changes tested? Ran `mvn test` with the `spark-3.5` profile. This was sufficient since the build failed with just the upgrade and without the required code change.
Which issue does this PR close?
Closes #1461
Rationale for this change
Spark 3.5.5 is the latest stable 3.5.x version, and should be supported.
What changes are included in this PR?
Just the upgrade of spark 3.5.4 to 3.5.5 and the only code change required in
ShimCometScanExec.How are these changes tested?
Ran
mvn testwith thespark-3.5profile.This was sufficient since the build failed with just the upgrade and without the required code change.