Skip to content

Conversation

@lamOrigin007
Copy link

Existing manifest files cannot have the snapshotID of the new snapshot being created.

@lamOrigin007 lamOrigin007 force-pushed the fix-fast-append-producer branch from 67dc6fc to 7937999 Compare December 14, 2025 08:50

for _, m := range manifests {
if m.HasAddedFiles() || m.HasExistingFiles() || m.SnapshotID() == fa.base.snapshotID {
if _, ok := previousSnapshotList[m.SnapshotID()]; m.HasAddedFiles() || m.HasExistingFiles() || ok {
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused, if we're saying that the existing manifests can't contain the current snapshot_id shouldn't this use !ok?

Why are we searching the previous snapshot list? Couldn't we just remove this check entirely and just do if m.HasAddedFiles() || m.HasExistingFiles() { ?

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