moomindani commented on PR #2802: URL: https://github.com/apache/iceberg-rust/pull/2802#issuecomment-5736693803
Thanks — happy to follow that direction. Before I start the wide mechanical part I'd like to confirm the target shape, since nothing references #3172 yet and this would be the first struct to get it. Plan, if it matches what you have in mind: 1. `PartitionField`: fields private with accessors, `source_ids: Vec<i32>`, serde module renamed to `_serde` — this PR, re-cut to that scope. 2. `SortField`: the same, as a follow-up. 3. `UnboundPartitionField`: the same, plus the bound/unbound conversion, so the multi-argument ids stop being dropped on the way into `TableCreation` and `TableUpdate::AddSpec`. Two questions where guessing would be expensive: - **Construction path.** The precedent I can see in the crate is `PartitionSpec`: private fields, `spec_id()`/`fields()` accessors, builder. Should `PartitionField` keep its derived `TypedBuilder`, or get a validating constructor, given that the point of #3172 is for a built instance to be known valid? - **`source_id` once the fields are private.** With `source_ids: Vec<i32>` always populated, should `source_id` stop being stored and become an accessor returning the first id, or stay as its own field? One note on sequencing: with the split, the bound-to-unbound conversion keeps dropping the extra ids until step 3 lands, because `UnboundPartitionField` has nowhere to put them. Fine by me — flagging it so it isn't a surprise in review. The two smaller points are agreed and will be in the re-cut: `_serde` naming with the modules merged, and `Vec<i32>` instead of `Option<Vec<i32>>`. -- 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]
