zeroshade commented on code in PR #2109:
URL: https://github.com/apache/iceberg-go/pull/2109#discussion_r4198577118


##########
table/metadata.go:
##########
@@ -1509,10 +1509,13 @@ func (b *MetadataBuilder) buildCommonMetadata() 
(*commonMetadata, error) {
        }
        defaultSpecID := b.defaultSpecID
 
-       if err := b.updateSnapshotLog(); err != nil {
+       snapshotLog, err := b.updatedSnapshotLog()
+       if err != nil {
                return nil, fmt.Errorf("%w: %w", ErrInvalidMetadata, err)
        }
 
+       // The timestamp is fixed by the first Build and kept on the builder on 
purpose, so
+       // repeated builds of the same changes report the same last-updated 
time.

Review Comment:
   Keep it cached. The failure described in the thread doesn't happen. 
`addSnapshotInternal` and `SetSnapshotRef` on `main` both overwrite 
`b.lastUpdatedMS`, so build → `AddSnapshot` at `t1+5s` → `SetSnapshotRef(main)` 
→ build reports `t1+5s`. Only changes that neither add a snapshot nor move 
`main` reuse the first build's timestamp, and that only affects staged 
metadata. Commits rebuild from base through `UpdateTableMetadata`, and its 
`MetadataBuilderFromBase` starts at `lastUpdatedMS = 0`. Java's 
`TableMetadata.Builder.build()` caches it the same way.
   
   I'd still reword the comment so it says when this is set:
   
   ```suggestion
        // If no change has set lastUpdatedMS yet, the first Build sets it and 
keeps it on
        // the builder on purpose (Java's TableMetadata.Builder does the same), 
so repeated
        // builds of the same changes report the same time. Adding a snapshot 
or moving
        // main after a Build still updates it.
   ```
   
   <sub>Drafted with an AI-assisted tool; confirmed by a maintainer.</sub>



-- 
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