alamb commented on code in PR #8048: URL: https://github.com/apache/arrow-rs/pull/8048#discussion_r2258266438
########## parquet/src/basic.rs: ########## @@ -817,6 +877,90 @@ impl From<ConvertedType> for Option<parquet::ConvertedType> { } } +// ---------------------------------------------------------------------- +// parquet::BloomFilterHash <=> BloomFilterHash conversion + +impl From<parquet::BloomFilterHash> for BloomFilterHash { + fn from(value: parquet::BloomFilterHash) -> Self { + match value { + parquet::BloomFilterHash::XXHASH(_) => BloomFilterHash::XXHASH, + } + } +} + +impl From<BloomFilterHash> for parquet::BloomFilterHash { Review Comment: BTW this is not anything you did, but I found the use if `use crate::format as parquet` very confusing -- given how many things are called parquet I would personally suggest we use `crate::format` explicitly everywhere for clarity ########## parquet/src/basic.rs: ########## @@ -817,6 +877,90 @@ impl From<ConvertedType> for Option<parquet::ConvertedType> { } } +// ---------------------------------------------------------------------- +// parquet::BloomFilterHash <=> BloomFilterHash conversion + +impl From<parquet::BloomFilterHash> for BloomFilterHash { Review Comment: Is the idea that these are transition mappings from the thrift structures --> the native Rust structures? I am hoping that the end stage will be a single `BloomFilterHash` enum in the rust parquet crate that is serialized/deserialized directly to thrift by your new fancy encoder/decoder. Is this your vision too? ########## parquet/src/basic.rs: ########## @@ -26,17 +28,11 @@ use crate::format as parquet; use crate::errors::{ParquetError, Result}; -// Re-export crate::format types used in this module Review Comment: this is the main API change in this PR, right? No more directly exporting the thrift definitions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org