sunchao commented on code in PR #5891:
URL: https://github.com/apache/datafusion-comet/pull/5891#discussion_r4010865700
##########
spark/src/main/scala/org/apache/comet/SparkErrorConverter.scala:
##########
@@ -86,11 +86,10 @@ object SparkErrorConverter extends ShimSparkErrorConverter {
val json = parse(e.getMessage)
val errorJson = json.extract[ErrorJson]
val rawParams = errorJson.params.getOrElse(Map.empty)
- // CannotReadFile carries the offending file path natively only for the
object_store NotFound
- // case; for corrupt/truncated parquet the native error has no path, so
fall back to the
- // per-task file list threaded in from CometExecIterator.
+ // File-read errors without a native path use the per-task file list from
CometExecIterator.
val params =
- if (errorJson.errorType == "CannotReadFile"
+ if ((errorJson.errorType == "CannotReadFile" ||
+ errorJson.errorType == "ParquetTimestampOverflow")
Review Comment:
### Correctness
[P2] Preserve the failing file path for multi-file tasks
Could we carry the active Parquet file's path with
`ParquetTimestampOverflow` instead of filling it from the whole task list?
`CometNativeScanExec` populates that list from every file in the
`FilePartition`, and normal bin packing can put several files in one task. With
one healthy file and one overflowing file, this branch sets Spark's `path`
parameter to `file:///data/good.parquet,file:///data/bad.parquet`, rather than
the single failing file that Spark's `FileScanRDD` supplies. I reproduced this
with the exact current converter and 4.x shim against Spark 4.0.4. The new
native error has no path parameter, so it always needs this fallback. The
existing tests use one file and cannot catch this case. Please preserve the
failing path at the native reader boundary and add a two-file, single-task
regression, including the fused shuffle path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
