Refactor Parquet reader handling of V2 page header info #13775
Conversation
|
Pull requests from external contributors require approval from a |
| } else { | ||
| init_rle(cur, cur + len); | ||
| } | ||
| init_rle(cur, cur + len); |
There was a problem hiding this comment.
Why do we have such big change here? Will this a breaking change?
There was a problem hiding this comment.
it's because now I set and test len in the if, so len is guaranteed non-zero and I can get rid of the redundant branching for the len==0 case.
There was a problem hiding this comment.
Oh, do we now hit the else branch at 915 instead? I see similar code with the branch that's removed here.
There was a problem hiding this comment.
Yes, exactly. The original fix from #13707 created a duplicate path to get to 0 bits for the level RLE encoding. This refactor gets rid of the duplicate.
|
/ok to test |
ttnghia
added
3 - Ready for Review
davidwendt
added
the
improvement
|
/ok to test |
|
/ok to test |
|
/merge |
Description
This PR replaces the
def_lvl_bytesandrep_lvl_bytesfields of thegpu::PageInfostruct with an array indexed bygpu::level_type(as is done with thelvl_decode_bufarray). This allows for some streamlining inInitLevelSection(), removing some redundant code and improving readability.See this comment for context.
Checklist