zeroshade commented on code in PR #575:
URL: https://github.com/apache/iceberg-go/pull/575#discussion_r2370310131
##########
table/metadata.go:
##########
@@ -380,6 +379,14 @@ func (b *MetadataBuilder) RemoveSnapshots(snapshotIds
[]int64) error {
b.snapshotLog = slices.DeleteFunc(b.snapshotLog, func(e
SnapshotLogEntry) bool {
return slices.Contains(snapshotIds, e.SnapshotID)
})
+
+ newRefs := make(map[string]SnapshotRef)
+ for name, ref := range b.refs {
+ if _, err := b.SnapshotByID(ref.SnapshotID); err == nil {
+ newRefs[name] = ref
+ }
+ }
Review Comment:
why build this map? we just drop it and don't do anything with it
##########
table/metadata.go:
##########
@@ -354,11 +354,10 @@ func (b *MetadataBuilder) AddSnapshot(snapshot *Snapshot)
error {
} else if s, _ := b.SnapshotByID(snapshot.SnapshotID); s != nil {
return fmt.Errorf("can't add snapshot with id %d, already
exists", snapshot.SnapshotID)
} else if b.formatVersion == 2 &&
- snapshot.SequenceNumber > 0 &&
snapshot.ParentSnapshotID != nil &&
snapshot.SequenceNumber <= *b.lastSequenceNumber {
return fmt.Errorf("can't add snapshot with sequence number %d,
must be > than last sequence number %d",
- snapshot.SequenceNumber, b.lastSequenceNumber)
+ snapshot.SequenceNumber, *b.lastSequenceNumber)
Review Comment:
are we sure this can't be nil here? Should we have a nil check?
##########
table/metadata.go:
##########
@@ -1313,6 +1366,14 @@ func (c *commonMetadata) validate() error {
c.constructRefs()
+ if err := c.checkMainRefMatchesCurrentSnapshot(); err != nil {
+ return err
+ }
Review Comment:
is this a requirement in the spec? That the `main` ref must be the current
snapshot?
--
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]
