Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 1 addition & 5 deletions crates/iceberg/src/spec/manifest/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,6 @@ impl ManifestWriter {
/// Add a delete manifest entry. This method will update following status of the entry:
/// - Update the entry status to `Deleted`
/// - Set the snapshot id to the current snapshot id
///
/// # TODO
/// Remove this allow later
#[allow(dead_code)]
pub(crate) fn add_delete_entry(&mut self, mut entry: ManifestEntry) -> Result<()> {
self.check_data_file(&entry.data_file)?;
entry.status = ManifestStatus::Deleted;
Expand Down Expand Up @@ -345,7 +341,7 @@ impl ManifestWriter {
Ok(())
}

/// Add an file as existing manifest entry. The original data and file sequence numbers, snapshot ID,
/// Add a file as existing manifest entry. The original data and file sequence numbers, snapshot ID,
/// which were assigned at commit, must be preserved when adding an existing entry.
pub fn add_existing_file(
&mut self,
Expand Down
2 changes: 1 addition & 1 deletion crates/iceberg/src/spec/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub const MAIN_BRANCH: &str = "main";

/// Reference to [`Snapshot`].
pub type SnapshotRef = Arc<Snapshot>;
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone)]
#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, Hash)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this need to be Hash now?

#[serde(rename_all = "lowercase")]
/// The operation field is used by some operations, like snapshot expiration, to skip processing certain snapshots.
pub enum Operation {
Expand Down
16 changes: 8 additions & 8 deletions crates/iceberg/src/transaction/append.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ impl FastAppendAction {
#[async_trait]
impl TransactionAction for FastAppendAction {
async fn commit(self: Arc<Self>, table: &Table) -> Result<ActionCommit> {
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(),
);
let snapshot_producer = SnapshotProducer::builder()
.with_table(table)
.with_commit_uuid(self.commit_uuid.unwrap_or_else(Uuid::now_v7))
.with_key_metadata(self.key_metadata.clone())
.with_snapshot_properties(self.snapshot_properties.clone())
.with_added_data_files(self.added_data_files.clone())
.build();

Comment on lines -87 to 94
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice change

// validate added files
snapshot_producer.validate_added_data_files()?;
Expand Down Expand Up @@ -122,7 +122,7 @@ impl SnapshotProduceOperation for FastAppendOperation {

async fn existing_manifest(
&self,
snapshot_produce: &SnapshotProducer<'_>,
snapshot_produce: &mut SnapshotProducer<'_>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this need to be mut?

) -> Result<Vec<ManifestFile>> {
let Some(snapshot) = snapshot_produce.table.metadata().current_snapshot() else {
return Ok(vec![]);
Expand Down
Loading
Loading