Skip to content

Commit b3519d5

Browse files
fix: StructType fails to deserialize JSON with type field (apache#1822)
This fix enables proper interoperability with Iceberg implementations that include the type discriminator field in their JSON serialization (such as Apache Iceberg Java). ## Which issue does this PR close? Partially address apache#1749. ## What changes are included in this PR? Fixes a bug in the `StructType` deserializer that causes JSON deserialization to fail with the error `expected ',' or '}' at line 1 column 8`. **Root Cause:** The `StructType::Deserialize` implementation was not properly consuming the `"type"` field value when deserializing JSON like: ```json {"type":"struct","fields":[{"id":1,"name":"foo","required":true,"type":"string"}]} ``` In serde's visitor pattern, when you call `map.next_key()`, you must call `map.next_value()` to consume the corresponding value, even if you're discarding it. The deserializer was calling `next_key()` for the "type" field but not consuming its value, causing the parser to remain stuck at the value position and fail when encountering the next field. Changes: - Modified `StructTypeVisitor::visit_map` to properly consume the type field value - Added validation that the type field equals "struct" ## Are these changes tested? Yes, two new unit tests have been added to `crates/iceberg/src/spec/datatypes.rs`: 1. **`struct_type_with_type_field`** - Verifies that `StructType` successfully deserializes from JSON containing the `"type":"struct"` field. This test would fail before the fix with the error `expected ',' or '}' at line 1 column 8` 2. **`struct_type_rejects_wrong_type`** - Validates that the deserializer properly rejects JSON with incorrect type values (e.g., `"type":"list"`) --------- Co-authored-by: Renjie Liu <[email protected]>
1 parent e08bc61 commit b3519d5

File tree

1 file changed

+54
-1
lines changed

1 file changed

+54
-1
lines changed

crates/iceberg/src/spec/datatypes.rs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,15 @@ impl<'de> Deserialize<'de> for StructType {
423423
let mut fields = None;
424424
while let Some(key) = map.next_key()? {
425425
match key {
426-
Field::Type => (),
426+
Field::Type => {
427+
let type_val: String = map.next_value()?;
428+
if type_val != "struct" {
429+
return Err(serde::de::Error::custom(format!(
430+
"expected type 'struct', got '{}'",
431+
type_val
432+
)));
433+
}
434+
}
427435
Field::Fields => {
428436
if fields.is_some() {
429437
return Err(serde::de::Error::duplicate_field("fields"));
@@ -1208,4 +1216,49 @@ mod tests {
12081216
assert!(ty.compatible(&literal));
12091217
}
12101218
}
1219+
1220+
#[test]
1221+
fn struct_type_with_type_field() {
1222+
// Test that StructType properly deserializes JSON with "type":"struct" field
1223+
// This was previously broken because the deserializer wasn't consuming the type field value
1224+
let json = r#"
1225+
{
1226+
"type": "struct",
1227+
"fields": [
1228+
{"id": 1, "name": "field1", "required": true, "type": "string"}
1229+
]
1230+
}
1231+
"#;
1232+
1233+
let struct_type: StructType = serde_json::from_str(json)
1234+
.expect("Should successfully deserialize StructType with type field");
1235+
1236+
assert_eq!(struct_type.fields().len(), 1);
1237+
assert_eq!(struct_type.fields()[0].name, "field1");
1238+
}
1239+
1240+
#[test]
1241+
fn struct_type_rejects_wrong_type() {
1242+
// Test that StructType validation rejects incorrect type field values
1243+
let json = r#"
1244+
{
1245+
"type": "list",
1246+
"fields": [
1247+
{"id": 1, "name": "field1", "required": true, "type": "string"}
1248+
]
1249+
}
1250+
"#;
1251+
1252+
let result = serde_json::from_str::<StructType>(json);
1253+
assert!(
1254+
result.is_err(),
1255+
"Should reject StructType with wrong type field"
1256+
);
1257+
assert!(
1258+
result
1259+
.unwrap_err()
1260+
.to_string()
1261+
.contains("expected type 'struct'")
1262+
);
1263+
}
12111264
}

0 commit comments

Comments
 (0)