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

Write FixedLenByteArray stats for FixedLenByteArray columns (not ByteArray stats) #662

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Aug 5, 2021

Which issue does this PR close?

Resolves #660

Rationale for this change

FixedLenByteArray columns were writing the wrong type of statistics

What changes are included in this PR?

Write FixedLenByteArray stats for FixedLenByteArray columns (not ByteArray stats)

Are there any user-facing changes?

FixedLenByteArray statistics are written rather than ByteArray statistics. I am not sure if this is a user visible change however (see description on #661 (comment))

@github-actions github-actions bot added the parquet Changes to the parquet crate label Aug 5, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #662 (18349ac) into master (6bf1988) will decrease coverage by 0.06%.
The diff coverage is 85.39%.

❗ Current head 18349ac differs from pull request most recent head 67add39. Consider uploading reports for the commit 67add39 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #662      +/-   ##
==========================================
- Coverage   82.50%   82.43%   -0.07%     
==========================================
  Files         168      168              
  Lines       47249    47325      +76     
==========================================
+ Hits        38984    39014      +30     
- Misses       8265     8311      +46     
Impacted Files Coverage Δ
parquet/src/column/writer.rs 92.81% <85.36%> (-0.49%) ⬇️
parquet/src/data_type.rs 77.29% <85.71%> (+0.75%) ⬆️
arrow/src/array/transform/boolean.rs 76.92% <0.00%> (-7.70%) ⬇️
arrow/src/array/transform/utils.rs 95.00% <0.00%> (-5.00%) ⬇️
arrow/src/array/equal_json.rs 85.21% <0.00%> (-3.48%) ⬇️
arrow/src/tensor.rs 85.00% <0.00%> (-2.50%) ⬇️
parquet/src/column/page.rs 97.36% <0.00%> (-1.32%) ⬇️
arrow/src/array/equal/utils.rs 74.00% <0.00%> (-1.00%) ⬇️
parquet/src/record/api.rs 91.60% <0.00%> (-0.88%) ⬇️
parquet/src/file/statistics.rs 93.80% <0.00%> (-0.83%) ⬇️
... and 17 more

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 6bf1988...67add39. Read the comment docs.

@alamb alamb force-pushed the alamb/fix_fixed_len_byte_array_stats branch from 67add39 to f99c351 Compare August 8, 2021 10:33
@alamb alamb marked this pull request as ready for review August 8, 2021 10:33
@alamb alamb requested a review from sunchao August 8, 2021 10:34
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

@sunchao sunchao merged commit fa5acd9 into apache:master Aug 10, 2021
alamb added a commit that referenced this pull request Aug 10, 2021
@alamb alamb deleted the alamb/fix_fixed_len_byte_array_stats branch August 10, 2021 21:11
alamb added a commit that referenced this pull request Aug 12, 2021
alamb added a commit that referenced this pull request Aug 12, 2021
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.

Parquet fixed length byte array columns write byte array statistics
4 participants