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


##########
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:
   > We have different builder methods for bounded and unbounded.
   
   So I think this will cause a problem: if we get a SortOrder, is that bound 
or unbound? Or we must use it to create a builder and build to ensure we get a 
bound SortOrder. If so, may be we need a interface like `fn new(SortOrder) -> 
SortOrderBuilder` to create a builder from existing SortOrder.
   
   
   



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