viirya commented on code in PR #2765:
URL: https://github.com/apache/iceberg-rust/pull/2765#discussion_r3972354009
##########
crates/iceberg/src/transaction/sort_order.rs:
##########
@@ -159,14 +209,115 @@ 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() {
+ 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();
+
+ let sort_order = match &updates[0] {
+ TableUpdate::AddSortOrder { sort_order } => sort_order,
+ other => panic!("expected AddSortOrder, got {other:?}"),
+ };
+ assert_eq!(sort_order.fields[0].transform, Transform::Bucket(16));
+ }
+
+ #[tokio::test]
+ async fn test_replace_sort_order_rejects_incompatible_transform() {
+ let table = make_v2_table();
+ // `x` is a `long` column; `year` only accepts date/timestamp types.
+ let action = Arc::new(ReplaceSortOrderAction::new().asc_with_transform(
+ "x",
+ Transform::Year,
+ NullOrder::First,
+ ));
+
+ let err = match TransactionAction::commit(action, &table).await {
+ Err(e) => e,
+ Ok(_) => panic!("year transform on a long column should be
rejected"),
+ };
+ assert_eq!(err.kind(), ErrorKind::Unexpected);
+ }
+
+ #[tokio::test]
+ async fn test_sort_order_transform_survives_metadata_json_round_trip() {
+ // Commit a transform-based sort order through a real catalog: the
memory
+ // catalog serializes the updated table metadata to a metadata.json
file
+ // (`TableMetadata::write_to`), and `load_table` reads that file back
and
+ // parses it (`TableMetadata::read_from`). This exercises the full
+ // JSON round-trip of the transform (e.g. `"bucket[16]"`), not just the
+ // in-memory `TableUpdate`.
+ let catalog = new_memory_catalog().await;
+ let table = make_v3_minimal_table_in_catalog(&catalog).await;
+
+ let tx = Transaction::new(&table);
+ let tx = 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 committed = tx.commit(&catalog).await.unwrap();
+
+ // Reload from the catalog: this parses the metadata.json written
above.
+ let reloaded =
catalog.load_table(committed.identifier()).await.unwrap();
+ let sort_order = reloaded.metadata().default_sort_order();
+
+ assert_eq!(sort_order.fields.len(), 2);
+ assert_eq!(sort_order.fields[0].transform, Transform::Bucket(16));
+ assert_eq!(sort_order.fields[0].direction, SortDirection::Ascending);
Review Comment:
Done in 0fba334f. The round-trip test now also asserts `NullOrder::First`
for the first field and `NullOrder::Last` for the second field after reloading
the metadata.
--
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]
