anuragmantri commented on code in PR #18486:
URL: https://github.com/apache/iceberg/pull/18486#discussion_r4297558202
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/source/BaseReader.java:
##########
@@ -148,12 +148,8 @@ public boolean next() throws IOException {
}
}
} catch (IOException | RuntimeException e) {
- if (currentTask != null && !currentTask.isDataTask()) {
- String filePaths =
- referencedFiles(currentTask)
- .map(ContentFile::location)
- .collect(Collectors.joining(", "));
- LOG.error("Error reading file(s): {}", filePaths, e);
+ if (currentTask instanceof ContentScanTask<?> contentTask &&
!currentTask.isDataTask()) {
+ LOG.error("Error reading file: {}", contentTask.file().location(), e);
Review Comment:
Before this change a read failure logged every file the task touched, so for
a `FileScanTask` you'd see the data file and its delete files. Now only
`task.file()` shows up. That last one has no path in its message, and it's
exactly the case this encryption work targets. Could we keep logging the delete
files too? Keeping `referencedFiles(TaskT)` in `BaseReader` would let this
block stay as it was. I added more detail in the top-level comment.
--
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]
