CTTY commented on code in PR #1625:
URL: https://github.com/apache/iceberg-rust/pull/1625#discussion_r2299295538


##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -176,6 +176,38 @@ impl PartitionSpec {
     }
 }
 
+/// A partition key represents a specific partition in a table, containing the 
partition spec,
+/// schema, and the actual partition values.
+#[derive(Clone, Debug)]
+pub struct PartitionKey {
+    /// The partition spec that contains the partition fields.
+    spec: PartitionSpec,
+    /// The schema to which the partition spec is bound.
+    schema: SchemaRef,
+    /// Partition fields' values in struct.
+    data: Struct,
+}
+
+impl PartitionKey {
+    /// Creates a new partition key with the given spec, schema, and data.
+    pub fn new(spec: PartitionSpec, schema: SchemaRef, data: Struct) -> Self {
+        Self { spec, schema, data }
+    }
+
+    /// Generates a partition path based on the partition values.
+    pub fn to_path(&self) -> String {
+        self.spec.partition_to_path(&self.data, self.schema.clone())
+    }
+}
+
+/// Checks if a partition key is effectively none.
+pub fn partition_key_is_none(partition_key: Option<&PartitionKey>) -> bool {

Review Comment:
   I think putting this in `PartitionKey` is a good idea. I'll fix this once 
the API design is finalized.
   
   This helper function may be used in custom implementations of location 
generators or used along with some custom writers, so I think it's better to 
leave it `pub`



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

Reply via email to