COLDMANYU commented on code in PR #10716:
URL: https://github.com/apache/paimon/pull/10716#discussion_r4373645420
##########
paimon-core/src/main/java/org/apache/paimon/io/DataFileRecordReader.java:
##########
@@ -256,4 +270,76 @@ private static boolean ignoreCorruptException(Throwable e,
boolean ignoreCorrupt
|| e instanceof RuntimeException
|| e instanceof InternalError);
}
+
+ /**
+ * Row-based formats (CSV, JSON, TEXT, Avro, ...) decode lazily in {@link
#next()}, so a file
+ * that is truncated or corrupt in the middle only fails after {@link
#readBatch()} returned.
+ * This iterator extends {@code scan.ignore-corrupt-files} to that point:
the rows already
+ * returned are kept and the rest of the file is skipped.
+ */
+ private class IgnoreCorruptFileRecordIterator implements
FileRecordIterator<InternalRow> {
+
+ private final FileRecordIterator<InternalRow> iterator;
+
+ private
IgnoreCorruptFileRecordIterator(FileRecordIterator<InternalRow> iterator) {
+ this.iterator = iterator;
+ }
+
+ @Nullable
+ @Override
+ public InternalRow next() throws IOException {
+ if (corrupted) {
+ return null;
+ }
+ try {
+ return iterator.next();
+ } catch (Exception e) {
+ return handleCorruptException(e, null);
Review Comment:
Good catch, thanks! `InternalError` was never reachable there: every catch
site only caught `Exception`, so the `InternalError` branch of
`ignoreCorruptException` was dead code. This was already true of the two
existing sites in `createReader()` and `readBatch()`, and I copied the same
pattern into the new ones.
I've fixed it in all of them:
- `DataFileRecordReader`: `createReader()`, `readBatch()`, and the
iterator's `next()` and `skip()` now catch `Exception | InternalError`.
- `FormatReadBuilder`: the reader creation catch does the same.
`InternalError` is only caught so that it can be ignored. When
`scan.ignore-corrupt-files` is disabled, it is rethrown unchanged, so the
default behavior stays the same. Other `Error`s, such as `OutOfMemoryError`,
are still not caught.
This also matches Spark. Since SPARK-52482 (4.1.0), `FileScanRDD` uses
`DataSourceUtils.shouldIgnoreCorruptFileException`, which treats
`RuntimeException | IOException | InternalError` as corruption. That change was
made for the same reason: a corrupt zstd header throws
`java.lang.InternalError: Unknown frame descriptor`.
I added `testZipErrorWhileReading` and `testZipErrorAtOpen` (CSV and JSON).
They use a `FileIO` that throws `ZipError` either while the file is being read
or when it is opened. Without the option, the `ZipError` propagates. With the
option, the corrupt file is skipped and the intact file is still read
completely. Both tests fail without this change.
--
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]
