laskoviymishka commented on PR #1068: URL: https://github.com/apache/iceberg-go/pull/1068#issuecomment-4444556172
`branch create` and `tag create` use `cat.CommitTable` with `NewSetSnapshotRefUpdate` and only `AssertTableUUID`, which makes `create` an *upsert* rather than a *create* — `iceberg branch create db.t main --snapshot-id 999` silently rewrites `main`. Java's `UpdateSnapshotReferencesOperation` and PyIceberg's `ManageSnapshots.create_branch` both reject existing refs (PyIceberg via `AssertRefSnapshotId(None, ref)`). Suggest matching: pre-flight check on `meta.Refs()` plus `table.AssertRefSnapshotID(name, nil)` in the requirements slice so REST catalogs enforce it server-side too. Related — these are the only write commands in the series without a `--yes` / `confirmAction` gate. `confirmAction` already exists in `maintenance.go` from #1073, so adding it keeps the pattern uniform and gives users an out on the overwrite case until the existence check lands. `TagCreateCmd` correctly omits the branch-only retention flags, so the spec-violation surface is library-internal — worth a separate guard in `table/metadata.go`'s `WithMaxSnapshotAgeMs` / `WithMinSnapshotsToKeep` (Java enforces `Tags do not support setting minSnapshotsToKeep`) but that can land as a follow-up. Test coverage is solid for what exists; integration vs the Docker REST catalog is the standing gap shared with the rest of the series. Inline comments left on the specific lines. -- 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]
