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.

Sign up for GitHub

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

Jump to bottom

Make Parquet ColumnIndex null_counts optional #14596

Merged
merged 6 commits into from
Dec 13, 2023
Merged

Make Parquet ColumnIndex null_counts optional #14596

merged 6 commits into from
Dec 13, 2023

Conversation

etseidl
Copy link
Contributor

@etseidl etseidl commented Dec 7, 2023

Description

The Parquet specification lists the null_counts field of the ColumnIndex structure as "optional". This PR brings the cudf ColumnIndex struct in line with the specification.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@etseidl etseidl requested a review from a team as a code owner December 7, 2023 20:10
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Dec 7, 2023
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 7, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@etseidl
Copy link
Contributor Author

etseidl commented Dec 7, 2023

There is no change to CompactProtocolWriter because the ColumnIndex is written in device code.

@vuule vuule added non-breaking Non-breaking change cuIO cuIO issue improvement Improvement / enhancement to an existing function labels Dec 8, 2023
@vuule
Copy link
Contributor

vuule commented Dec 8, 2023

/ok to test

@vuule
Copy link
Contributor

vuule commented Dec 12, 2023

/ok to test

@vuule vuule added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Dec 12, 2023
@vuule
Copy link
Contributor

vuule commented Dec 13, 2023

/merge

@rapids-bot rapids-bot bot merged commit 420dc5d into rapidsai:branch-24.02 Dec 13, 2023
67 checks passed
@etseidl etseidl deleted the null_counts_optional branch December 13, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vuule vuule vuule approved these changes

@ttnghia ttnghia ttnghia approved these changes

@robertmaynard robertmaynard Awaiting requested review from robertmaynard robertmaynard is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@etseidl @vuule @ttnghia