blackmwk commented on code in PR #3030:
URL: https://github.com/apache/iceberg-rust/pull/3030#discussion_r3828065646
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -44,7 +44,8 @@ fn parse_location_property(path: &str) -> Result<String> {
///
/// Returns an error if the codec is not "none", "", or "gzip"
(case-insensitive).
/// Lz4 and Zstd are not supported for metadata file compression.
-pub(crate) fn parse_metadata_file_compression(
+#[cfg(test)]
+fn parse_metadata_file_compression(
Review Comment:
Remove this if not used any more.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -141,6 +143,14 @@ pub struct TableMetadata {
}
impl TableMetadata {
+ pub(crate) fn parse_table_properties(
Review Comment:
I don't want this, you should modify the TryFrom in TableProperties.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -95,6 +95,8 @@ pub struct TableMetadata {
/// affect reading and writing and is not intended to be used for
arbitrary metadata.
/// For example, commit.retry.num-retries is used to control the number of
commit retries.
pub(crate) properties: HashMap<String, String>,
+ /// Typed table properties parsed from `properties` and kept in sync by
the metadata builder.
+ pub(crate) table_properties: TableProperties,
Review Comment:
Make this private
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -499,7 +508,7 @@ impl TableMetadata {
let json_data = serde_json::to_vec(self)?;
// Check if compression codec from properties matches the one in
metadata_location
- let codec = parse_metadata_file_compression(&self.properties)?;
+ let codec = *self.table_properties.metadata_compression_codec();
Review Comment:
I perfer to do clone here.
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -140,7 +141,7 @@ fn parse_parquet_compression(
}
/// TableProperties that contains the properties of a table.
-#[derive(Debug, Properties)]
+#[derive(Clone, Debug, Eq, PartialEq, Properties)]
Review Comment:
Remove the Eq
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -385,14 +395,13 @@ impl TableMetadata {
///
/// Returns an error if the compression codec property has an invalid
value.
pub fn metadata_compression_codec(&self) -> Result<CompressionCodec> {
- parse_metadata_file_compression(&self.properties)
+ Ok(*self.table_properties.metadata_compression_codec())
Review Comment:
Clone
--
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]
