This is an automated email from the ASF dual-hosted git repository.
laskoviymishka 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 c64f9a79d refactor(table)!: return branch transaction initialization
errors directly (#2313)
c64f9a79d is described below
commit c64f9a79dc25fd4fdf76f4293ef22d3d1632d520
Author: Minh Vu <[email protected]>
AuthorDate: Sun Oct 25 21:03:22 2026 +0100
refactor(table)!: return branch transaction initialization errors directly
(#2313)
---
table/branch_planning_test.go | 40 +++++++++++------------
table/rewrite_data_files.go | 4 +--
table/rewrite_data_files_test.go | 6 ++--
table/rewrite_manifests_test.go | 6 ++--
table/table.go | 26 ++++-----------
table/transaction_assert_ref_test.go | 4 ++-
table/transaction_internal_test.go | 63 ++++++++++++++++++------------------
website/src/api.md | 3 +-
website/src/getting-started.md | 5 ++-
9 files changed, 77 insertions(+), 80 deletions(-)
diff --git a/table/branch_planning_test.go b/table/branch_planning_test.go
index cb8667ac8..35951a96d 100644
--- a/table/branch_planning_test.go
+++ b/table/branch_planning_test.go
@@ -99,7 +99,7 @@ func idRecordReader(t *testing.T, tbl *table.Table, ids
...int64) array.RecordRe
func appendRowsOnRef(t *testing.T, tbl *table.Table, ref string, ids ...int64)
*table.Table {
t.Helper()
- txn, err := tbl.NewTransactionOnBranchWithError(ref)
+ txn, err := tbl.NewTransactionOnBranch(ref)
require.NoError(t, err)
rdr := idRecordReader(t, tbl, ids...)
@@ -253,7 +253,7 @@ func mainOnlyDataPath(t *testing.T, tbl *table.Table)
string {
func TestBranchDeleteClassifiesBranchFiles(t *testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.Delete(t.Context(),
iceberg.GreaterThanEqual(iceberg.Reference("id"), int64(3)), nil))
tbl, err = txn.Commit(t.Context())
@@ -270,7 +270,7 @@ func TestBranchDeleteClassifiesBranchFiles(t *testing.T) {
func TestBranchDeleteRewritesPartiallyMatchedBranchFile(t *testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.Delete(t.Context(),
iceberg.EqualTo(iceberg.Reference("id"), int64(3)), nil))
tbl, err = txn.Commit(t.Context())
@@ -286,7 +286,7 @@ func TestBranchDeleteRewritesPartiallyMatchedBranchFile(t
*testing.T) {
func TestBranchOverwriteReplacesBranchOnlyFiles(t *testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
rdr := idRecordReader(t, tbl, 9)
@@ -307,7 +307,7 @@ func TestOverwriteCreatingBranchPlansAgainstMainHead(t
*testing.T) {
tbl = appendRowsOnRef(t, tbl, table.MainBranch, 1, 2)
require.Nil(t, tbl.SnapshotByName(branchPlanningRef), "the branch must
not exist yet")
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
rdr := idRecordReader(t, tbl, 9)
@@ -359,7 +359,7 @@ func TestBranchOverwriteReplaysOnBranchHeadAfterConflict(t
*testing.T) {
}
writer := table.New(branchPlanningIdent, seedCat.current, metaLoc,
branchPlanningFS, cat)
- txn, err := writer.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := writer.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
rdr := idRecordReader(t, writer, 9)
defer rdr.Release()
@@ -384,7 +384,7 @@ func TestBranchOverwriteReplaysOnBranchHeadAfterConflict(t
*testing.T) {
func TestBranchFilteredOverwriteClassifiesBranchFiles(t *testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
rdr := idRecordReader(t, tbl, 7)
@@ -406,7 +406,7 @@ func TestBranchAddDataFilesDuplicateCheckUsesBranchHead(t
*testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
branchOnly := branchOnlyDataPath(t, tbl)
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
err = txn.AddDataFiles(t.Context(),
[]iceberg.DataFile{buildDataFile(t, branchOnly)}, nil)
@@ -418,7 +418,7 @@ func TestBranchAddDataFilesDuplicateCheckUsesBranchHead(t
*testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
mainOnly := mainOnlyDataPath(t, tbl)
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.AddDataFiles(t.Context(),
[]iceberg.DataFile{buildDataFile(t, mainOnly)}, nil))
@@ -436,7 +436,7 @@ func TestBranchAddFilesDuplicateCheckUsesBranchHead(t
*testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
branchOnly := branchOnlyDataPath(t, tbl)
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
err = txn.AddFiles(t.Context(), []string{branchOnly}, nil,
false)
@@ -448,7 +448,7 @@ func TestBranchAddFilesDuplicateCheckUsesBranchHead(t
*testing.T) {
tbl := seedDivergentBranch(t, newBranchPlanningTable(t, nil))
mainOnly := mainOnlyDataPath(t, tbl)
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.AddFiles(t.Context(),
[]string{mainOnly}, nil, false))
@@ -476,7 +476,7 @@ func TestReplaceOnBranchOnlyTable(t *testing.T) {
require.NoError(t, err)
writeParquetFile(t, replacement, arrowSchema, `[{"id":8}]`)
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.ReplaceDataFilesByPath(t.Context(),
original, []string{replacement}, nil))
tbl, err = txn.Commit(t.Context())
@@ -490,7 +490,7 @@ func TestReplaceOnBranchOnlyTable(t *testing.T) {
tbl := newBranchPlanningTable(t, nil)
staged := buildDataFile(t,
tbl.Location()+"/data/branch-1.parquet")
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.AddDataFiles(t.Context(),
[]iceberg.DataFile{staged}, nil))
tbl, err = txn.Commit(t.Context())
@@ -498,7 +498,7 @@ func TestReplaceOnBranchOnlyTable(t *testing.T) {
require.Nil(t, tbl.CurrentSnapshot(), "fixture must leave main
without a head")
replacement := buildDataFile(t,
tbl.Location()+"/data/branch-2.parquet")
- txn, err =
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err = tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.ReplaceDataFiles(t.Context(),
[]iceberg.DataFile{staged},
[]iceberg.DataFile{replacement}, nil))
@@ -519,14 +519,14 @@ func TestBranchReplaceFilesResolvesBranchDeleteFiles(t
*testing.T) {
staged := buildDataFile(t, tbl.Location()+"/data/branch-staged.parquet")
posDelete := buildPosDeleteFile(t,
tbl.Location()+"/data/branch-pos-delete.parquet")
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t,
txn.NewRowDelta(nil).AddRows(staged).AddDeletes(posDelete).Commit(t.Context()))
tbl, err = txn.Commit(t.Context())
require.NoError(t, err)
replacement := buildDataFile(t,
tbl.Location()+"/data/branch-compacted.parquet")
- txn, err = tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err = tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.ReplaceFiles(t.Context(),
[]iceberg.DataFile{staged}, []iceberg.DataFile{replacement},
[]iceberg.DataFile{posDelete}, nil))
@@ -553,7 +553,7 @@ func
TestBranchMergeOnReadDeleteMergesBranchDeletionVector(t *testing.T) {
tbl = appendRowsOnRef(t, tbl, table.MainBranch, 1, 2, 3, 4, 5)
for _, id := range []int64{2, 3} {
- txn, err :=
tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.Delete(t.Context(),
iceberg.EqualTo(iceberg.Reference("id"), id), nil))
tbl, err = txn.Commit(t.Context())
@@ -578,7 +578,7 @@ func TestBranchMergeOnReadDeleteTargetsBranchOnlyFile(t
*testing.T) {
tbl = appendRowsOnRef(t, tbl, table.MainBranch, 1, 2)
tbl = appendRowsOnRef(t, tbl, branchPlanningRef, 3, 4)
- txn, err := tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err := tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t, txn.Delete(t.Context(),
iceberg.EqualTo(iceberg.Reference("id"), int64(3)), nil))
tbl, err = txn.Commit(t.Context())
@@ -608,7 +608,7 @@ func TestBranchRowDeltaResolvesRemovedDeletesOnBranchHead(t
*testing.T) {
// The first vector (hiding position 1, id 2) exists only on the branch.
dv1 := writeDV(t, location, "dv-001.puffin", dataPath, []int64{1})
- txn, err = tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err = tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t,
txn.NewRowDelta(nil).AddDeletes(dv1).Commit(t.Context()))
tbl, err = txn.Commit(t.Context())
@@ -616,7 +616,7 @@ func TestBranchRowDeltaResolvesRemovedDeletesOnBranchHead(t
*testing.T) {
require.Equal(t, []int64{1, 3, 4, 5}, idsOnRef(t, tbl,
branchPlanningRef))
dv2 := writeDV(t, location, "dv-002.puffin", dataPath, []int64{1, 3})
- txn, err = tbl.NewTransactionOnBranchWithError(branchPlanningRef)
+ txn, err = tbl.NewTransactionOnBranch(branchPlanningRef)
require.NoError(t, err)
require.NoError(t,
txn.NewRowDelta(nil).AddDeletes(dv2).RemoveDeletes(dv1).Commit(t.Context()))
tbl, err = txn.Commit(t.Context())
diff --git a/table/rewrite_data_files.go b/table/rewrite_data_files.go
index a66e90da2..a0d167989 100644
--- a/table/rewrite_data_files.go
+++ b/table/rewrite_data_files.go
@@ -93,7 +93,7 @@ type CompactionGroupFailure struct {
// normally be invoked through this action so callers do not accidentally try
// to commit the parent transaction again.
func (t Table) RewriteDataFiles(ctx context.Context, groups
[]CompactionTaskGroup, opts RewriteDataFilesOptions) (*RewriteResult, error) {
- txn, err := t.NewTransactionOnBranchWithError(MainBranch)
+ txn, err := t.NewTransactionOnBranch(MainBranch)
if err != nil {
return nil, err
}
@@ -692,7 +692,7 @@ func (t *Transaction) rewriteDataFilesPartial(ctx
context.Context, groups []Comp
}
deletesToRemove := append(deadPositionDeletes, safeDVs...)
- groupTxn, err :=
current.NewTransactionOnBranchWithError(t.branch)
+ groupTxn, err := current.NewTransactionOnBranch(t.branch)
if err != nil {
return result, cleanupBatch(fmt.Errorf("create
transaction for partial rewrite batch: %w", err))
}
diff --git a/table/rewrite_data_files_test.go b/table/rewrite_data_files_test.go
index d9bc9dc6d..242fe6436 100644
--- a/table/rewrite_data_files_test.go
+++ b/table/rewrite_data_files_test.go
@@ -882,7 +882,7 @@ func addPartitionedRowsOnRef(t *testing.T, tbl
*table.Table, ref, name, partitio
info, err := os.Stat(dataPath)
require.NoError(t, err)
- txn, err := tbl.NewTransactionOnBranchWithError(ref)
+ txn, err := tbl.NewTransactionOnBranch(ref)
require.NoError(t, err)
builder, err := iceberg.NewDataFileBuilder(
tbl.Spec(), iceberg.EntryContentData, dataPath,
iceberg.ParquetFile,
@@ -927,7 +927,7 @@ func
TestRewriteDataFiles_PartialProgressUsesTargetBranchSnapshot(t *testing.T)
tbl.Spec(), iceberg.EntryContentPosDeletes, posDelPath,
iceberg.ParquetFile,
map[int]any{1000: "branch"}, nil, nil, 2, info.Size())
require.NoError(t, err)
- txn, err := tbl.NewTransactionOnBranchWithError(branch)
+ txn, err := tbl.NewTransactionOnBranch(branch)
require.NoError(t, err)
require.NoError(t,
txn.NewRowDelta(nil).AddDeletes(delBuilder.Build()).Commit(t.Context()))
tbl, err = txn.Commit(t.Context())
@@ -948,7 +948,7 @@ func
TestRewriteDataFiles_PartialProgressUsesTargetBranchSnapshot(t *testing.T)
}
require.NotNil(t, rewriteTask.File)
- txn, err = tbl.NewTransactionOnBranchWithError(branch)
+ txn, err = tbl.NewTransactionOnBranch(branch)
require.NoError(t, err)
result, err := txn.RewriteDataFiles(t.Context(),
[]table.CompactionTaskGroup{{
PartitionKey: "branch",
diff --git a/table/rewrite_manifests_test.go b/table/rewrite_manifests_test.go
index b5b205c11..c27356870 100644
--- a/table/rewrite_manifests_test.go
+++ b/table/rewrite_manifests_test.go
@@ -425,7 +425,8 @@ func TestRewriteManifestsOnBranchOnlyTable(t *testing.T) {
rec := array.NewRecordBatch(rewriteArrowSchema,
[]arrow.Array{col}, 1)
arrTbl := array.NewTableFromRecords(rewriteArrowSchema,
[]arrow.RecordBatch{rec})
- txn := tbl.NewTransactionOnBranch("feature")
+ txn, err := tbl.NewTransactionOnBranch("feature")
+ require.NoError(t, err)
require.NoError(t, txn.AppendTable(ctx, arrTbl, 1024, nil))
tbl, err = txn.Commit(ctx)
require.NoError(t, err)
@@ -443,7 +444,8 @@ func TestRewriteManifestsOnBranchOnlyTable(t *testing.T) {
require.NoError(t, err)
require.Len(t, before, numData)
- txn := tbl.NewTransactionOnBranch("feature")
+ txn, err := tbl.NewTransactionOnBranch("feature")
+ require.NoError(t, err)
res, err := txn.RewriteManifests(ctx)
require.NoError(t, err)
require.False(t, res.IsNoOp(),
diff --git a/table/table.go b/table/table.go
index b2c12d29c..3807f5e7f 100644
--- a/table/table.go
+++ b/table/table.go
@@ -174,7 +174,7 @@ func (t Table) LocationProvider() (LocationProvider, error)
{
}
func (t Table) NewTransaction() *Transaction {
- txn, err := t.NewTransactionOnBranchWithError(MainBranch)
+ txn, err := t.NewTransactionOnBranch(MainBranch)
if err != nil {
return t.newBrokenTransaction(MainBranch, err)
}
@@ -182,17 +182,6 @@ func (t Table) NewTransaction() *Transaction {
return txn
}
-// NewTransactionOnBranch creates a new transaction that commits to the named
-// branch. Use [NewTransaction] to commit to the default "main" branch.
-func (t Table) NewTransactionOnBranch(branch string) *Transaction {
- txn, err := t.NewTransactionOnBranchWithError(branch)
- if err != nil {
- return t.newBrokenTransaction(branch, err)
- }
-
- return txn
-}
-
func (t Table) newBrokenTransaction(branch string, err error) *Transaction {
return &Transaction{
tbl: &t,
@@ -202,13 +191,12 @@ func (t Table) newBrokenTransaction(branch string, err
error) *Transaction {
}
}
-// NewTransactionOnBranchWithError creates a new transaction and returns any
metadata
-// initialization error that prevents builder construction.
-//
-// This preserves the old non-failing constructor contract while allowing
-// callers to receive the precise initialization error instead of hitting
-// panic/undefined behavior later.
-func (t Table) NewTransactionOnBranchWithError(branch string) (*Transaction,
error) {
+// NewTransactionOnBranch creates a transaction that commits to the named
+// branch. Initialization failures and attempts to target a tag are returned
+// immediately instead of being stored in a broken transaction.
+// A missing branch is allowed; its absence is checked at commit time.
+// Use [NewTransaction] to target the default "main" branch.
+func (t Table) NewTransactionOnBranch(branch string) (*Transaction, error) {
for name, ref := range t.metadata.Refs() {
if name != branch {
continue
diff --git a/table/transaction_assert_ref_test.go
b/table/transaction_assert_ref_test.go
index 47297f9f0..97b61bd98 100644
--- a/table/transaction_assert_ref_test.go
+++ b/table/transaction_assert_ref_test.go
@@ -169,7 +169,9 @@ func TestTransactionAssertRefSnapshotID(t *testing.T) {
changed := graftSnapshotOnto(t, withFeature, "feature", 200)
tbl, cat := newAssertRefTestTable(t, withFeature, changed)
- err = commitPropsWithAssertedRef(t,
tbl.NewTransactionOnBranch("feature"), "")
+ branchTxn, err := tbl.NewTransactionOnBranch("feature")
+ require.NoError(t, err)
+ err = commitPropsWithAssertedRef(t, branchTxn, "")
assert.ErrorIs(t, err, ErrCommitFailed)
assert.NotContains(t, cat.metadata.Properties(), "offsets")
})
diff --git a/table/transaction_internal_test.go
b/table/transaction_internal_test.go
index 0b09ed835..eec5adb36 100644
--- a/table/transaction_internal_test.go
+++ b/table/transaction_internal_test.go
@@ -157,7 +157,7 @@ func TestBranchWriteCommitsThroughCatalogPath(t *testing.T)
{
// CommitTable where the branch requirement is
validated.
cat := &headTrackingCatalog{metadata: divergedMeta}
tbl := New(ident, divergedMeta, "metadata.json",
func(context.Context) (iceio.IO, error) { return memIO, nil }, cat)
- txnFeat, err :=
tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
spFeat := tc.newProd(tc.op, txnFeat, memIO, nil, nil)
@@ -218,7 +218,7 @@ func TestBranchCreateForksFromMainHead(t *testing.T) {
// First write to the new "feature" branch.
cat := &headTrackingCatalog{metadata: mainMeta}
tbl := New(ident, mainMeta, "metadata.json", func(context.Context)
(iceio.IO, error) { return memIO, nil }, cat)
- txnFeat, err := tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
spFeat := newFastAppendFilesProducer(OpAppend, txnFeat, memIO, nil, nil)
spFeat.appendDataFile(newTestDataFile(t, spec,
"file://feature.parquet", nil))
@@ -268,7 +268,7 @@ func TestBranchCreateForksFromMainHeadAcrossRetry(t
*testing.T) {
cat := &flakyCatalog{metadata: mainMeta, failUntilAttempt: 1, failWith:
fmt.Errorf("REST: %w", ErrCommitFailed)}
tbl := New(ident, mainMeta, "metadata.json", func(context.Context)
(iceio.IO, error) { return memIO, nil }, cat)
- txnFeat, err := tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
spFeat := newFastAppendFilesProducer(OpAppend, txnFeat, memIO, nil, nil)
spFeat.appendDataFile(newTestDataFile(t, spec,
"file://feature.parquet", nil))
@@ -333,7 +333,7 @@ func TestBranchRetryParentsOnPeerAdvancedBranchHead(t
*testing.T) {
tbl := New(ident, baseMeta, "metadata.json",
func(context.Context) (iceio.IO, error) { return wfs, nil },
cat)
- txnFeat, err := tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
spFeat := newFastAppendFilesProducer(OpAppend, txnFeat, wfs, nil, nil)
spFeat.appendDataFile(newTestDataFile(t, spec,
"mem://default/table-location/data/feature.parquet", nil))
@@ -359,7 +359,7 @@ func TestBranchRetryParentsOnPeerAdvancedBranchHead(t
*testing.T) {
// TestBranchCommitRejectsNameThatBecomesATagOnRetry covers the absent-branch
to
// tag race. "feature" does not exist when the transaction is constructed, so
-// the ref-type guard in NewTransactionOnBranchWithError has nothing to reject
+// the ref-type guard in NewTransactionOnBranch has nothing to reject
// and the commit carries an "assert this ref is absent" requirement
// (baseRefSnapshotID returns nil for a branch absent from the base). If a peer
// creates that name as a TAG before the retry, the replay must fail: without
@@ -399,7 +399,7 @@ func TestBranchCommitRejectsNameThatBecomesATagOnRetry(t
*testing.T) {
tbl := New(ident, mainMeta, "metadata.json",
func(context.Context) (iceio.IO, error) { return memIO, nil },
cat)
- txnFeat, err := tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err, "the name is still absent here, so construction
cannot reject it")
spFeat := newFastAppendFilesProducer(OpAppend, txnFeat, memIO, nil, nil)
spFeat.appendDataFile(newTestDataFile(t, spec,
"file://feature.parquet", nil))
@@ -450,7 +450,7 @@ func
TestOverwriteOnBranchOnlyTableKeepsOverwriteSemantics(t *testing.T) {
t.Run("branch with a head overwrites", func(t *testing.T) {
tbl := New(ident, branchOnlyMeta, "metadata.json",
func(context.Context) (iceio.IO, error) { return memIO,
nil }, nil)
- txnFeat, err := tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
prod := txnFeat.updateSnapshot(memIO, nil,
OpOverwrite).mergeOverwrite(nil, nil)
@@ -525,7 +525,7 @@ func TestBranchMultipleStagedSnapshotsChainOnRetry(t
*testing.T) {
tbl := New(ident, mainMeta, "metadata.json",
func(context.Context) (iceio.IO, error) { return memIO, nil },
cat)
- txnFeat, err := tbl.NewTransactionOnBranchWithError("feature")
+ txnFeat, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
// A1 forks from main's head; A2 is staged on top of A1.
@@ -969,7 +969,7 @@ func
TestTransactionApplyDedupesEquivalentRequirementsWithinAndAcrossCalls(t *te
requireContainsRefSnapshotRequirement(t, txn.reqs, MainBranch,
&mainSnapshotID)
}
-func TestNewTransactionOnBranchWithErrorReturnsTransactionInitError(t
*testing.T) {
+func TestNewTransactionOnBranchReportsInitializationError(t *testing.T) {
baseMeta, err := NewMetadata(simpleSchema(), iceberg.UnpartitionedSpec,
UnsortedSortOrder, "table-location", nil)
require.NoError(t, err, "new metadata")
@@ -977,7 +977,7 @@ func
TestNewTransactionOnBranchWithErrorReturnsTransactionInitError(t *testing.T
Metadata: baseMeta,
}, "metadata.json", func(context.Context) (iceio.IO, error) {
return nil, nil
- }, nil).NewTransactionOnBranchWithError(MainBranch)
+ }, nil).NewTransactionOnBranch(MainBranch)
require.Error(t, err, "expected metadata builder initialization to
fail")
require.ErrorContains(t, err, "current schema is missing")
require.ErrorIs(t, err, ErrInvalidMetadata)
@@ -993,14 +993,10 @@ func TestNewTransactionOnBranchRejectsTags(t *testing.T) {
tbl := New(Identifier{"db", "table"}, meta, "metadata.json", nil, nil)
before := tbl.Metadata()
- transaction, err := tbl.NewTransactionOnBranchWithError("release")
+ transaction, err := tbl.NewTransactionOnBranch("release")
require.ErrorIs(t, err, iceberg.ErrInvalidArgument)
require.Nil(t, transaction)
require.True(t, before.Equals(tbl.Metadata()))
-
- legacyTransaction := tbl.NewTransactionOnBranch("release")
- require.ErrorIs(t, legacyTransaction.initErr,
iceberg.ErrInvalidArgument)
- require.ErrorContains(t,
legacyTransaction.SetProperties(iceberg.Properties{"k": "v"}), "tags cannot be
transaction targets")
}
func TestTransactionCommitRejectsSameIDTagRace(t *testing.T) {
@@ -1035,13 +1031,13 @@ func
TestNewTransactionOnBranchAllowsBranchesAndNewRefs(t *testing.T) {
tbl := New(Identifier{"db", "table"}, meta, "metadata.json", nil, nil)
for _, branch := range []string{MainBranch, "feature", "new-branch"} {
- transaction, err := tbl.NewTransactionOnBranchWithError(branch)
+ transaction, err := tbl.NewTransactionOnBranch(branch)
require.NoError(t, err, branch)
require.NotNil(t, transaction, branch)
}
}
-func TestNewTransactionOnBranchKeepsLegacySignatureAndFailsOnUse(t *testing.T)
{
+func TestNewTransactionStillDefersInitializationErrors(t *testing.T) {
baseMeta, err := NewMetadata(simpleSchema(), iceberg.UnpartitionedSpec,
UnsortedSortOrder, "table-location", nil)
require.NoError(t, err, "new metadata")
@@ -1749,7 +1745,7 @@ func requireUntouchedRollbackRefs(t *testing.T, txn
*Transaction, except string)
func TestRollbackToSnapshotOnBranchDoesNotCorruptMain(t *testing.T) {
tbl := newRollbackBranchTable(t)
- branchTxn, err := tbl.NewTransactionOnBranchWithError("feature")
+ branchTxn, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
require.NoError(t, branchTxn.RollbackToSnapshot(10))
@@ -1767,7 +1763,7 @@ func TestRollbackToSnapshotOnBranchDoesNotCorruptMain(t
*testing.T) {
func TestRollbackToSnapshotValidatesAncestryAgainstTargetBranch(t *testing.T) {
tbl := newRollbackBranchTable(t)
- branchTxn, err := tbl.NewTransactionOnBranchWithError("diverged")
+ branchTxn, err := tbl.NewTransactionOnBranch("diverged")
require.NoError(t, err)
err = branchTxn.RollbackToSnapshot(20)
@@ -1789,7 +1785,7 @@ func
TestRollbackToSnapshotValidatesAncestryAgainstTargetBranch(t *testing.T) {
func TestRollbackToSnapshotRejectsUnknownBranch(t *testing.T) {
tbl := newRollbackBranchTable(t)
- branchTxn, err := tbl.NewTransactionOnBranchWithError("ghost")
+ branchTxn, err := tbl.NewTransactionOnBranch("ghost")
require.NoError(t, err)
err = branchTxn.RollbackToSnapshot(10)
@@ -1808,7 +1804,7 @@ func TestRollbackToSnapshotRejectsUnknownBranch(t
*testing.T) {
func TestRollbackToSnapshotRejectsDanglingBranchRef(t *testing.T) {
tbl := newRollbackBranchTable(t)
- branchTxn, err := tbl.NewTransactionOnBranchWithError("feature")
+ branchTxn, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
branchTxn.meta.refs["feature"] = SnapshotRef{SnapshotID: 9999,
SnapshotRefType: BranchRef}
@@ -1827,7 +1823,7 @@ func TestRollbackToSnapshotRejectsDanglingBranchRef(t
*testing.T) {
func TestRollbackToSnapshotRejectsTagRef(t *testing.T) {
tbl := newRollbackBranchTable(t)
- branchTxn, err := tbl.NewTransactionOnBranchWithError("feature")
+ branchTxn, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
branchTxn.meta.refs["feature"] = SnapshotRef{SnapshotID: 30,
SnapshotRefType: TagRef}
@@ -1899,7 +1895,7 @@ func
TestRollbackToSnapshotSharesOneBaseAssertionWithProducer(t *testing.T) {
t.Run("producer then rollback", func(t *testing.T) {
tbl := newRollbackBranchTable(t)
- txn, err := tbl.NewTransactionOnBranchWithError("feature")
+ txn, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
require.NoError(t, producerAssertion(txn))
@@ -1911,7 +1907,7 @@ func
TestRollbackToSnapshotSharesOneBaseAssertionWithProducer(t *testing.T) {
t.Run("rollback then producer", func(t *testing.T) {
tbl := newRollbackBranchTable(t)
- txn, err := tbl.NewTransactionOnBranchWithError("feature")
+ txn, err := tbl.NewTransactionOnBranch("feature")
require.NoError(t, err)
require.NoError(t, txn.RollbackToSnapshot(10))
@@ -1939,7 +1935,7 @@ func TestRollbackToSnapshotCommitRejectsBranchTurnedTag(t
*testing.T) {
raced := New(tbl.Identifier(), baseMeta, "metadata.json",
func(context.Context) (iceio.IO, error) { return
iceio.LocalFS{}, nil }, cat)
- txn, err := raced.NewTransactionOnBranchWithError("feature")
+ txn, err := raced.NewTransactionOnBranch("feature")
require.NoError(t, err)
require.NoError(t, txn.RollbackToSnapshot(10))
@@ -2060,7 +2056,8 @@ func TestTableCommitFencesTargetBranch(t *testing.T) {
t.Run("branch transaction pins only that branch", func(t *testing.T) {
base := metadataWithRef(t, newConflictTestMetadata(t, &head),
"audit", BranchRef)
- tx := multiTableTestTable(t,
base).NewTransactionOnBranch("audit")
+ tx, err := multiTableTestTable(t,
base).NewTransactionOnBranch("audit")
+ require.NoError(t, err)
require.NoError(t,
tx.SetProperties(iceberg.Properties{"offsets": "42"}))
tc, err := tx.TableCommit()
@@ -2075,7 +2072,8 @@ func TestTableCommitFencesTargetBranch(t *testing.T) {
t.Run("absent target branch requires absence", func(t *testing.T) {
base := newConflictTestMetadata(t, &head)
- tx := multiTableTestTable(t,
base).NewTransactionOnBranch("audit")
+ tx, err := multiTableTestTable(t,
base).NewTransactionOnBranch("audit")
+ require.NoError(t, err)
require.NoError(t,
tx.SetProperties(iceberg.Properties{"offsets": "42"}))
tc, err := tx.TableCommit()
@@ -2094,7 +2092,8 @@ func TestTableCommitFencesTargetBranch(t *testing.T) {
// enforces snapshot-id equality alone.
t.Run("target name created as a tag is rejected locally", func(t
*testing.T) {
base := newConflictTestMetadata(t, &head)
- tx := multiTableTestTable(t,
base).NewTransactionOnBranch("audit")
+ tx, err := multiTableTestTable(t,
base).NewTransactionOnBranch("audit")
+ require.NoError(t, err)
require.NoError(t,
tx.SetProperties(iceberg.Properties{"offsets": "42"}))
tc, err := tx.TableCommit()
@@ -2147,13 +2146,15 @@ func TestTableCommitRequirementsMatchCommit(t
*testing.T) {
require.Equal(t, base, committed.Metadata())
require.Equal(t, base, payload.Metadata())
- txCommit := committed.NewTransactionOnBranch(branch)
+ txCommit, err :=
committed.NewTransactionOnBranch(branch)
+ require.NoError(t, err)
require.NoError(t,
txCommit.SetProperties(iceberg.Properties{"offsets": "42"}))
- _, err := txCommit.Commit(t.Context())
+ _, err = txCommit.Commit(t.Context())
require.NoError(t, err)
require.Len(t, cat.reqs, 1)
- txPayload := payload.NewTransactionOnBranch(branch)
+ txPayload, err := payload.NewTransactionOnBranch(branch)
+ require.NoError(t, err)
require.NoError(t,
txPayload.SetProperties(iceberg.Properties{"offsets": "42"}))
tc, err := txPayload.TableCommit()
require.NoError(t, err)
diff --git a/website/src/api.md b/website/src/api.md
index f8df5d69d..85504ee5a 100644
--- a/website/src/api.md
+++ b/website/src/api.md
@@ -388,7 +388,8 @@ newTbl, err := txn.Commit(ctx)
To target a specific branch:
```go
-txn := tbl.NewTransactionOnBranch("staging")
+txn, err := tbl.NewTransactionOnBranch("staging")
+if err != nil { /* ... */ }
```
`Commit` retries automatically on conflict (`ErrCommitFailed`) - tune via the
`commit.retry.*` table properties.
diff --git a/website/src/getting-started.md b/website/src/getting-started.md
index fc871da84..3ef644477 100644
--- a/website/src/getting-started.md
+++ b/website/src/getting-started.md
@@ -235,7 +235,10 @@ func createExperimentBranch(ctx context.Context, cat
*sqlcat.Catalog, tbl *table
// Step 2: open a transaction on the branch and write.
func writeOnBranch(ctx context.Context, tbl *table.Table, arrowTbl
arrow.Table) (*table.Table, error) {
- txn := tbl.NewTransactionOnBranch("experiment")
+ txn, err := tbl.NewTransactionOnBranch("experiment")
+ if err != nil {
+ return nil, err
+ }
if err := txn.AppendTable(ctx, arrowTbl, 1024, nil); err != nil {
return nil, err
}
