viirya commented on PR #59118:
URL: https://github.com/apache/spark/pull/59118#issuecomment-5885471919
Thanks @dongjoon-hyun and @HyukjinKwon. Updated in fb12cc34738:
- For columns without definition levels (max definition level 0), the page
value count is now checked exactly, for both v1 and v2 pages, since every row
has a value there. For columns with definition levels it stays an upper bound.
I didn't use `num_values - num_nulls` of `DataPageV2`: neither parquet-java's
read path nor Spark reads `num_nulls` today, so if a writer gets it wrong, its
files read correctly now and enforcing it would start rejecting them. For
nested columns, whether empty lists and null parents are counted in `num_nulls`
also depends on the writer. The code comment and the PR description now say
that a page with definition levels that is off by whole values is not detected,
same as parquet-java.
- Fixed the comment on the page length check.
- Tests:
- `ParquetEncodingSuite`: a round trip with v1 and v2 pages, many pages
per column, and required, optional and array columns of all BYTE_STREAM_SPLIT
types.
- `ParquetColumnIndexSuite`: unaligned BYTE_STREAM_SPLIT pages with v1 and
v2 pages, required and nullable columns. Making the skips throw fails this test
for both page versions, and making the exact count check throw fails the round
trip for both.
- The exact-fit test now checks the values of every batch read.
`Int.MaxValue` after a skip covers the overflow (a rewrite to `offset + total >
valueCount` fails it). `assertCorrupted` uses `withClue` with `Position`, and
FLBA of width 5 now runs the shared suite, replacing the separate negative test.
- `nextIndex()` calls `checkRemaining(1)`, and a negative count throws
`IllegalArgumentException`.
I'll backport this to `branch-4.x` and `branch-4.3` after it's merged.
--
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]
