LuciferYang opened a new pull request, #9576:
URL: https://github.com/apache/paimon/pull/9576
### Purpose
close #9575
`ParquetReaderFactory.createReader` closes what it opened if reading the
footer or constructing the `ParquetFileReader` throws. The stretch after that
was not covered: building the shredding read plan, resolving the requested
schema, checking the batch size, allocating the writable vectors and
constructing `VectorizedParquetRecordReader` can all throw, and at that point
the reader is only a local variable, so the caller cannot close it and the open
stream is lost. This wraps that stretch too, with the same `catch (Throwable)`
shape as the block directly above it, closing `reader` instead of `inputStream`.
Two notes on reading the diff. Ignoring whitespace it is 10 added lines, one
comment plus `try {` plus the eight-line catch; the rest of the churn is the
indentation of the wrapped block, so `git diff -w` is the useful view. And the
two catches are mutually exclusive, so nothing is closed twice.
What makes it worth fixing is that `DataFileRecordReader` treats an
`IOException` or `RuntimeException` from `createReader` as a corrupt file when
`scan.ignore-corrupt-files` is on: it logs a WARN, returns null and the scan
continues, so each unreadable file costs one descriptor and nothing says so.
Reaching it does not need a damaged file. A column whose stored type does not
match the read type throws `Schema evolution not supported.` from
`VectorizedParquetRecordReader`'s constructor, and a case-insensitive read of
two columns differing only in case throws `Found duplicate field(s)` from the
requested-schema build.
### Tests
`ParquetReaderFactoryLeakTest` overrides `computeBatchSize` to return zero,
which fails the positive-batch-size check inside the newly protected stretch.
That is a deterministic trigger rather than a schema quirk, and it uses the
method's documented extension point: the javadoc says subclasses may override
it for per-file batch sizing, and the `checkArgument` exists to reject a
non-positive result from exactly that.
The `FileIO` counts `close()` calls per stream rather than recording a
boolean, so the second test pins a successful read at exactly one close. That
one passes against the unfixed code as well, which is the point of it: it
guards against a careless unconditional close. The leak test fails against the
unfixed code on the close count.
`mvn -pl paimon-format clean test` on JDK 8: 598 tests, 0 failures.
`spotless:check` and `checkstyle:check` are clean.
No overlap with #9550, which touches `clipParquetType` and
`ParquetSchemaConverter` rather than `createReader`.
--
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]
