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

Update triplet.rs to support REPEATED field with null value at one place #556

Merged
merged 1 commit into from
Jul 20, 2021
Merged

Update triplet.rs to support REPEATED field with null value at one place #556

merged 1 commit into from
Jul 20, 2021

Conversation

aiglematth
Copy link
Contributor

Update triplet.rs to support REPEATED field with null value at one place

Which issue does this PR close?

No

Rationale for this change

Used to manage REPEATED fields with null values. Drill generates it for example when creating the parquet.

What changes are included in this PR?

Replace the assert statement by an if statement that return a Field::Null value in the pub fn current_value(&self) -> Field locate in record/triplet.rs.

Are there any user-facing changes?

No

Update triplet.rs to support REPEATED field with null value at one time
@github-actions github-actions bot added the parquet Changes to the parquet crate label Jul 15, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #556 (e1725c7) into master (f873d77) will decrease coverage by 0.00%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #556      +/-   ##
==========================================
- Coverage   82.47%   82.47%   -0.01%     
==========================================
  Files         167      167              
  Lines       46144    46145       +1     
==========================================
  Hits        38059    38059              
- Misses       8085     8086       +1     
Impacted Files Coverage Δ
parquet/src/record/triplet.rs 92.37% <50.00%> (-0.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f873d77...e1725c7. Read the comment docs.

@alamb alamb changed the title Update triplet.rs Update triplet.rs to support REPEATED field with null value at one place Jul 19, 2021
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.

This looks fine to me, but I wonder if there is any way to add a test?

@sunchao do you want to review as well?

@alamb
Copy link
Contributor

alamb commented Jul 19, 2021

Thank you @aiglematth for the contribution

Copy link
Member

@sunchao sunchao left a comment

Choose a reason for hiding this comment

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

LGTM too. Thanks @alamb for pinging.

@alamb alamb merged commit 03269aa into apache:master Jul 20, 2021
alamb pushed a commit that referenced this pull request Jul 20, 2021
Update triplet.rs to support REPEATED field with null value at one time
alamb added a commit that referenced this pull request Jul 21, 2021
Update triplet.rs to support REPEATED field with null value at one time

Co-authored-by: aiglematth <aiglematth@protonmail.com>
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