From 2afca4ebe086863b6bac7064251f7e826c1023c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Bj=C3=B6rk?= Date: Thu, 12 Feb 2026 21:12:45 +0100 Subject: [PATCH 1/2] fix(rest-catalog): omit null optional fields in CreateTableRequest JSON --- crates/catalog/rest/src/types.rs | 4 ++++ crates/iceberg/src/spec/partition.rs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/crates/catalog/rest/src/types.rs b/crates/catalog/rest/src/types.rs index ab44c40ee3..d661bafeda 100644 --- a/crates/catalog/rest/src/types.rs +++ b/crates/catalog/rest/src/types.rs @@ -251,14 +251,18 @@ pub struct CreateTableRequest { /// Name of the table to create pub name: String, /// Optional table location. If not provided, the server will choose a location. + #[serde(skip_serializing_if = "Option::is_none")] pub location: Option, /// Table schema pub schema: Schema, /// Optional partition specification. If not provided, the table will be unpartitioned. + #[serde(skip_serializing_if = "Option::is_none")] pub partition_spec: Option, /// Optional sort order for the table + #[serde(skip_serializing_if = "Option::is_none")] pub write_order: Option, /// Whether to stage the create for a transaction (true) or create immediately (false) + #[serde(skip_serializing_if = "Option::is_none")] pub stage_create: Option, /// Optional properties to set on the table #[serde(default, skip_serializing_if = "HashMap::is_empty")] diff --git a/crates/iceberg/src/spec/partition.rs b/crates/iceberg/src/spec/partition.rs index 255aabd476..8ffc850a1e 100644 --- a/crates/iceberg/src/spec/partition.rs +++ b/crates/iceberg/src/spec/partition.rs @@ -246,6 +246,7 @@ pub struct UnboundPartitionField { /// A partition field id that is used to identify a partition field and is unique within a partition spec. /// In v2 table metadata, it is unique across all partition specs. #[builder(default, setter(strip_option(fallback = field_id_opt)))] + #[serde(skip_serializing_if = "Option::is_none")] pub field_id: Option, /// A partition name. pub name: String, @@ -260,6 +261,7 @@ pub struct UnboundPartitionField { #[serde(rename_all = "kebab-case")] pub struct UnboundPartitionSpec { /// Identifier for PartitionSpec + #[serde(skip_serializing_if = "Option::is_none")] pub(crate) spec_id: Option, /// Details of the partition spec pub(crate) fields: Vec, From 335fbd80e43424a29c8af5dc39da481cba6dd922 Mon Sep 17 00:00:00 2001 From: Matte Date: Sat, 21 Feb 2026 00:41:00 +0100 Subject: [PATCH 2/2] 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 --- crates/catalog/rest/src/types.rs | 60 +++++++++++++++++++++++++ crates/iceberg/src/spec/partition.rs | 65 ++++++++++++++++------------ 2 files changed, 97 insertions(+), 28 deletions(-) diff --git a/crates/catalog/rest/src/types.rs b/crates/catalog/rest/src/types.rs index d661bafeda..087a4e4b65 100644 --- a/crates/catalog/rest/src/types.rs +++ b/crates/catalog/rest/src/types.rs @@ -359,4 +359,64 @@ mod tests { json_no_props ); } + + #[test] + fn test_create_table_request_serde() { + let json_full = serde_json::json!({ + "name": "my_table", + "location": "s3://bucket/table", + "schema": { + "schema-id": 0, + "type": "struct", + "fields": [ + {"id": 1, "name": "id", "required": true, "type": "int"} + ] + }, + "partition-spec": { + "fields": [ + {"source-id": 1, "name": "id_bucket", "transform": "bucket[16]"} + ] + }, + "write-order": { + "order-id": 0, + "fields": [] + }, + "stage-create": true, + "properties": {"key": "value"} + }); + let request_full: CreateTableRequest = + serde_json::from_value(json_full.clone()).expect("Deserialization failed"); + assert_eq!(request_full.name, "my_table"); + assert_eq!(request_full.location.as_deref(), Some("s3://bucket/table")); + assert!(request_full.partition_spec.is_some()); + assert_eq!(request_full.stage_create, Some(true)); + assert_eq!( + serde_json::to_value(&request_full).expect("Serialization failed"), + json_full + ); + + // Without optional fields — they must be omitted, not null + let json_minimal = serde_json::json!({ + "name": "my_table", + "schema": { + "schema-id": 0, + "type": "struct", + "fields": [ + {"id": 1, "name": "id", "required": true, "type": "int"} + ] + } + }); + let request_minimal: CreateTableRequest = + serde_json::from_value(json_minimal.clone()).expect("Deserialization failed"); + assert_eq!(request_minimal.name, "my_table"); + assert_eq!(request_minimal.location, None); + assert_eq!(request_minimal.partition_spec, None); + assert_eq!(request_minimal.write_order, None); + assert_eq!(request_minimal.stage_create, None); + assert!(request_minimal.properties.is_empty()); + assert_eq!( + serde_json::to_value(&request_minimal).expect("Serialization failed"), + json_minimal + ); + } } diff --git a/crates/iceberg/src/spec/partition.rs b/crates/iceberg/src/spec/partition.rs index 8ffc850a1e..0a6a6aeef6 100644 --- a/crates/iceberg/src/spec/partition.rs +++ b/crates/iceberg/src/spec/partition.rs @@ -856,27 +856,28 @@ mod tests { #[test] fn test_unbound_partition_spec() { + // With all optional fields present let spec = r#" - { - "spec-id": 1, - "fields": [ { - "source-id": 4, - "field-id": 1000, - "name": "ts_day", - "transform": "day" - }, { - "source-id": 1, - "field-id": 1001, - "name": "id_bucket", - "transform": "bucket[16]" - }, { - "source-id": 2, - "field-id": 1002, - "name": "id_truncate", - "transform": "truncate[4]" - } ] - } - "#; + { + "spec-id": 1, + "fields": [ { + "source-id": 4, + "field-id": 1000, + "name": "ts_day", + "transform": "day" + }, { + "source-id": 1, + "field-id": 1001, + "name": "id_bucket", + "transform": "bucket[16]" + }, { + "source-id": 2, + "field-id": 1002, + "name": "id_truncate", + "transform": "truncate[4]" + } ] + } + "#; let partition_spec: UnboundPartitionSpec = serde_json::from_str(spec).unwrap(); assert_eq!(Some(1), partition_spec.spec_id); @@ -896,15 +897,20 @@ mod tests { assert_eq!("id_truncate", partition_spec.fields[2].name); assert_eq!(Transform::Truncate(4), partition_spec.fields[2].transform); + let expected: serde_json::Value = serde_json::from_str(spec).unwrap(); + assert_eq!(serde_json::to_value(&partition_spec).unwrap(), expected); + + // Without optional fields — they must be omitted, not null let spec = r#" - { - "fields": [ { - "source-id": 4, - "name": "ts_day", - "transform": "day" - } ] - } - "#; + { + "fields": [ { + "source-id": 4, + "name": "ts_day", + "transform": "day" + } ] + } + "#; + let partition_spec: UnboundPartitionSpec = serde_json::from_str(spec).unwrap(); assert_eq!(None, partition_spec.spec_id); @@ -912,6 +918,9 @@ mod tests { assert_eq!(None, partition_spec.fields[0].field_id); assert_eq!("ts_day", partition_spec.fields[0].name); assert_eq!(Transform::Day, partition_spec.fields[0].transform); + + let expected: serde_json::Value = serde_json::from_str(spec).unwrap(); + assert_eq!(serde_json::to_value(&partition_spec).unwrap(), expected); } #[test]