LuciferYang opened a new pull request, #9579:
URL: https://github.com/apache/paimon/pull/9579

   ### Purpose
   
   close #9578
   
   `AbstractTextFileReader`'s constructor opens the file, wraps it for 
decompression and then builds the line reader. Nothing else holds the stream 
until that last assignment, so if either step throws, the constructor never 
returns and the stream is lost.
   
   The reachable case is an unreadable compressed file. The wrapper is chosen 
from the path suffix, so a compressed path such as `.gz` or `.zst` goes through 
a Hadoop codec, `StandardLineReader` reads in its own constructor, and a bad 
header fails there. That is a plain read with the default delimiter and no 
offset, and it loses the descriptor plus the decompressor the codec borrowed 
from `CodecPool`. It matters most under `scan.ignore-corrupt-files`, where the 
caller swallows the failure and continues to the next file.
   
   Closing the decompression wrapper closes the stream underneath it, and when 
the file is not compressed the wrapper is that same stream, so one 
`closeQuietly` covers both cases rather than one close per layer.
   
   `TextLineReader.create` also throws for a custom line delimiter combined 
with an offset, which the tests cover second. That path is defensive rather 
than reachable: `SplitEnumerator.preferToSplitFile` only splits CSV and JSON 
files that use the default delimiter and are not compressed, so no `FileMeta` 
with an offset is ever produced for one.
   
   ### Tests
   
   `TextReaderCtorLeakTest` wraps `LocalFileIO` so each stream counts its own 
`close()` calls, and asserts exactly one close rather than just "was closed": a 
corrupt `.gz`, the custom-delimiter-with-offset case, and a successful read. 
The two failure cases fail against the unfixed constructor. The success case 
passes there too, which is the point of it: it pins the single close, which a 
per-layer close would turn into two.
   
   `mvn -pl paimon-format test` on JDK 8: 599 tests, 0 failures. 
`spotless:check` and `checkstyle:check` are clean.
   
   One related gap this does not cover: `CsvFileReader` builds its `CsvParser` 
after `super(...)` returns, and `createProjectionMapping` throws 
`IllegalArgumentException` when a projected field is missing. The line reader 
exists by then, so that path leaks too, but fixing it belongs in the subclass 
rather than in this constructor.
   


-- 
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]

Reply via email to