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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
