Skip to content

Commit

Permalink
RemovePartitionSpecs update
Browse files Browse the repository at this point in the history
  • Loading branch information
c-thiel committed Dec 15, 2024
1 parent 394cb7e commit e8340c7
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 0 deletions.
22 changes: 22 additions & 0 deletions crates/iceberg/src/catalog/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,12 @@ pub enum TableUpdate {
/// Snapshot id to remove partition statistics for.
snapshot_id: i64,
},
/// Remove partition specs
#[serde(rename_all = "kebab-case")]
RemovePartitionSpecs {
/// Partition spec ids to remove.
spec_ids: Vec<i32>,
},
}

impl TableUpdate {
Expand Down Expand Up @@ -510,6 +516,9 @@ impl TableUpdate {
TableUpdate::RemovePartitionStatistics { snapshot_id } => {
Ok(builder.remove_partition_statistics(snapshot_id))
}
TableUpdate::RemovePartitionSpecs { spec_ids } => {
Ok(builder.remove_partition_specs(&spec_ids))
}
}
}
}
Expand Down Expand Up @@ -2019,4 +2028,17 @@ mod tests {
},
)
}
fn test_remove_partition_specs_update() {
test_serde_json(
r#"
{
"action": "remove-partition-specs",
"spec-ids": [1, 2]
}
"#,
TableUpdate::RemovePartitionSpecs {
spec_ids: vec![1, 2],
},
);
}
}
53 changes: 53 additions & 0 deletions crates/iceberg/src/spec/table_metadata_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,32 @@ impl TableMetadataBuilder {
.set_default_partition_spec(Self::LAST_ADDED)
}

/// Remove partition specs by their ids from the table metadata.
/// Does nothing if a spec id is not present.
///
/// If the default partition spec is removed, it is re-added
/// upon build.
pub fn remove_partition_specs(mut self, spec_ids: &[i32]) -> Self {
let mut removed_specs = Vec::with_capacity(spec_ids.len());

self.metadata.partition_specs.retain(|k, _| {
if spec_ids.contains(k) {
removed_specs.push(*k);
false
} else {
true
}
});

if !removed_specs.is_empty() {
self.changes.push(TableUpdate::RemovePartitionSpecs {
spec_ids: removed_specs,
});
}

self
}

/// Add a sort order to the table metadata.
///
/// The spec is bound eagerly to the current schema and must be valid for it.
Expand Down Expand Up @@ -1592,6 +1618,20 @@ mod tests {
pretty_assertions::assert_eq!(build_result.changes[0], TableUpdate::AddSpec {
spec: expected_change
});

// Remove the spec
let build_result = build_result
.metadata
.into_builder(Some(
"s3://bucket/test/location/metadata/metadata1.json".to_string(),
))
.remove_partition_specs(&[1])
.build()
.unwrap();

assert_eq!(build_result.changes.len(), 1);
assert_eq!(build_result.metadata.partition_specs.len(), 1);
assert!(build_result.metadata.partition_spec_by_id(1).is_none());
}

#[test]
Expand Down Expand Up @@ -2297,5 +2337,18 @@ mod tests {
.unwrap();
assert_eq!(build_result.metadata.partition_statistics.len(), 0);
assert_eq!(build_result.changes.len(), 0);
fn test_default_spec_cannot_be_removed() {
let builder = builder_without_changes(FormatVersion::V2);

let metadata = builder
.remove_partition_specs(&[0])
.build()
.unwrap()
.metadata;

assert_eq!(metadata.default_spec.spec_id(), 0);
assert_eq!(metadata.partition_specs.len(), 1);
assert!(metadata.partition_spec_by_id(0).is_some());
}
}
}

0 comments on commit e8340c7

Please sign in to comment.