c-thiel commented on code in PR #491: URL: https://github.com/apache/iceberg-rust/pull/491#discussion_r1704310954
########## crates/iceberg/src/spec/partition.rs: ########## @@ -117,22 +144,312 @@ pub struct UnboundPartitionField { } /// Unbound partition spec can be built without a schema and later bound to a schema. -#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Default, Builder)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Default)] #[serde(rename_all = "kebab-case")] -#[builder(setter(prefix = "with"))] pub struct UnboundPartitionSpec { /// Identifier for PartitionSpec - #[builder(default, setter(strip_option))] pub spec_id: Option<i32>, /// Details of the partition spec - #[builder(setter(each(name = "with_unbound_partition_field")))] pub fields: Vec<UnboundPartitionField>, } impl UnboundPartitionSpec { /// Create unbound partition spec builer - pub fn builder() -> UnboundPartitionSpecBuilder { - UnboundPartitionSpecBuilder::default() + pub fn builder() -> PartitionSpecBuilder { + PartitionSpecBuilder::default() + } +} + +/// Create valid partition specs for a given schema. +#[derive(Debug, Default)] +pub struct PartitionSpecBuilder { + spec_id: Option<i32>, + last_assigned_field_id: i32, + fields: Vec<UnboundPartitionField>, +} + +impl PartitionSpecBuilder { + pub(crate) const UNPARTITIONED_LAST_ASSIGNED_ID: i32 = 999; + // Default partition spec id is only used for building `PartitionSpec`. + // When building unbound partition specs, the spec id is not set by default. + pub(crate) const DEFAULT_PARTITION_SPEC_ID: i32 = 0; + + /// Create a new partition spec builder with the given schema. + pub fn new() -> Self { + Self { + spec_id: None, + fields: vec![], + last_assigned_field_id: Self::UNPARTITIONED_LAST_ASSIGNED_ID, + } + } + + /// Set the last assigned field id for the partition spec. + /// This is useful when re-binding partition specs. + pub fn with_last_assigned_field_id(mut self, last_assigned_field_id: i32) -> Self { Review Comment: I am not certain if we need it anymore if we split the builder. As field_ids need to be global in V2 spec, we need to set the last assigned field id when creating a new partition spec. Because it's a new spec, we are only adding unbound partition fields with no assigned `field_id` yet. Thus we need to call with_last_assigned_field_id when binding the unbound fields to a schema. `last_assigned_field_id ` is obtained from the previous Table Metadata. -- 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