comphead commented on code in PR #7228:
URL: https://github.com/apache/datafusion-comet/pull/7228#discussion_r4379459359
##########
native/core/src/parquet/file_error_context.rs:
##########
@@ -299,4 +314,67 @@ mod tests {
assert!(matches!(error, DataFusionError::External(source)
if matches!(source.downcast_ref::<SparkError>(),
Some(SparkError::DivideByZero))));
}
+
+ fn cannot_read_path(error: &DataFusionError) -> Option<&str> {
+ let DataFusionError::External(source) = error else {
+ return None;
+ };
+ match source.downcast_ref::<SparkError>() {
+ Some(SparkError::CannotReadFile { file_path, .. }) =>
Some(file_path),
+ _ => None,
+ }
+ }
+
+ #[test]
+ fn read_failures_name_the_failed_file() {
Review Comment:
The case this fixes is a partition with more than one file. With one file
per partition, the JVM fallback in `SparkErrorConverter` already names the
right file, because it joins the task's `filePath`s with commas. The existing
end-to-end tests are all that shape: `CometExecSuite`'s "native parquet read
failure surfaces as FAILED_READ_FILE with the file path" reads a single file,
and `CometTopKSuite`'s corrupt footer and page tests assert one file per
partition. A variant of the TopK test with both files in one partition
(`LEAF_NODE_DEFAULT_PARALLELISM` 1 and a large `FILES_MAX_PARTITION_BYTES`)
that asserts the structured `filePath` equals only the corrupt file would fail
on main with both paths and pass here. It would also be the JVM check the
description asks for. I have not run it.
--
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]
