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

Reply via email to