sankalpsthakur opened a new pull request, #3875: URL: https://github.com/apache/parquet-java/pull/3875
### Rationale for this change Files written by parquet-mr up to 1.2.8 do not count the dictionary page header in the size of a column chunk (PARQUET-816), so the end of the last page of such a chunk is not in the buffer that is read for it. ParquetFileReader has a workaround for that, but it only half works. On master (c972bd04), `ChunkListBuilder.build` (ParquetFileReader.java:2059) uses `WorkaroundChunk` only for the very last chunk that was read, and `WorkaroundChunk.readAsBytesInput` (:2361) reads the missing bytes from the file stream at its current position, assuming the stream still sits at the end of that chunk. All part lists are read first and decoded afterwards, and a vectored read never moves the stream, so that assumption does not hold. The bytes that are read are also never flipped, so they are dropped even when the position happens to be right. For nation.dict-malformed.parquet from apache/parquet-testing (the file in #3336) this means: reading all columns fails with an EOFException, and reading only `name` or only `comment_col` does not fail but returns wrong values, e.g. row 8 reads ALGERIA instead of INDIA. ### What changes are included in this PR? - Every chunk that ends a consecutive part list becomes a `WorkaroundChunk` that knows where its part list ends, in the sequential and in the vectored read. It seeks there before reading the missing bytes instead of trusting the stream position. - The bytes read that way are flipped before they are used. - The completion is limited to 100 bytes, the same bound the C++ reader uses for this bug, so a really truncated chunk still ends in an EOFException. Healthy files take the same path as before: the workaround only runs where a page would otherwise run past the end of its chunk. ### Are these changes tested? The new `TestOldWriterDictionaryChunks` reads a copy of nation.dict-malformed.parquet (2.8 KB, identical to the apache/parquet-testing file) with vectored IO on and off, for all columns, for `name` only and for `comment_col` only, and checks all 25 values of both string columns. The file is in the test resources instead of being downloaded, because of the network problems mentioned in #3670. I ran it on GitHub Actions in my fork with Hadoop 3.3.0 (what the build pins, no vectored IO) and with 3.4.2 (vectored IO): it passes with this change and fails with master's ParquetFileReader on both. With 3.3.0 the whole parquet-hadoop module passes (833 tests, 0 failures; the 24 skipped are TestVectorIoBridge and TestFileRangeBridge, which skip on 3.3.0 because it has no vectored API). Run: https://github.com/sankalpsthakur/parquet-java/actions/runs/39575535093 ### Are there any user-facing changes? Files from old parquet-mr writers with dictionary encoded columns can be read with the default settings again. Nothing changes for other files. This does not cover the second file in the issue, fixed_length_byte_array.parquet. pyarrow cannot read it either ("Unexpected end of stream"), and a comment on the issue points to nulls in a required field (apache/arrow#47662), so it looks like a different problem and the issue stays open for it. Part of #3336 AI disclosure: Claude Code helped write the fix and the test; I reviewed them and checked the CI results before submitting. -- 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]
