fix: address failure caused by method signature change in SPARK-48791 #693
Conversation
| @@ -33,4 +37,24 @@ object ShimBatchReader { | |||
| Array.empty[String], | |||
| 0, | |||
| 0) | |||
|
|
|||
| def getTaskAccumulator(taskMetrics: TaskMetrics): Option[AccumulatorV2[_, _]] = { | |||
There was a problem hiding this comment.
This looks good, but it seems to be the same implementation in all of the shims? Deos it even need to be in the shim layer?
There was a problem hiding this comment.
I could move this into the BatchReader itself. Perhaps that is cleaner.
| import org.apache.spark.sql.catalyst.InternalRow | ||
| import org.apache.spark.sql.execution.datasources.PartitionedFile | ||
| import org.apache.spark.util.AccumulatorV2 | ||
|
|
||
| object ShimBatchReader { |
There was a problem hiding this comment.
nit: the shiimmed method is for getting an accumulator from TaskMetrics and does not seem specific to BatchReader, so maybe it should be in a specific ShimTaskMetric object?
There was a problem hiding this comment.
I believe we tend to put the methods in the class they are used. Anyway, let me move this to BatchReader and avoid the code duplication.
|
@andygrove removed the shims and moved the code into |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #693 +/- ##
============================================
+ Coverage 33.63% 33.69% +0.06%
- Complexity 821 827 +6
============================================
Files 109 109
Lines 42529 42567 +38
Branches 9343 9360 +17
============================================
+ Hits 14304 14343 +39
- Misses 25262 25268 +6
+ Partials 2963 2956 -7 ☔ View full report in Codecov by Sentry. |
…apache#693) * fix: address failure caused by method signature change in SPARK-48791 * fix build * remove shim * add comment
Which issue does this PR close?
Closes #692
Rationale for this change
A private method in TaskMetrics that Comet accesses changed its signature
What changes are included in this PR?
Use reflection to call the method
How are these changes tested?
Local build of Spark with the breaking change and then running HiveParquetSuite