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

Fix clippy #900

Merged
merged 1 commit into from
Nov 1, 2021
Merged

Fix clippy #900

merged 1 commit into from
Nov 1, 2021

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Nov 1, 2021

Rationale for this change

Fix a clippy error introduced by #878

I think due to a logical conflict with #591

What changes are included in this PR?

remove unnecessary use

Are there any user-facing changes?

no

@alamb alamb requested a review from jimexist November 1, 2021 13:50
@github-actions github-actions bot added the parquet Changes to the parquet crate label Nov 1, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #900 (995427f) into master (06f730e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 995427f differs from pull request most recent head bd69580. Consider uploading reports for the commit bd69580 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #900      +/-   ##
==========================================
- Coverage   82.30%   82.30%   -0.01%     
==========================================
  Files         168      168              
  Lines       48031    48029       -2     
==========================================
- Hits        39533    39530       -3     
- Misses       8498     8499       +1     
Impacted Files Coverage Δ
parquet/src/util/bit_packing.rs 99.96% <ø> (ø)
parquet/src/util/bit_util.rs 93.19% <ø> (ø)
arrow/src/compute/kernels/comparison.rs 92.63% <100.00%> (ø)
parquet/src/util/hash_util.rs 95.65% <100.00%> (-0.13%) ⬇️
parquet_derive/src/parquet_field.rs 65.98% <0.00%> (-0.46%) ⬇️
arrow/src/datatypes/datatype.rs 64.93% <0.00%> (-0.44%) ⬇️
arrow/src/array/transform/mod.rs 85.33% <0.00%> (+0.13%) ⬆️
parquet/src/encodings/encoding.rs 93.71% <0.00%> (+0.19%) ⬆️

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 0892595...bd69580. Read the comment docs.

@@ -49,7 +49,6 @@ const MURMUR_R: i32 = 47;

/// Rust implementation of MurmurHash2, 64-bit version for 64-bit platforms
fn murmur_hash2_64a(data_bytes: &[u8], seed: u64) -> u64 {
Copy link
Member

Choose a reason for hiding this comment

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

would call for using murmur hash 3 if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jimexist shall I file a ticket? I don't really know why the parquet implementation is using its own hash function rather than, say, aHash or some other standard hashing library in rust

@alamb alamb merged commit 2cf3178 into apache:master Nov 1, 2021
@alamb alamb deleted the alamb/parquet-clippy branch November 1, 2021 14:43
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.

3 participants