Skip to content

Commit 43f1ed8

Browse files
fix: Reserved sort order ID cannot contain any fields (#1978)
## Which issue does this PR close? - Closes #1963. ## What changes are included in this PR? This change validates that table metadata with reserved sort order ID (0) cannot contain fields associated with it. If this is found, we error out instead of silently parsing arbitrary field values. ## Are these changes tested? Added the unit test described in the issue and verified that the check is now enforced.
1 parent 76cdf28 commit 43f1ed8

File tree

1 file changed

+66
-0
lines changed

1 file changed

+66
-0
lines changed

crates/iceberg/src/spec/table_metadata.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,19 @@ impl TableMetadata {
506506

507507
/// If the default sort order is unsorted but the sort order is not present, add it
508508
fn try_normalize_sort_order(&mut self) -> Result<()> {
509+
// Validate that sort order ID 0 (reserved for unsorted) has no fields
510+
if let Some(sort_order) = self.sort_order_by_id(SortOrder::UNSORTED_ORDER_ID)
511+
&& !sort_order.fields.is_empty()
512+
{
513+
return Err(Error::new(
514+
ErrorKind::Unexpected,
515+
format!(
516+
"Sort order ID {} is reserved for unsorted order",
517+
SortOrder::UNSORTED_ORDER_ID
518+
),
519+
));
520+
}
521+
509522
if self.sort_order_by_id(self.default_sort_order_id).is_some() {
510523
return Ok(());
511524
}
@@ -3795,4 +3808,57 @@ mod tests {
37953808
assert!(final_metadata.name_exists_in_any_schema("new_field")); // only in current schema
37963809
assert!(!final_metadata.name_exists_in_any_schema("never_existed"));
37973810
}
3811+
3812+
#[test]
3813+
fn test_invalid_sort_order_id_zero_with_fields() {
3814+
let metadata = r#"
3815+
{
3816+
"format-version": 2,
3817+
"table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c1",
3818+
"location": "s3://bucket/test/location",
3819+
"last-sequence-number": 111,
3820+
"last-updated-ms": 1600000000000,
3821+
"last-column-id": 3,
3822+
"current-schema-id": 1,
3823+
"schemas": [
3824+
{
3825+
"type": "struct",
3826+
"schema-id": 1,
3827+
"fields": [
3828+
{"id": 1, "name": "x", "required": true, "type": "long"},
3829+
{"id": 2, "name": "y", "required": true, "type": "long"}
3830+
]
3831+
}
3832+
],
3833+
"default-spec-id": 0,
3834+
"partition-specs": [{"spec-id": 0, "fields": []}],
3835+
"last-partition-id": 999,
3836+
"default-sort-order-id": 0,
3837+
"sort-orders": [
3838+
{
3839+
"order-id": 0,
3840+
"fields": [
3841+
{
3842+
"transform": "identity",
3843+
"source-id": 1,
3844+
"direction": "asc",
3845+
"null-order": "nulls-first"
3846+
}
3847+
]
3848+
}
3849+
],
3850+
"properties": {},
3851+
"current-snapshot-id": -1,
3852+
"snapshots": []
3853+
}
3854+
"#;
3855+
3856+
let result: Result<TableMetadata, serde_json::Error> = serde_json::from_str(metadata);
3857+
3858+
// Should fail because sort order ID 0 is reserved for unsorted order and cannot have fields
3859+
assert!(
3860+
result.is_err(),
3861+
"Parsing should fail for sort order ID 0 with fields"
3862+
);
3863+
}
37983864
}

0 commit comments

Comments
 (0)