Skip to content
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

parquet writer: Raise an error when the row_group_index overflows i16 #6378

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

progval
Copy link
Contributor

@progval progval commented Sep 10, 2024

Which issue does this PR close?

Rationale for this change

This caused confusing panics down the line because 'ordinal' is negative.

This is easy to reach, because Parquet requires ordinals to be i16, which only allows 32k row groups.

What changes are included in this PR?

Adds an overflow check when casting usize to i16; and an extra check when incrementing the usize for good measure (though it should never happen)

Are there any user-facing changes?

new error message

@github-actions github-actions bot added the parquet Changes to the parquet crate label Sep 10, 2024
@progval progval changed the title Raise an error when the row_group_index overflows i16 parquet writer: Raise an error when the row_group_index overflows i16 Sep 10, 2024
This caused confusing panics down the line because 'ordinal' is
negative.
@mapleFU
Copy link
Member

mapleFU commented Sep 10, 2024

Nice catch!

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @progval and @mapleFU

@tustvold tustvold merged commit f80bc5f into apache:master Sep 13, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants