badalprasadsingh commented on code in PR #1637:
URL: https://github.com/apache/iceberg-go/pull/1637#discussion_r3720136153
##########
table/transaction_internal_test.go:
##########
@@ -44,6 +45,140 @@ func
TestTransactionApplyKeepsDistinctRequirementsOfSameType(t *testing.T) {
requireContainsRefSnapshotRequirement(t, txn.reqs, "feature",
&featureSnapshotID)
}
+func TestCurrentSnapshotForRefResolvesBranchHead(t *testing.T) {
+ txn := newTransactionWithSnapshotRefs(t)
+
+ main := txn.meta.currentSnapshotForRef(MainBranch)
+ require.NotNil(t, main)
+ require.Equal(t, int64(10), main.SnapshotID)
+
+ empty := txn.meta.currentSnapshotForRef("")
+ require.NotNil(t, empty)
+ require.Equal(t, int64(10), empty.SnapshotID, "empty ref must resolve
like main")
+
+ feature := txn.meta.currentSnapshotForRef("feature")
+ require.NotNil(t, feature)
+ require.Equal(t, int64(20), feature.SnapshotID, "feature branch must
resolve to its own head (20), not main (10)")
+
+ missing := txn.meta.currentSnapshotForRef("does-not-exist")
+ require.NotNil(t, missing)
+ require.Equal(t, int64(10), missing.SnapshotID, "a not-yet-created
branch falls back to main's head")
+}
+
+func TestCurrentSnapshotIDForRefResolvesBranchHead(t *testing.T) {
+ txn := newTransactionWithSnapshotRefs(t)
+
+ require.NotNil(t, txn.meta.currentSnapshotIDForRef(MainBranch))
+ require.Equal(t, int64(10),
*txn.meta.currentSnapshotIDForRef(MainBranch))
+
+ require.NotNil(t, txn.meta.currentSnapshotIDForRef("feature"))
+ require.Equal(t, int64(20),
*txn.meta.currentSnapshotIDForRef("feature"),
+ "feature branch assertion id must be the branch head (20), not
main (10)")
+
+ require.Nil(t, txn.meta.currentSnapshotIDForRef("does-not-exist"),
+ "a not-yet-created branch must assert non-existence (nil), not
main's head")
+}
+
+func TestCreateSnapshotProducerParentsOnBranchHead(t *testing.T) {
+ t.Run("feature branch parents on feature head", func(t *testing.T) {
+ txn := newTransactionWithSnapshotRefs(t)
+ txn.branch = "feature"
+ sp := createSnapshotProducer(OpAppend, txn, nil, nil, nil)
+ require.Equal(t, int64(20), sp.parentSnapshotID,
+ "append on feature must layer on the feature head (20),
not main head (10)")
+ })
+
+ t.Run("main branch still parents on main head", func(t *testing.T) {
+ txn := newTransactionWithSnapshotRefs(t)
+ txn.branch = ""
+ sp := createSnapshotProducer(OpAppend, txn, nil, nil, nil)
+ require.Equal(t, int64(10), sp.parentSnapshotID)
+ })
+}
+
+func TestBranchWriteCommitsThroughCatalogPath(t *testing.T) {
+ ctx := context.Background()
+ spec := iceberg.NewPartitionSpec()
+ ident := Identifier{"db", "tbl"}
+
+ producers := []struct {
+ name string
+ op Operation
+ newProd func(Operation, *Transaction, iceio.WriteFileIO,
*uuid.UUID, iceberg.Properties) *snapshotProducer
+ }{
+ {"fast append", OpAppend, newFastAppendFilesProducer},
+ {"merge append", OpAppend, newMergeAppendFilesProducer},
+ {"overwrite", OpOverwrite, newOverwriteFilesProducer},
+ }
+
+ for _, tc := range producers {
+ t.Run(tc.name, func(t *testing.T) {
+ txn, memIO := createTestTransactionWithMemIO(t, spec)
+
+ // 1. Create the "feature" branch on a fresh table. The
branch does
+ // not exist yet, so the snapshot has no parent and the
requirement
+ // asserts the branch is absent (nil).
+ txn.branch = "feature"
+ sp1 := newFastAppendFilesProducer(OpAppend, txn, memIO,
nil, nil)
+ sp1.appendDataFile(newTestDataFile(t, spec,
"file://feature-1.parquet", nil))
+ up1, rq1, err := sp1.commit(ctx)
+ require.NoError(t, err)
+ addSnap1, ok := up1[0].(*addSnapshotUpdate)
+ require.True(t, ok)
+ require.Nil(t, addSnap1.Snapshot.ParentSnapshotID,
"first feature snapshot has no parent")
Review Comment:
Done.
It was vacuous. Kept the fresh-table case as said but added
`TestBranchCreateForksFromMainHead`, which starts from a table where main
already has data, so `ParentSnapshotID` must equal main's head and the new
branch's manifests must include main's files - it now fails if the `!ok`
fallback is removed. As you noted, this is the same scenario the retry path got
wrong, so the retry variant pins it a second time.
--
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]
