c-thiel commented on code in PR #1362: URL: https://github.com/apache/iceberg-rust/pull/1362#discussion_r2140843618
########## crates/iceberg/src/spec/table_metadata.rs: ########## @@ -172,10 +180,18 @@ pub struct TableMetadata { /// even if the refs map is null. pub(crate) refs: HashMap<String, SnapshotReference>, /// Mapping of snapshot ids to statistics files. + #[builder(default, setter(transform = |stats: Vec<StatisticsFile>| { + stats.into_iter().map(|s| (s.snapshot_id, s)).collect() Review Comment: Its not - I missed the default for `snapshots` and `current_snapshot_id` and a few others. My basic reasoning is that all fields that must be set in order to produce valid metadata should not have a default, even if the Rust type would allow it. An example for a non-default field would be `schemas`, as we require at least one entry in the HashMap to be able to produce valid Metadata (`current_schema_id` must be set and be present). Valid `TableMetadata` must not contain `snapshots` though, and `current_snapshot_id` can be None, so those should have defaults. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org