-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for Parquet page V2 on native reader #70807
Support for Parquet page V2 on native reader #70807
Conversation
This is an automated comment for commit 0f9a2bd with description of existing statuses. It's updated for the latest CI running ✅ Click here to open a full report in a separate page Successful checks
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, there are 1.5 bugs, but also, optionally, can you please fix two easy pre-existing bugs along the way, thanks :)
|
||
if (col_descriptor.max_repetition_level() > 0) | ||
{ | ||
auto rep_levels_bytes = repetition_level_decoder.SetData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the repetition_level_decoder
declaration too, in 2 places.
…eader-v2-native-reader 24.8.8 Backport of ClickHouse#70807 parquet page header v2 native reader
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Support for Parquet page V2 on native reader
Documentation entry for user-facing changes
CI Settings (Only check the boxes if you know what you are doing):