Skip to content

Conversation

@dentiny
Copy link
Contributor

@dentiny dentiny commented Jul 21, 2025

Which issue does this PR close?

What changes are included in this PR?

I have a real-world production requirement: I need the capability to add snapshot properties to a new snapshot, whether or not there's new data files appended at the same time.

In this PR, I tried to reuse the FastAppend interface, which allows people only added properties but not data files.
I don't find other available interfaces at the moment.

Are these changes tested?

Unit tested added.

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 @dentiny for this pr, generally LGTM!

) -> Result<Vec<ManifestFile>> {
let added_manifest = self.write_added_manifest().await?;
// Assert current snapshot producer contains new content to add to new snapshot.
if self.added_data_files.is_empty() && self.snapshot_properties.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create an issue to track that we need to remove this workaround when we introduced all necessary tx apis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I created an issue to track: #1548
Also leave TODO items in the code along with the issue attached.

@dentiny dentiny requested a review from liurenjie1024 July 23, 2025 10:57
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 @dentiny for this pr, LGTM!

@liurenjie1024 liurenjie1024 merged commit b3ea8d1 into apache:main Jul 23, 2025
17 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.

Feature request: support set snapshot property

2 participants