c-thiel commented on code in PR #491:
URL: https://github.com/apache/iceberg-rust/pull/491#discussion_r1704331295


##########
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 believe there is another use-case: Adding a new partition field.
   We would use `with_partition_field` for all fields of the old spec we want 
to keep.
   Then we use the new function you mentioned below to add a new field.
   This field must obtain a larger field_id than the last_assigned_field_id in 
the previous TableMetadata Version. Thus we need to set it.



-- 
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