Skip to content

fix(transaction): fix previous snapshot summary fetching#2391

Merged
CTTY merged 4 commits intoapache:mainfrom
dentiny:hjiang/fix-snapshot-summary
May 9, 2026
Merged

fix(transaction): fix previous snapshot summary fetching#2391
CTTY merged 4 commits intoapache:mainfrom
dentiny:hjiang/fix-snapshot-summary

Conversation

@dentiny
Copy link
Copy Markdown
Contributor

@dentiny dentiny commented May 1, 2026

Which issue does this PR close?

What changes are included in this PR?

The root cause for the issue is, we use the current snapshot id (which hasn't been reflected to table metadata yet) to get previous snapshot summary.

Are these changes tested?

Yes.

@dentiny
Copy link
Copy Markdown
Contributor Author

dentiny commented May 1, 2026

The failure doesn't seem to be related to my change.
image

@timactive
Copy link
Copy Markdown

+1

fix, `total-records` of the new snapshot was set to `added-records`
(=2 for a 2-row delta) instead of `parent.total + added` (=484065).
With the patch applied locally, the snapshot summary correctly
reports the cumulative and downstream consumers reading metadata.json
directly (iceberg-java in our case) see the right value on the next
commit. 36/36 snapshot tests stay green.

Edge case for the docs / future follow-up: tables with history
already corrupted by the buggy version will keep propagating wrong
totals because parent.total-records is read as-is. In practice
re-bootstrapping unblocks; a chain-walk recompute could self-heal
if needed, but it's not warranted for this PR. Thanks for the fix!```

@dentiny
Copy link
Copy Markdown
Contributor Author

dentiny commented May 8, 2026

Hi @Xuanwo I'm wondering if you could take a look when you have some time? Thank you!

Copy link
Copy Markdown
Collaborator

@CTTY CTTY left a comment

Choose a reason for hiding this comment

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

LGTM! just one comment

Comment thread crates/iceberg/src/transaction/mod.rs Outdated
@dentiny dentiny requested a review from CTTY May 8, 2026 21:45
Copy link
Copy Markdown
Collaborator

@CTTY CTTY left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix!

@CTTY CTTY merged commit 4b13ad5 into apache:main May 9, 2026
20 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.

BUG: snapshot summary doesn't consider previous summary

3 participants