Skip to content

Conversation

@imor
Copy link
Contributor

@imor imor commented Oct 20, 2025

Which issue does this PR close?

  • Closes #. There was no open issue for this. I noticed this while trying out iceberg-rust.

What changes are included in this PR?

The first line in the method SnapshotProducer::write_added_manifest (let added_data_files = std::mem::take(&mut self.added_data_files);) sets the self.added_data_files to an empty vec. SnapshotProducer::write_added_manifest is called via the call chain SnapshotProducer::commit -> SnapshotProducer::manifest_file -> SnapshotProducer::write_added_manifest). Hence, if SnapshotProducer::summary is called after calling SnapshotProducer::manifest_file, the summary produced was empty due to empty self.added_data_files.

This PR rearranges the code in SnapshotProducer::commit to make sure the produced summary is not empty.

Are these changes tested?

No new tests have been added for this as there were no previous tests.

@imor imor marked this pull request as ready for review October 20, 2025 12:26
Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

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

Thanks @imor for this fix!

@liurenjie1024 liurenjie1024 merged commit 3aa7ade into apache:main Oct 22, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants