liurenjie1024 commented on code in PR #1851:
URL: https://github.com/apache/iceberg-rust/pull/1851#discussion_r2537368596
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
Do we really need this method? I think passing a `None` to default value in
`parse_property` would be enough?
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,59 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
Review Comment:
Why not put the imports at the beginning of file as others do?
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -34,6 +34,25 @@ where
})
}
+// Helper function to parse an optional property from a HashMap
+// If the property is not found, returns None
+fn parse_optional_property<T: std::str::FromStr>(
+ properties: &HashMap<String, String>,
+ key: &str,
+) -> Result<Option<T>, anyhow::Error>
Review Comment:
We should use this crate's error.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -461,9 +461,59 @@ impl TableMetadata {
file_io: &FileIO,
metadata_location: impl AsRef<str>,
) -> Result<()> {
+ use std::io::Write as _;
+
+ use flate2::write::GzEncoder;
+
+ let json_data = serde_json::to_vec(self)?;
+
+ // Check if compression is enabled via table properties
+ let codec = self
+ .properties
+
.get(crate::spec::table_properties::TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC)
Review Comment:
Why we need a fully qualified name here? It's difficult to read.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
