moomindani opened a new pull request, #3267: URL: https://github.com/apache/iceberg-rust/pull/3267
## Which issue does this PR close? Part of #3172 and #2801. First of the three per-struct PRs @blackmwk asked for in #2802, starting with `UnboundPartitionField` as he suggested. #2802 stays open and will be narrowed to `PartitionField` once this lands. ## What changes are included in this PR? `UnboundPartitionField` gets the #3172 treatment — private fields with accessors — and with it the v3 `source-ids` spelling. - The four fields are private, with `source_id() -> Result<i32>`, `source_ids() -> &[i32]`, `field_id()`, `name()` and `transform()` accessors. The derived `TypedBuilder` is kept but its visibility is limited, so an instance can only be built inside the crate or read out of a spec JSON. - `source_ids: Vec<i32>` replaces `source_id: i32` and always holds at least one id. `source_id()` returns the single id, and an error for a multi-argument field rather than quietly handing back the first one. - A `_serde` module reads either spelling — `source-id`, or `source-ids` for a v3 multi-argument transform — and writes back the one that matches the field. An empty `source-ids`, a missing id, and a `source-id` that disagrees with the first `source-ids` entry are all rejected. - Binding a multi-argument field now fails with a clear error. A bound `PartitionField` still carries a single `source_id`, so until that struct is converted there is nowhere to put the extra ids, and failing loudly beats dropping them on the way into `TableCreation` or `TableUpdate::AddSpec`. - `check_for_redundant_partitions` compares the whole id list instead of one id. Three derived decisions worth your call: - The builder is `pub(crate)` rather than module-private, because `table_metadata_builder`, `partitioning` and the expression visitors all construct these. Say the word if you want it tighter and I will route those through `UnboundPartitionSpecBuilder`. - `add_unbound_field` and `add_unbound_fields` stay public, but outside the crate they can now only be fed a field cloned out of an existing spec. Happy to make them `pub(crate)` if that is the intent. - The field-id reuse path in `table_metadata_builder` mutated `field_id` in place, which private fields no longer allow, so there is a crate-internal `with_field_id`. Not in this PR, and not lost: `PartitionField` (#2802, to be narrowed once this lands) and `SortField` get the same treatment, and with them the multi-argument read support and the `Transform::Unknown` mapping that #2801 is about. ## Are these changes tested? Yes — four new unit tests in `spec/partition.rs`: reading a `source-ids`-only field and writing it back, the single-id round trip, the three malformed-id rejections, and binding a multi-argument field failing loudly. `cargo test -p iceberg --lib` passes (1768 tests), with `cargo fmt`, `cargo clippy --workspace --all-targets --all-features -- -D warnings` and a full workspace build clean. `crates/iceberg/public-api.txt` is regenerated: four public fields and the public `builder()` out, five accessors in. ## AI Disclosure This pull request and its description were written by Isaac. -- 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]
