fqaiser94 commented on code in PR #115:
URL: https://github.com/apache/iceberg-rust/pull/115#discussion_r1421245396


##########
crates/iceberg/src/spec/sort.rs:
##########
@@ -88,15 +91,106 @@ impl SortOrder {
     pub fn is_unsorted(&self) -> bool {
         self.fields.is_empty()
     }
+
+    /// Converts to an unbound sort order
+    pub fn to_unbound(&self) -> UnboundSortOrder {
+        UnboundSortOrder::builder()
+            .with_order_id(self.order_id)
+            .with_fields(
+                self.fields
+                    .iter()
+                    .map(|x| UnboundSortField {
+                        source_id: x.source_id,
+                        transform: x.transform,
+                        direction: x.direction,
+                        null_order: x.null_order,
+                    })
+                    .collect_vec(),
+            )
+            .build()
+            .unwrap()
+    }
+}
+
+/// Reference to [`UnboundSortOrder`].
+pub type UnboundSortOrderRef = Arc<UnboundSortOrder>;
+
+/// Entry for every column that is to be sorted
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, TypedBuilder)]
+#[serde(rename_all = "kebab-case")]
+pub struct UnboundSortField {
+    /// A source column id from the table’s schema
+    pub source_id: i32,
+    /// A transform that is used to produce values to be sorted on from the 
source column.
+    pub transform: Transform,
+    /// A sort direction, that can only be either asc or desc
+    pub direction: SortDirection,
+    /// A null order that describes the order of null values when sorted.
+    pub null_order: NullOrder,
+}
+
+/// Unbound sort order can be later bound to a schema.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Builder, 
Default)]
+#[serde(rename_all = "kebab-case")]
+#[builder(setter(prefix = "with"))]
+#[builder(build_fn(skip))]
+pub struct UnboundSortOrder {
+    /// Identifier for the SortOrder, order_id `0` is no sort order.
+    pub order_id: i64,
+    /// Details of the sort
+    #[builder(setter(each(name = "with_sort_field")))]
+    pub fields: Vec<UnboundSortField>,
+}

Review Comment:
   TBH I'm not really sure what I've achieved anymore with this PR as 
`UnboundSortOrder` is pretty much a copy of `SortOrder`. 
   
   In the Java implementation, 
[`SortOrder`](https://github.com/apache/iceberg/blob/4d0b69beba104e6912f6e6850189121fcd23ef8a/api/src/main/java/org/apache/iceberg/SortOrder.java#L41)
 and 
[`UnboundSortOrder`](https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/UnboundSortOrder.java)
 are more differentiated since `SortOrder` is associated with a `Schema`. That 
association does not exist explicitly in the Rust implementation today. I think 
we could add a `Schema` member to `SortOrder` but that should probably be a 
separate PR. 



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