Skip to content

Commit 1105050

Browse files
committed
test: add unit tests for skip_serializing_if on optional fields
Add round-trip serialization tests that verify optional fields are omitted (not serialized as null) when absent, as requested in #2136. - CreateTableRequest: full and minimal round-trip serde test - UnboundPartitionSpec: assert serialized output matches expected JSON
1 parent 245b8da commit 1105050

2 files changed

Lines changed: 97 additions & 28 deletions

File tree

crates/catalog/rest/src/types.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,4 +359,64 @@ mod tests {
359359
json_no_props
360360
);
361361
}
362+
363+
#[test]
364+
fn test_create_table_request_serde() {
365+
let json_full = serde_json::json!({
366+
"name": "my_table",
367+
"location": "s3://bucket/table",
368+
"schema": {
369+
"schema-id": 0,
370+
"type": "struct",
371+
"fields": [
372+
{"id": 1, "name": "id", "required": true, "type": "int"}
373+
]
374+
},
375+
"partition-spec": {
376+
"fields": [
377+
{"source-id": 1, "name": "id_bucket", "transform": "bucket[16]"}
378+
]
379+
},
380+
"write-order": {
381+
"order-id": 0,
382+
"fields": []
383+
},
384+
"stage-create": true,
385+
"properties": {"key": "value"}
386+
});
387+
let request_full: CreateTableRequest =
388+
serde_json::from_value(json_full.clone()).expect("Deserialization failed");
389+
assert_eq!(request_full.name, "my_table");
390+
assert_eq!(request_full.location.as_deref(), Some("s3://bucket/table"));
391+
assert!(request_full.partition_spec.is_some());
392+
assert_eq!(request_full.stage_create, Some(true));
393+
assert_eq!(
394+
serde_json::to_value(&request_full).expect("Serialization failed"),
395+
json_full
396+
);
397+
398+
// Without optional fields — they must be omitted, not null
399+
let json_minimal = serde_json::json!({
400+
"name": "my_table",
401+
"schema": {
402+
"schema-id": 0,
403+
"type": "struct",
404+
"fields": [
405+
{"id": 1, "name": "id", "required": true, "type": "int"}
406+
]
407+
}
408+
});
409+
let request_minimal: CreateTableRequest =
410+
serde_json::from_value(json_minimal.clone()).expect("Deserialization failed");
411+
assert_eq!(request_minimal.name, "my_table");
412+
assert_eq!(request_minimal.location, None);
413+
assert_eq!(request_minimal.partition_spec, None);
414+
assert_eq!(request_minimal.write_order, None);
415+
assert_eq!(request_minimal.stage_create, None);
416+
assert!(request_minimal.properties.is_empty());
417+
assert_eq!(
418+
serde_json::to_value(&request_minimal).expect("Serialization failed"),
419+
json_minimal
420+
);
421+
}
362422
}

crates/iceberg/src/spec/partition.rs

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -856,27 +856,28 @@ mod tests {
856856

857857
#[test]
858858
fn test_unbound_partition_spec() {
859+
// With all optional fields present
859860
let spec = r#"
860-
{
861-
"spec-id": 1,
862-
"fields": [ {
863-
"source-id": 4,
864-
"field-id": 1000,
865-
"name": "ts_day",
866-
"transform": "day"
867-
}, {
868-
"source-id": 1,
869-
"field-id": 1001,
870-
"name": "id_bucket",
871-
"transform": "bucket[16]"
872-
}, {
873-
"source-id": 2,
874-
"field-id": 1002,
875-
"name": "id_truncate",
876-
"transform": "truncate[4]"
877-
} ]
878-
}
879-
"#;
861+
{
862+
"spec-id": 1,
863+
"fields": [ {
864+
"source-id": 4,
865+
"field-id": 1000,
866+
"name": "ts_day",
867+
"transform": "day"
868+
}, {
869+
"source-id": 1,
870+
"field-id": 1001,
871+
"name": "id_bucket",
872+
"transform": "bucket[16]"
873+
}, {
874+
"source-id": 2,
875+
"field-id": 1002,
876+
"name": "id_truncate",
877+
"transform": "truncate[4]"
878+
} ]
879+
}
880+
"#;
880881

881882
let partition_spec: UnboundPartitionSpec = serde_json::from_str(spec).unwrap();
882883
assert_eq!(Some(1), partition_spec.spec_id);
@@ -896,22 +897,30 @@ mod tests {
896897
assert_eq!("id_truncate", partition_spec.fields[2].name);
897898
assert_eq!(Transform::Truncate(4), partition_spec.fields[2].transform);
898899

900+
let expected: serde_json::Value = serde_json::from_str(spec).unwrap();
901+
assert_eq!(serde_json::to_value(&partition_spec).unwrap(), expected);
902+
903+
// Without optional fields — they must be omitted, not null
899904
let spec = r#"
900-
{
901-
"fields": [ {
902-
"source-id": 4,
903-
"name": "ts_day",
904-
"transform": "day"
905-
} ]
906-
}
907-
"#;
905+
{
906+
"fields": [ {
907+
"source-id": 4,
908+
"name": "ts_day",
909+
"transform": "day"
910+
} ]
911+
}
912+
"#;
913+
908914
let partition_spec: UnboundPartitionSpec = serde_json::from_str(spec).unwrap();
909915
assert_eq!(None, partition_spec.spec_id);
910916

911917
assert_eq!(4, partition_spec.fields[0].source_id);
912918
assert_eq!(None, partition_spec.fields[0].field_id);
913919
assert_eq!("ts_day", partition_spec.fields[0].name);
914920
assert_eq!(Transform::Day, partition_spec.fields[0].transform);
921+
922+
let expected: serde_json::Value = serde_json::from_str(spec).unwrap();
923+
assert_eq!(serde_json::to_value(&partition_spec).unwrap(), expected);
915924
}
916925

917926
#[test]

0 commit comments

Comments
 (0)