Skip to content

Commit b78542f

Browse files
authored
Make key-metadata optional (#1434)
## Which issue does this PR close? This aligns more closely with the spec: <img width="708" alt="image" src="https://github.com/user-attachments/assets/9691a919-6fb0-4be8-b5d3-8e88cc081c98" /> - Closes #. ## What changes are included in this PR? ## Are these changes tested?
1 parent cd103fb commit b78542f

File tree

5 files changed

+19
-31
lines changed

5 files changed

+19
-31
lines changed

Cargo.lock

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/iceberg/src/expr/visitors/manifest_evaluator.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ mod test {
661661
existing_rows_count: None,
662662
deleted_rows_count: None,
663663
partitions: Some(partitions),
664-
key_metadata: vec![],
664+
key_metadata: None,
665665
}
666666
}
667667

crates/iceberg/src/spec/manifest/writer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ impl ManifestWriter {
416416
existing_rows_count: Some(self.existing_rows),
417417
deleted_rows_count: Some(self.deleted_rows),
418418
partitions: Some(partition_summary),
419-
key_metadata: self.key_metadata,
419+
key_metadata: Some(self.key_metadata),
420420
})
421421
}
422422
}

crates/iceberg/src/spec/manifest_list.rs

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ pub struct ManifestFile {
579579
/// field: 519
580580
///
581581
/// Implementation-specific key metadata for encryption
582-
pub key_metadata: Vec<u8>,
582+
pub key_metadata: Option<Vec<u8>>,
583583
}
584584

585585
impl ManifestFile {
@@ -830,7 +830,7 @@ pub(super) mod _serde {
830830
existing_rows_count: Some(self.existing_rows_count.try_into()?),
831831
deleted_rows_count: Some(self.deleted_rows_count.try_into()?),
832832
partitions: self.partitions,
833-
key_metadata: self.key_metadata.map(|b| b.into_vec()).unwrap_or_default(),
833+
key_metadata: self.key_metadata.map(|b| b.into_vec()),
834834
})
835835
}
836836
}
@@ -862,7 +862,7 @@ pub(super) mod _serde {
862862
.transpose()?,
863863
deleted_rows_count: self.deleted_rows_count.map(TryInto::try_into).transpose()?,
864864
partitions: self.partitions,
865-
key_metadata: self.key_metadata.map(|b| b.into_vec()).unwrap_or_default(),
865+
key_metadata: self.key_metadata.map(|b| b.into_vec()),
866866
// as ref: https://iceberg.apache.org/spec/#partitioning
867867
// use 0 when reading v1 manifest lists
868868
content: super::ManifestContentType::Data,
@@ -872,11 +872,10 @@ pub(super) mod _serde {
872872
}
873873
}
874874

875-
fn convert_to_serde_key_metadata(key_metadata: Vec<u8>) -> Option<ByteBuf> {
876-
if key_metadata.is_empty() {
877-
None
878-
} else {
879-
Some(ByteBuf::from(key_metadata))
875+
fn convert_to_serde_key_metadata(key_metadata: Option<Vec<u8>>) -> Option<ByteBuf> {
876+
match key_metadata {
877+
Some(metadata) if !metadata.is_empty() => Some(ByteBuf::from(metadata)),
878+
_ => None,
880879
}
881880
}
882881

@@ -1025,7 +1024,7 @@ mod test {
10251024
existing_rows_count: Some(0),
10261025
deleted_rows_count: Some(0),
10271026
partitions: Some(vec![]),
1028-
key_metadata: vec![],
1027+
key_metadata: None,
10291028
}
10301029
]
10311030
};
@@ -1076,7 +1075,7 @@ mod test {
10761075
partitions: Some(
10771076
vec![FieldSummary { contains_null: false, contains_nan: Some(false), lower_bound: Some(Datum::long(1).to_bytes().unwrap()), upper_bound: Some(Datum::long(1).to_bytes().unwrap())}]
10781077
),
1079-
key_metadata: vec![],
1078+
key_metadata: None,
10801079
},
10811080
ManifestFile {
10821081
manifest_path: "s3a://icebergdata/demo/s1/t1/metadata/05ffe08b-810f-49b3-a8f4-e88fc99b254a-m1.avro".to_string(),
@@ -1095,7 +1094,7 @@ mod test {
10951094
partitions: Some(
10961095
vec![FieldSummary { contains_null: false, contains_nan: Some(false), lower_bound: Some(Datum::float(1.1).to_bytes().unwrap()), upper_bound: Some(Datum::float(2.1).to_bytes().unwrap())}]
10971096
),
1098-
key_metadata: vec![],
1097+
key_metadata: None,
10991098
}
11001099
]
11011100
};
@@ -1144,7 +1143,7 @@ mod test {
11441143
existing_rows_count: Some(0),
11451144
deleted_rows_count: Some(0),
11461145
partitions: None,
1147-
key_metadata: vec![],
1146+
key_metadata: None,
11481147
}]
11491148
}.try_into().unwrap();
11501149
let result = serde_json::to_string(&manifest_list).unwrap();
@@ -1174,7 +1173,7 @@ mod test {
11741173
partitions: Some(
11751174
vec![FieldSummary { contains_null: false, contains_nan: Some(false), lower_bound: Some(Datum::long(1).to_bytes().unwrap()), upper_bound: Some(Datum::long(1).to_bytes().unwrap())}]
11761175
),
1177-
key_metadata: vec![],
1176+
key_metadata: None,
11781177
}]
11791178
}.try_into().unwrap();
11801179
let result = serde_json::to_string(&manifest_list).unwrap();
@@ -1204,7 +1203,7 @@ mod test {
12041203
partitions: Some(
12051204
vec![FieldSummary { contains_null: false, contains_nan: Some(false), lower_bound: Some(Datum::long(1).to_bytes().unwrap()), upper_bound: Some(Datum::long(1).to_bytes().unwrap())}],
12061205
),
1207-
key_metadata: vec![],
1206+
key_metadata: None,
12081207
}]
12091208
};
12101209

@@ -1250,7 +1249,7 @@ mod test {
12501249
partitions: Some(
12511250
vec![FieldSummary { contains_null: false, contains_nan: Some(false), lower_bound: Some(Datum::long(1).to_bytes().unwrap()), upper_bound: Some(Datum::long(1).to_bytes().unwrap())}]
12521251
),
1253-
key_metadata: vec![],
1252+
key_metadata: None,
12541253
}]
12551254
};
12561255

@@ -1295,7 +1294,7 @@ mod test {
12951294
partitions: Some(
12961295
vec![FieldSummary { contains_null: false, contains_nan: Some(false), lower_bound: Some(Datum::long(1).to_bytes().unwrap()), upper_bound: Some(Datum::long(1).to_bytes().unwrap())}]
12971296
),
1298-
key_metadata: vec![],
1297+
key_metadata: None,
12991298
}]
13001299
};
13011300

crates/iceberg/src/spec/snapshot_summary.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ mod tests {
849849
existing_rows_count: Some(0),
850850
deleted_rows_count: Some(50),
851851
partitions: Some(Vec::new()),
852-
key_metadata: Vec::new(),
852+
key_metadata: None,
853853
};
854854

855855
collector
@@ -973,7 +973,7 @@ mod tests {
973973
existing_rows_count: Some(0),
974974
deleted_rows_count: Some(0),
975975
partitions: Some(Vec::new()),
976-
key_metadata: Vec::new(),
976+
key_metadata: None,
977977
});
978978

979979
summary_four.add_file(

0 commit comments

Comments
 (0)