blackmwk commented on code in PR #3030:
URL: https://github.com/apache/iceberg-rust/pull/3030#discussion_r3828848756
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -141,6 +143,48 @@ pub struct TableMetadata {
}
impl TableMetadata {
+ /// Create empty table metadata for initialization by
[`TableMetadataBuilder`].
+ pub(super) fn new_empty(format_version: FormatVersion) -> Result<Self> {
+ let properties = HashMap::new();
+ let table_properties = TableProperties::try_from(&properties)?;
+
+ Ok(Self {
+ format_version,
+ table_uuid: Uuid::now_v7(),
+ location: String::new(),
+ last_sequence_number: 0,
+ last_updated_ms: 0,
+ last_column_id: -1,
+ schemas: HashMap::new(),
+ current_schema_id: -1,
+ partition_specs: HashMap::new(),
+ default_spec:
Arc::new(PartitionSpec::unpartition_spec().with_spec_id(-1)),
+ default_partition_type: StructType::new(vec![]),
+ last_partition_id: UNPARTITIONED_LAST_ASSIGNED_ID,
+ properties,
+ table_properties,
+ current_snapshot_id: None,
+ snapshots: HashMap::new(),
+ snapshot_log: vec![],
+ metadata_log: vec![],
+ sort_orders: HashMap::new(),
+ default_sort_order_id: -1,
+ refs: HashMap::new(),
+ statistics: HashMap::new(),
+ partition_statistics: HashMap::new(),
+ encryption_keys: HashMap::new(),
+ next_row_id: INITIAL_ROW_ID,
+ })
+ }
+
+ /// Replace the raw and typed table properties together.
+ pub(super) fn replace_properties(&mut self, properties: HashMap<String,
String>) -> Result<()> {
Review Comment:
Remove this
##########
crates/iceberg/src/spec/table_properties.rs:
##########
@@ -140,7 +117,7 @@ fn parse_parquet_compression(
}
/// TableProperties that contains the properties of a table.
-#[derive(Debug, Properties)]
+#[derive(Clone, Debug, PartialEq, Properties)]
Review Comment:
I'm wrong, add back Eq
##########
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.
+ table_properties: TableProperties,
Review Comment:
Ok, I'm wrong, make it pub(crate)
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -384,15 +428,15 @@ impl TableMetadata {
/// # Errors
///
/// Returns an error if the compression codec property has an invalid
value.
+ #[allow(clippy::clone_on_copy)]
pub fn metadata_compression_codec(&self) -> Result<CompressionCodec> {
- parse_metadata_file_compression(&self.properties)
+ Ok(self.table_properties.metadata_compression_codec().clone())
Review Comment:
Ok, I'm wrong, remove clone
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -491,6 +535,7 @@ impl TableMetadata {
}
/// Write table metadata to the given location.
+ #[allow(clippy::clone_on_copy)]
Review Comment:
I don't wnat to see this, remove newly added clone.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -141,6 +143,48 @@ pub struct TableMetadata {
}
impl TableMetadata {
+ /// Create empty table metadata for initialization by
[`TableMetadataBuilder`].
+ pub(super) fn new_empty(format_version: FormatVersion) -> Result<Self> {
Review Comment:
Remove this
--
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]
