viirya commented on code in PR #2765:
URL: https://github.com/apache/iceberg-rust/pull/2765#discussion_r3526989390


##########
crates/iceberg/src/transaction/sort_order.rs:
##########
@@ -159,14 +195,90 @@ mod tests {
         assert_eq!(replace_sort_order.pending_sort_fields, vec![
             PendingSortField {
                 name: String::from("x"),
+                transform: Transform::Identity,
                 direction: SortDirection::Ascending,
                 null_order: NullOrder::First,
             },
             PendingSortField {
                 name: String::from("y"),
+                transform: Transform::Identity,
                 direction: SortDirection::Descending,
                 null_order: NullOrder::Last,
             }
         ]);
     }
+
+    #[test]
+    fn test_replace_sort_order_with_transform() {
+        let table = make_v2_table();
+        let tx = Transaction::new(&table);
+        let replace_sort_order = tx.replace_sort_order();
+
+        let tx = replace_sort_order
+            .asc_with_transform("x", Transform::Bucket(16), NullOrder::First)
+            .desc_with_transform("y", Transform::Truncate(4), NullOrder::Last)
+            .apply(tx)
+            .unwrap();
+
+        let replace_sort_order = (*tx.actions[0])
+            .downcast_ref::<ReplaceSortOrderAction>()
+            .unwrap();
+
+        assert_eq!(replace_sort_order.pending_sort_fields, vec![
+            PendingSortField {
+                name: String::from("x"),
+                transform: Transform::Bucket(16),
+                direction: SortDirection::Ascending,
+                null_order: NullOrder::First,
+            },
+            PendingSortField {
+                name: String::from("y"),
+                transform: Transform::Truncate(4),
+                direction: SortDirection::Descending,
+                null_order: NullOrder::Last,
+            }
+        ]);
+    }
+
+    #[tokio::test]
+    async fn test_replace_sort_order_with_transform_commits() {
+        use std::sync::Arc;
+
+        use crate::TableUpdate;
+
+        let table = make_v2_table();
+        let action = Arc::new(ReplaceSortOrderAction::new().asc_with_transform(
+            "x",
+            Transform::Bucket(16),
+            NullOrder::First,
+        ));
+
+        let mut action_commit = TransactionAction::commit(action, 
&table).await.unwrap();
+        let updates = action_commit.take_updates();

Review Comment:
   Good idea — added in eee40b8f: 
`test_sort_order_transform_survives_metadata_json_round_trip`.
   
   It commits a transform-based sort order (`bucket[16]` asc + `truncate[4]` 
desc) through the in-crate memory catalog, which serializes the updated table 
metadata to an actual metadata.json file (`TableMetadata::write_to` on 
`FileIO`), then reloads the table via `catalog.load_table` — which reads and 
parses that file back (`TableMetadata::read_from`) — and asserts both 
transforms and directions survive. So the transform string (`"bucket[16]"` 
etc.) is exercised through the real serialize-file-parse path, not just the 
in-memory `TableUpdate`.
   



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