Skip to content

Implement skip_rep_levels #2122

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
Closed
thinkharderdev wants to merge 2 commits into from
Closed

Implement skip_rep_levels #2122

thinkharderdev wants to merge 2 commits into from

Conversation

@thinkharderdev
Copy link
Contributor

@thinkharderdev thinkharderdev commented Jul 21, 2022

Which issue does this PR close?

Closes #2108

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@thinkharderdev
Copy link
Contributor Author

thinkharderdev commented Jul 21, 2022

@tustvold @Ted-Jiang Leaving this as a draft rn as I'm not sure I understand how all the pieces fit together here and what an appropriate unit test would be.

@github-actions github-actions bot added the parquet Changes to the parquet crate label Jul 21, 2022
@Ted-Jiang
Copy link
Member

Ted-Jiang commented Jul 22, 2022

@thinkharderdev Thanks for pin me, But i am not familiar with this?
I think we only need skip_rep_levels when need support Complex type 🤔
Just curious , is there a situation you need this?

@thinkharderdev
Copy link
Contributor Author

thinkharderdev commented Jul 22, 2022

@thinkharderdev Thanks for pin me, But i am not familiar with this? I think we only need skip_rep_levels when need support Complex type 🤔 Just curious , is there a situation you need this?

Not anything in particular, was just trying to knock out todo items for skipping :)

@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2022

Codecov Report

Merging #2122 (285df0c) into master (1621c71) will decrease coverage by 0.03%.
The diff coverage is 59.57%.

@@            Coverage Diff             @@
##           master    #2122      +/-   ##
==========================================
- Coverage   82.85%   82.81%   -0.04%     
==========================================
  Files         237      237              
  Lines       61381    61473      +92     
==========================================
+ Hits        50856    50909      +53     
- Misses      10525    10564      +39
Impacted Files Coverage Δ
parquet/src/column/reader/decoder.rs 39.31% <0.00%> (-10.69%) ⬇️
parquet/src/encodings/rle.rs 91.51% <68.96%> (-1.28%) ⬇️
parquet/src/util/bit_util.rs 93.76% <94.73%> (+0.06%) ⬆️
arrow/src/datatypes/datatype.rs 62.69% <0.00%> (-0.32%) ⬇️
parquet_derive/src/parquet_field.rs 65.75% <0.00%> (-0.23%) ⬇️
parquet/src/encodings/encoding.rs 93.43% <0.00%> (-0.20%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@tustvold tustvold mentioned this pull request Aug 16, 2022
Merged
@tustvold
Copy link
Contributor

tustvold commented Aug 16, 2022

Would you be happy for me to pick this one up?

@thinkharderdev
Copy link
Contributor Author

thinkharderdev commented Aug 16, 2022

Would you be happy for me to pick this one up?

Yes please :)

@tustvold
Copy link
Contributor

tustvold commented Nov 10, 2022

Closing as #2999 has been merged

@tustvold tustvold closed this Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

parquet Changes to the parquet crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support skip_rep_levels in ColumnLevelDecoder

4 participants

@thinkharderdev @Ted-Jiang @codecov-commenter @tustvold