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 schema index type consistent #14256

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Make parquet schema index type consistent #14256

merged 4 commits into from
Oct 10, 2023

Conversation

hyperbolic2346
Copy link
Contributor

Description

While working on parquet schema issue I noticed that the parent and child index didn't match. Discussion ensued and size_type was decided.

Checklist

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

@hyperbolic2346 hyperbolic2346 added 3 - Ready for Review Ready for review by team code quality libcudf Affects libcudf (C++/CUDA) code. cuIO cuIO issue non-breaking Non-breaking change labels Oct 5, 2023
@hyperbolic2346 hyperbolic2346 requested a review from a team as a code owner October 5, 2023 17:25
@hyperbolic2346 hyperbolic2346 self-assigned this Oct 5, 2023
@copy-pr-bot
Copy link

copy-pr-bot bot commented Oct 5, 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.

@hyperbolic2346 hyperbolic2346 added the improvement Improvement / enhancement to an existing function label Oct 5, 2023
@hyperbolic2346
Copy link
Contributor Author

/ok to test

@hyperbolic2346 hyperbolic2346 changed the title Changing index types of child and parent to match Make parquet schema index type consistent Oct 5, 2023
vuule
vuule approved these changes Oct 5, 2023
View reviewed changes
cpp/src/io/parquet/parquet.hpp
Comment on lines +158 to +159
size_type parent_idx = 0;
std::vector<size_type> children_idx;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like how they were both wrong

@hyperbolic2346
Copy link
Contributor Author

/ok to test

@hyperbolic2346
Copy link
Contributor Author

/ok to test

@hyperbolic2346
Copy link
Contributor Author

/ok to test

@ajschmidt8
Copy link
Member

/ok to test

@hyperbolic2346, you can prevent having to type /ok to test by configuring your commits to be signed as mentioned in the contributor link on the comment below

@vuule
Copy link
Contributor

vuule commented Oct 10, 2023

/merge

@rapids-bot rapids-bot bot merged commit 053da82 into rapidsai:branch-23.12 Oct 10, 2023
60 checks passed
@hyperbolic2346 hyperbolic2346 deleted the mwilson/parent-child-idx branch October 10, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mythrocks mythrocks mythrocks approved these changes

@ttnghia ttnghia ttnghia approved these changes

@vuule vuule vuule approved these changes

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

Assignees

@hyperbolic2346 hyperbolic2346

Labels
3 - Ready for Review Ready for review by team cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
cuDF/Dask/Numba/UCX
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hyperbolic2346 @ajschmidt8 @vuule @mythrocks @ttnghia