(iceberg-go) branch main updated: fix(table): include expected vs found snapshot id in AssertRefSnapshotID errors (#940) (#952)

[ze...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git


The following commit(s) were added to refs/heads/main by this push:
     new f925806a fix(table): include expected vs found snapshot id in AssertRefSnapshotID errors (#940) (#952)
f925806a is described below

commit f925806aa7ba290f2536ad776c5e9d9a936639fc
Author: Tanmay Rauth <t_...@apple.com>
AuthorDate: Thu Apr 30 09:27:51 2026 -0700

    fix(table): include expected vs found snapshot id in AssertRefSnapshotID errors (#940) (#952)
    
    Format ref name, expected snapshot id, and found snapshot id into all
    three error paths so operators can see what changed during commit
    conflicts. Also fixes incorrect pointer formatting with %d.
    
    Fixes #940
---
 table/requirement_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 table/requirements.go     |  6 +++---
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/table/requirement_test.go b/table/requirement_test.go
index 2fe4e23b..8f15f709 100644
--- a/table/requirement_test.go
+++ b/table/requirement_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/apache/iceberg-go/table"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestParseRequirementBytes(t *testing.T) {
@@ -145,3 +146,50 @@ func TestParseRequirementList(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestAssertRefSnapshotIDValidate(t *testing.T) {
+	meta, err := table.ParseMetadataBytes([]byte(table.ExampleTableMetadataV2))
+	require.NoError(t, err)
+
+	t.Run("matching ref passes", func(t *testing.T) {
+		req := table.AssertRefSnapshotID("test", ptr(int64(3051729675574597004)))
+		assert.NoError(t, req.Validate(meta))
+	})
+
+	t.Run("mismatched snapshot id includes expected and found", func(t *testing.T) {
+		req := table.AssertRefSnapshotID("test", ptr(int64(1)))
+		err := req.Validate(meta)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `"test"`)
+		assert.Contains(t, err.Error(), "expected id 1")
+		assert.Contains(t, err.Error(), "found 3051729675574597004")
+		assert.Contains(t, err.Error(), "has changed")
+	})
+
+	t.Run("nil expected but ref exists includes found snapshot", func(t *testing.T) {
+		req := table.AssertRefSnapshotID("test", nil)
+		err := req.Validate(meta)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `"test"`)
+		assert.Contains(t, err.Error(), "was created concurrently")
+	})
+
+	t.Run("ref missing but expected includes expected snapshot", func(t *testing.T) {
+		req := table.AssertRefSnapshotID("nonexistent", ptr(int64(42)))
+		err := req.Validate(meta)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `"nonexistent"`)
+		assert.Contains(t, err.Error(), "is missing")
+		assert.Contains(t, err.Error(), "expected 42")
+	})
+
+	t.Run("nil metadata returns error", func(t *testing.T) {
+		req := table.AssertRefSnapshotID("main", ptr(int64(1)))
+		assert.Error(t, req.Validate(nil))
+	})
+
+	t.Run("nil expected and ref missing passes", func(t *testing.T) {
+		req := table.AssertRefSnapshotID("nonexistent", nil)
+		assert.NoError(t, req.Validate(meta))
+	})
+}
diff --git a/table/requirements.go b/table/requirements.go
index 9ce85bc9..7b70f464 100644
--- a/table/requirements.go
+++ b/table/requirements.go
@@ -180,14 +180,14 @@ func (a *assertRefSnapshotID) Validate(meta Metadata) error {
 
 	if r != nil {
 		if a.SnapshotID == nil {
-			return fmt.Errorf("requirement failed: %s %s was created concurrently", r.SnapshotRefType, a.Ref)
+			return fmt.Errorf("requirement failed: %s %q was created concurrently", r.SnapshotRefType, a.Ref)
 		}
 
 		if r.SnapshotID != *a.SnapshotID {
-			return fmt.Errorf("requirement failed: %s %s has changed: expected id %d, found %d", r.SnapshotRefType, a.Ref, a.SnapshotID, r.SnapshotID)
+			return fmt.Errorf("requirement failed: %s %q has changed: expected id %d, found %d", r.SnapshotRefType, a.Ref, *a.SnapshotID, r.SnapshotID)
 		}
 	} else if a.SnapshotID != nil {
-		return fmt.Errorf("requirement failed: branch or tag %s is missing, expected %d", a.Ref, a.SnapshotID)
+		return fmt.Errorf("requirement failed: branch or tag %q is missing, expected %d", a.Ref, *a.SnapshotID)
 	}
 
 	return nil