y0psolo commented on PR #62: URL: https://github.com/apache/iceberg-rust/pull/62#issuecomment-1751805399
> Hi, @y0psolo Thanks for the effort! But I'm a little worried about the api here, since it's error prone, e.g. it's quite easy to construct an invalid table metadata. My suggestion would be to remove the derived builder, and for public apis of modification we can follow the java implementation: https://github.com/apache/iceberg/blob/abc3f746bfb00fdcd7ea1f170df242f1857ced75/core/src/main/java/org/apache/iceberg/TableMetadata.java#L842 Ok from what i read from the java class, below is the short list of functionalities you want to only provide on the builder : - set UUID - set FormatVersion - add a schema (a single schema not a map) - set current schema - add a PartitonSpec (a single PartitonSpec not a map) - set default PartitionSpec - add a default SortOrder (a single SortOrder not a map) - set the default sort order - add a snapshot (a single snpashot not a map) - set branch snapshot - add a ref (a single ref not a map) I will leave only this part public and not allow the builder to generate useless method. I don't know yet if we should remove the builder as it seems to provide some internal struct builder and default value at least. We will see with what is left at the end under his responsability. -- 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
