1fanwang commented on code in PR #2057:
URL: https://github.com/apache/iceberg-go/pull/2057#discussion_r4162556064


##########
table/snapshot_producers.go:
##########
@@ -543,6 +566,13 @@ func (m *manifestMergeManager) createManifest(specID int, 
bin []iceberg.Manifest
                }
        }
 
+       // A bin with no live entries produces no manifest. This diverges from
+       // Java/PyIceberg, which write a zero-count manifest; the omission is

Review Comment:
   Done in 
https://github.com/apache/iceberg-go/commit/4e958128577cf5ba785051ae0e3b6b09f481b8ef.
 The comment now names only the Java reference implementation.
   



##########
table/snapshot_producers.go:
##########
@@ -1762,16 +1795,13 @@ func (sp *snapshotProducer) 
commitManifests(newManifests, addedContent []iceberg
        // creates it).
        baseHeadID := sp.txn.baseRefSnapshotID(branch)
 
-       return []Update{
-                       addSnap,
-                       // Carry over the branch's existing retention settings 
so advancing
-                       // the ref on commit does not silently discard them. 
The update
-                       // encodes exactly the current ref's retention 
(settings the branch
-                       // lacks stay 0 and are dropped by the `omitempty` 
tags); the catalog
-                       // applies a set-snapshot-ref as a pure replace, so 
this fully
-                       // determines the resulting ref rather than merging 
with the old one.
-                       sp.txn.meta.NewRetainingSnapshotRefUpdate(branch, 
sp.snapshotID, BranchRef),
-               }, []Requirement{
-                       AssertRefSnapshotID(branch, baseHeadID),
-               }, nil
+       // Carry over the branch's existing retention settings so advancing
+       // the ref on commit does not silently discard them. The update
+       // encodes exactly the current ref's retention (settings the branch
+       // lacks stay 0 and are dropped by the `omitempty` tags); the catalog
+       // applies a set-snapshot-ref as a pure replace, so this fully
+       // determines the resulting ref rather than merging with the old one.
+       retainingSnapshotRef := 
sp.txn.meta.NewRetainingSnapshotRefUpdate(branch, sp.snapshotID, BranchRef)

Review Comment:
   Done in 
https://github.com/apache/iceberg-go/commit/dcc96bbbaa851bbfc2e66eaa507c297fe54b39b5.
 The PR diff no longer has the reflow or the trailing-comma changes.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to