test: Enable Spark 4.0 tests#537
Conversation
spark/src/main/spark-3.2/org/apache/comet/shims/CometExprShim.scala
Outdated
Show resolved
Hide resolved
spark/src/main/spark-3.3/org/apache/comet/shims/CometExprShim.scala
Outdated
Show resolved
Hide resolved
spark/src/main/spark-3.2/org/apache/comet/shims/CometExprShim.scala
Outdated
Show resolved
Hide resolved
spark/src/main/spark-3.3/org/apache/comet/shims/CometExprShim.scala
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #537 +/- ##
============================================
+ Coverage 54.68% 54.70% +0.01%
Complexity 795 795
============================================
Files 102 103 +1
Lines 9688 9707 +19
Branches 1845 1849 +4
============================================
+ Hits 5298 5310 +12
- Misses 3433 3444 +11
+ Partials 957 953 -4 ☔ View full report in Codecov by Sentry. |
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala
Outdated
Show resolved
Hide resolved
| case _: TimestampType => 9 | ||
| case _: DecimalType => 10 | ||
| case dt if dt.typeName == "timestamp_ntz" => 11 | ||
| case dt if isTimestampNTZType(dt) => 11 |
|
I didn't review the Spark diff, but the other changes LGTM from a first pass. I will review more carefully tomorrow. |
|
@andygrove just checking in to see if you have more feedback |
|
Friendly ping @andygrove @comphead @huaxingao @parthchandra @viirya |
andygrove
left a comment
andygrove
left a comment
There was a problem hiding this comment.
LGTM. Thanks @kazuyukitanimura
|
Waiting for #581 gets merged first |
comphead
left a comment
comphead
left a comment
There was a problem hiding this comment.
lgtm, thanks @kazuyukitanimura 👍
| java-version: [11] | ||
| spark-version: [{short: '3.4', full: '3.4.2'}] | ||
| java-version: [17] | ||
| spark-version: [{short: '4.0', full: '4.0.0-preview1'}] |
There was a problem hiding this comment.
Is this particular for ansi test? Hmm, is there any difference between 3.4 and 4.0?
There was a problem hiding this comment.
This has been disabled. List of failing tests #551
There was a problem hiding this comment.
Hmm, before we can enable it, can we still use 3.4 to run these tests?
There was a problem hiding this comment.
I have not tried running this with 3.4, but based on the comment, it likely fails.
There was a problem hiding this comment.
Ah, I forgot we don't run ansi test in CI.
|
Merged thank you @andygrove @comphead @viirya |
## Rationale for this change To be ready for Spark 4.0 ## What changes are included in this PR? This PR enables the spark-4.0 tests with comet enabled except for the ones listed in apache#551 ## How are these changes tested? ANSI is enabled for Spark-4.0
5 participants
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 enables the spark-4.0 tests with comet enabled except for the ones listed in #551
How are these changes tested?
ANSI is enabled for Spark-4.0