Skip to content

refactor(encryption) remove key_metadata from the constructor of an unencrypted manifest writer #2666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blackmwk merged 3 commits into from
Jul 1, 2026
Merged

refactor(encryption) remove key_metadata from the constructor of an unencrypted manifest writer #2666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da91f32
refactor(encryption) remove key_metadata from the constructor of an u…
xanderbailey Jun 17, 2026
8edfb3c
comment
xanderbailey Jun 17, 2026
fb5df7e
remove from fast append
xanderbailey Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/iceberg/public-api.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2078,7 +2078,7 @@ pub fn iceberg::spec::ManifestWriterBuilder::build_v2_data(self) -> iceberg::spe
pub fn iceberg::spec::ManifestWriterBuilder::build_v2_deletes(self) -> iceberg::spec::ManifestWriter
pub fn iceberg::spec::ManifestWriterBuilder::build_v3_data(self) -> iceberg::spec::ManifestWriter
pub fn iceberg::spec::ManifestWriterBuilder::build_v3_deletes(self) -> iceberg::spec::ManifestWriter
pub fn iceberg::spec::ManifestWriterBuilder::new(output: iceberg::io::OutputFile, snapshot_id: core::option::Option<i64>, key_metadata: core::option::Option<alloc::vec::Vec<u8>>, schema: iceberg::spec::SchemaRef, partition_spec: iceberg::spec::PartitionSpec) -> Self
pub fn iceberg::spec::ManifestWriterBuilder::new(output: iceberg::io::OutputFile, snapshot_id: core::option::Option<i64>, schema: iceberg::spec::SchemaRef, partition_spec: iceberg::spec::PartitionSpec) -> Self
pub fn iceberg::spec::ManifestWriterBuilder::new_from_encrypted(encrypted_output: iceberg::encryption::EncryptedOutputFile, snapshot_id: core::option::Option<i64>, key_metadata: core::option::Option<alloc::vec::Vec<u8>>, schema: iceberg::spec::SchemaRef, partition_spec: iceberg::spec::PartitionSpec) -> Self
pub struct iceberg::spec::Map
impl iceberg::spec::Map
Expand Down
1 change: 0 additions & 1 deletion crates/iceberg/src/io/object_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
self.next_manifest_file(),
Some(current_snapshot.snapshot_id()),
None,
current_schema.clone(),
current_partition_spec.as_ref().clone(),
)
Expand Down
4 changes: 0 additions & 4 deletions crates/iceberg/src/scan/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,6 @@ pub mod tests {
let mut writer = ManifestWriterBuilder::new(
self.next_manifest_file(),
Some(current_snapshot.snapshot_id()),
None,
current_schema.clone(),
current_partition_spec.as_ref().clone(),
)
Expand Down Expand Up @@ -1054,7 +1053,6 @@ pub mod tests {
let mut writer = ManifestWriterBuilder::new(
self.next_manifest_file(),
Some(current_snapshot.snapshot_id()),
None,
current_schema.clone(),
current_partition_spec.as_ref().clone(),
)
Expand Down Expand Up @@ -1165,7 +1163,6 @@ pub mod tests {
let mut writer = ManifestWriterBuilder::new(
self.next_manifest_file(),
Some(current_snapshot.snapshot_id()),
None,
current_schema.clone(),
current_partition_spec.as_ref().clone(),
)
Expand Down Expand Up @@ -1200,7 +1197,6 @@ pub mod tests {
let mut writer = ManifestWriterBuilder::new(
self.next_manifest_file(),
Some(current_snapshot.snapshot_id()),
None,
current_schema.clone(),
current_partition_spec.as_ref().clone(),
)
Expand Down
6 changes: 0 additions & 6 deletions crates/iceberg/src/spec/manifest/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(1),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down Expand Up @@ -572,7 +571,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(2),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down Expand Up @@ -669,7 +667,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(3),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down Expand Up @@ -778,7 +775,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(2),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down Expand Up @@ -886,7 +882,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(2),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down Expand Up @@ -1165,7 +1160,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(1),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down
7 changes: 2 additions & 5 deletions crates/iceberg/src/spec/manifest/writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,10 @@ pub struct ManifestWriterBuilder {
}

impl ManifestWriterBuilder {
/// Create a new builder.
/// Create a new builder for unencrypted manifests.
pub fn new(
Comment thread
blackmwk marked this conversation as resolved.
output: OutputFile,
snapshot_id: Option<i64>,
key_metadata: Option<Vec<u8>>,
schema: SchemaRef,
partition_spec: PartitionSpec,
) -> Self {
Expand All @@ -69,7 +68,7 @@ impl ManifestWriterBuilder {
writer_future: Box::pin(async move { output.writer().await }),
location,
snapshot_id,
key_metadata,
key_metadata: None,
schema,
partition_spec,
}
Expand Down Expand Up @@ -732,7 +731,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(3),
None,
metadata.schema.clone(),
metadata.partition_spec.clone(),
)
Expand Down Expand Up @@ -820,7 +818,6 @@ mod tests {
let mut writer = ManifestWriterBuilder::new(
output_file,
Some(1),
None,
schema.clone(),
partition_spec.clone(),
)
Expand Down
11 changes: 0 additions & 11 deletions crates/iceberg/src/transaction/append.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ pub struct FastAppendAction {
check_duplicate: bool,
// below are properties used to create SnapshotProducer when commit
commit_uuid: Option<Uuid>,
key_metadata: Option<Vec<u8>>,
snapshot_properties: HashMap<String, String>,
added_data_files: Vec<DataFile>,
}
Expand All @@ -44,7 +43,6 @@ impl FastAppendAction {
Self {
check_duplicate: true,
commit_uuid: None,
key_metadata: None,
snapshot_properties: HashMap::default(),
added_data_files: vec![],
}
Expand All @@ -68,12 +66,6 @@ impl FastAppendAction {
self
}

/// Set key metadata for manifest files.
pub fn set_key_metadata(mut self, key_metadata: Vec<u8>) -> Self {
self.key_metadata = Some(key_metadata);
self
}

/// Set snapshot summary properties.
pub fn set_snapshot_properties(mut self, snapshot_properties: HashMap<String, String>) -> Self {
self.snapshot_properties = snapshot_properties;
Expand All @@ -87,7 +79,6 @@ impl TransactionAction for FastAppendAction {
let snapshot_producer = SnapshotProducer::new(
table,
self.commit_uuid.unwrap_or_else(Uuid::now_v7),
self.key_metadata.clone(),
self.snapshot_properties.clone(),
self.added_data_files.clone(),
);
Expand Down Expand Up @@ -233,7 +224,6 @@ mod tests {
let mut data_writer = ManifestWriterBuilder::new(
next_manifest_file(&table_location_str),
Some(current_snapshot.snapshot_id()),
None,
schema.clone(),
partition_spec.as_ref().clone(),
)
Expand Down Expand Up @@ -263,7 +253,6 @@ mod tests {
let mut delete_writer = ManifestWriterBuilder::new(
next_manifest_file(&table_location_str),
Some(current_snapshot.snapshot_id()),
None,
schema.clone(),
partition_spec.as_ref().clone(),
)
Expand Down
4 changes: 0 additions & 4 deletions crates/iceberg/src/transaction/snapshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ pub(crate) struct SnapshotProducer<'a> {
pub(crate) table: &'a Table,
snapshot_id: i64,
commit_uuid: Uuid,
key_metadata: Option<Vec<u8>>,
snapshot_properties: HashMap<String, String>,
added_data_files: Vec<DataFile>,
// A counter used to generate unique manifest file names.
Expand All @@ -126,15 +125,13 @@ impl<'a> SnapshotProducer<'a> {
pub(crate) fn new(
table: &'a Table,
commit_uuid: Uuid,
key_metadata: Option<Vec<u8>>,
snapshot_properties: HashMap<String, String>,
added_data_files: Vec<DataFile>,
) -> Self {
Self {
table,
snapshot_id: Self::generate_unique_snapshot_id(table),
commit_uuid,
key_metadata,
snapshot_properties,
added_data_files,
manifest_counter: (0..),
Expand Down Expand Up @@ -255,7 +252,6 @@ impl<'a> SnapshotProducer<'a> {
let builder = ManifestWriterBuilder::new(
output_file,
Some(self.snapshot_id),
self.key_metadata.clone(),

@xanderbailey xanderbailey Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The metadata here comes from the fast append action which I think is also wrong but I'll remove that in a further follow up PR to keep these refactors targeted.

@xanderbailey xanderbailey Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh metadata becomes dead code on snapshot producer so maybe I will remove here.

self.table.metadata().current_schema().clone(),
self.table
.metadata()
Expand Down
Loading