Handle trailing padding when skipping repetition levels (#3911) #4319
Conversation
| for _ in 0..10 { | ||
| let mut rng = thread_rng(); |
There was a problem hiding this comment.
Moving this into the for loop makes it more likely to encounter issues inherent to the RLE data, as opposed to the selection of it
There was a problem hiding this comment.
it might make sense to add a note to the test giving the rationale for the loop (and maybe name the test 'test_ski-_fuzz` or something
There was a problem hiding this comment.
I can't say I really understand the guts of the parquet decoder, but I reviewed the test coverage and that looks solid to me. 👍 Thanks for the cleanup @tustvold
Also thank you for the improvements to readability (comments and more descriptive variable names)
| for _ in 0..10 { | ||
| let mut rng = thread_rng(); |
There was a problem hiding this comment.
it might make sense to add a note to the test giving the rationale for the loop (and maybe name the test 'test_ski-_fuzz` or something
There was a problem hiding this comment.
I can't say I really understand the guts of the parquet decoder, but I reviewed the test coverage and that looks solid to me. 👍 Thanks for the cleanup @tustvold
Also thank you for the improvements to readability (comments and more descriptive variable names)
Which issue does this PR close?
Closes #3911
Rationale for this change
The length of RLE encoded data is ambiguous if the final block is bit packed. As such the decoder might think it has more level data than it actually has when skipping repetition levels, which conceivably could cause the decoder to get out of sync
What changes are included in this PR?
Are there any user-facing changes?
No, these APIs are not public