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 48c85561 chore(metadata): remove Builder returns from MetadataBuilder
(#566)
48c85561 is described below
commit 48c85561da582ccd35aa2466c1398a429512cde5
Author: Tobias Pütz <tob...@minio.io>
AuthorDate: Thu Sep 18 00:55:00 2025 +0200
chore(metadata): remove Builder returns from MetadataBuilder (#566)
This removes the `MetadataBuilder` return value from the mutators of
`MetadataBuilder`. Since all those functions are fallible, we cannot use
it as a builder anyways. Removing them makes the API more concise and
simpler.
---
catalog/glue/schema_test.go | 4 +-
io/azure_integration_test.go | 8 +-
table/arrow_utils_internal_test.go | 2 +-
table/metadata.go | 155 +++++++++++++++-----------------
table/metadata_builder_internal_test.go | 96 ++++++++------------
table/metadata_internal_test.go | 39 +++-----
table/time_travel_test.go | 3 +-
table/updates.go | 64 ++++---------
8 files changed, 148 insertions(+), 223 deletions(-)
diff --git a/catalog/glue/schema_test.go b/catalog/glue/schema_test.go
index e2622afa..88412db0 100644
--- a/catalog/glue/schema_test.go
+++ b/catalog/glue/schema_test.go
@@ -440,9 +440,9 @@ func TestSchemasToGlueColumns(t *testing.T) {
mb, err := table.MetadataBuilderFromBase(metadata)
assert.NoError(t, err)
- mb, err = mb.AddSchema(schemas[1])
+ err = mb.AddSchema(schemas[1])
assert.NoError(t, err)
- mb, err = mb.SetCurrentSchemaID(1)
+ err = mb.SetCurrentSchemaID(1)
assert.NoError(t, err)
metadata, err = mb.Build()
diff --git a/io/azure_integration_test.go b/io/azure_integration_test.go
index 49189443..cfe5ecb6 100644
--- a/io/azure_integration_test.go
+++ b/io/azure_integration_test.go
@@ -95,10 +95,10 @@ func (s *AzureBlobIOTestSuite)
TestAzuriteWarehouseConnectionString() {
path := "iceberg-test-azure/test-table-azure"
containerName := "warehouse"
properties := iceberg.Properties{
- "uri": ":memory:",
- sqlcat.DriverKey: sqliteshim.ShimName,
- sqlcat.DialectKey: string(sqlcat.SQLite),
- "type": "sql",
+ "uri": ":memory:",
+ sqlcat.DriverKey: sqliteshim.ShimName,
+ sqlcat.DialectKey: string(sqlcat.SQLite),
+ "type": "sql",
io.AdlsConnectionStringPrefix + accountName: connectionString,
}
diff --git a/table/arrow_utils_internal_test.go
b/table/arrow_utils_internal_test.go
index 7a19d627..25b1983c 100644
--- a/table/arrow_utils_internal_test.go
+++ b/table/arrow_utils_internal_test.go
@@ -184,7 +184,7 @@ func (suite *FileStatsMetricsSuite) getDataFile(meta
iceberg.Properties, writeSt
if len(meta) > 0 {
bldr, err := MetadataBuilderFromBase(tableMeta)
suite.Require().NoError(err)
- _, err = bldr.SetProperties(meta)
+ err = bldr.SetProperties(meta)
suite.Require().NoError(err)
tableMeta, err = bldr.Build()
suite.Require().NoError(err)
diff --git a/table/metadata.go b/table/metadata.go
index 8da21a70..568c9fd3 100644
--- a/table/metadata.go
+++ b/table/metadata.go
@@ -261,7 +261,7 @@ func (b *MetadataBuilder) currentSnapshot() *Snapshot {
return s
}
-func (b *MetadataBuilder) AddSchema(schema *iceberg.Schema) (*MetadataBuilder,
error) {
+func (b *MetadataBuilder) AddSchema(schema *iceberg.Schema) error {
newSchemaID := b.reuseOrCreateNewSchemaID(schema)
if _, err := b.GetSchemaByID(newSchemaID); err == nil {
@@ -270,7 +270,7 @@ func (b *MetadataBuilder) AddSchema(schema *iceberg.Schema)
(*MetadataBuilder, e
b.lastAddedSchemaID = &newSchemaID
}
- return b, nil
+ return nil
}
b.lastColumnId = max(b.lastColumnId, schema.HighestFieldID())
@@ -281,15 +281,15 @@ func (b *MetadataBuilder) AddSchema(schema
*iceberg.Schema) (*MetadataBuilder, e
b.updates = append(b.updates, NewAddSchemaUpdate(schema))
b.lastAddedSchemaID = &newSchemaID
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) AddPartitionSpec(spec *iceberg.PartitionSpec,
initial bool) (*MetadataBuilder, error) {
+func (b *MetadataBuilder) AddPartitionSpec(spec *iceberg.PartitionSpec,
initial bool) error {
newSpecID := b.reuseOrCreateNewPartitionSpecID(*spec)
freshSpec, err := spec.BindToSchema(b.CurrentSchema(),
b.lastPartitionID, &newSpecID, false)
if err != nil {
- return nil, err
+ return err
}
spec = nil
@@ -299,7 +299,7 @@ func (b *MetadataBuilder) AddPartitionSpec(spec
*iceberg.PartitionSpec, initial
b.lastAddedPartitionID = &newSpecID
}
- return b, nil
+ return nil
}
maxFieldID := 0
@@ -309,7 +309,7 @@ func (b *MetadataBuilder) AddPartitionSpec(spec
*iceberg.PartitionSpec, initial
if b.formatVersion <= 1 {
expectedID := partitionFieldStartID + fieldCount
if f.FieldID != expectedID {
- return nil, fmt.Errorf("v1 constraint:
partition field IDs are not sequential: expected %d, got %d", expectedID,
f.FieldID)
+ return fmt.Errorf("v1 constraint: partition
field IDs are not sequential: expected %d, got %d", expectedID, f.FieldID)
}
fieldCount++
}
@@ -334,25 +334,25 @@ func (b *MetadataBuilder) AddPartitionSpec(spec
*iceberg.PartitionSpec, initial
b.lastAddedPartitionID = &newSpecID
b.updates = append(b.updates, NewAddPartitionSpecUpdate(&freshSpec,
initial))
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) AddSnapshot(snapshot *Snapshot) (*MetadataBuilder,
error) {
+func (b *MetadataBuilder) AddSnapshot(snapshot *Snapshot) error {
if snapshot == nil {
- return nil, nil
+ return nil
}
if len(b.schemaList) == 0 {
- return nil, errors.New("can't add snapshot with no added
schemas")
+ return errors.New("can't add snapshot with no added schemas")
} else if len(b.specs) == 0 {
- return nil, errors.New("can't add snapshot with no added
partition specs")
+ return errors.New("can't add snapshot with no added partition
specs")
} else if s, _ := b.SnapshotByID(snapshot.SnapshotID); s != nil {
- return nil, fmt.Errorf("can't add snapshot with id %d, already
exists", snapshot.SnapshotID)
+ 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 nil, fmt.Errorf("can't add snapshot with sequence number
%d, must be > than last sequence number %d",
+ return fmt.Errorf("can't add snapshot with sequence number %d,
must be > than last sequence number %d",
snapshot.SequenceNumber, b.lastSequenceNumber)
}
@@ -361,12 +361,12 @@ func (b *MetadataBuilder) AddSnapshot(snapshot *Snapshot)
(*MetadataBuilder, err
b.lastSequenceNumber = &snapshot.SequenceNumber
b.snapshotList = append(b.snapshotList, *snapshot)
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) RemoveSnapshots(snapshotIds []int64)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) RemoveSnapshots(snapshotIds []int64) error {
if slices.Contains(snapshotIds, *b.currentSnapshotID) {
- return nil, errors.New("current snapshot cannot be removed")
+ return errors.New("current snapshot cannot be removed")
}
b.snapshotList = slices.DeleteFunc(b.snapshotList, func(e Snapshot)
bool {
@@ -377,10 +377,10 @@ func (b *MetadataBuilder) RemoveSnapshots(snapshotIds
[]int64) (*MetadataBuilder
})
b.updates = append(b.updates, NewRemoveSnapshotsUpdate(snapshotIds))
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) AddSortOrder(sortOrder *SortOrder, initial bool)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) AddSortOrder(sortOrder *SortOrder, initial bool)
error {
var sortOrders []SortOrder
if !initial {
sortOrders = append(sortOrders, b.sortOrderList...)
@@ -388,19 +388,19 @@ func (b *MetadataBuilder) AddSortOrder(sortOrder
*SortOrder, initial bool) (*Met
for _, s := range sortOrders {
if s.OrderID == sortOrder.OrderID {
- return nil, fmt.Errorf("sort order with id %d already
exists", sortOrder.OrderID)
+ return fmt.Errorf("sort order with id %d already
exists", sortOrder.OrderID)
}
}
b.sortOrderList = append(sortOrders, *sortOrder)
b.updates = append(b.updates, NewAddSortOrderUpdate(sortOrder, initial))
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) RemoveProperties(keys []string) (*MetadataBuilder,
error) {
+func (b *MetadataBuilder) RemoveProperties(keys []string) error {
if len(keys) == 0 {
- return b, nil
+ return nil
}
b.updates = append(b.updates, NewRemovePropertiesUpdate(keys))
@@ -408,33 +408,33 @@ func (b *MetadataBuilder) RemoveProperties(keys []string)
(*MetadataBuilder, err
delete(b.props, key)
}
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetCurrentSchemaID(currentSchemaID int)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetCurrentSchemaID(currentSchemaID int) error {
if currentSchemaID == -1 {
if b.lastAddedSchemaID == nil {
- return nil, errors.New("can't set current schema to
last added schema, no schema has been added")
+ return errors.New("can't set current schema to last
added schema, no schema has been added")
}
currentSchemaID = *b.lastAddedSchemaID
}
if currentSchemaID == b.currentSchemaID {
- return b, nil
+ return nil
}
_, err := b.GetSchemaByID(currentSchemaID)
if err != nil {
- return nil, fmt.Errorf("can't set current schema to schema with
id %d: %w", currentSchemaID, err)
+ return fmt.Errorf("can't set current schema to schema with id
%d: %w", currentSchemaID, err)
}
b.updates = append(b.updates,
NewSetCurrentSchemaUpdate(currentSchemaID))
b.currentSchemaID = currentSchemaID
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetDefaultSortOrderID(defaultSortOrderID int)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetDefaultSortOrderID(defaultSortOrderID int) error {
if defaultSortOrderID == -1 {
defaultSortOrderID = maxBy(b.sortOrderList, func(s SortOrder)
int {
return s.OrderID
@@ -442,41 +442,41 @@ func (b *MetadataBuilder)
SetDefaultSortOrderID(defaultSortOrderID int) (*Metada
if !slices.ContainsFunc(b.updates, func(u Update) bool {
return u.Action() == UpdateAddSortOrder &&
u.(*addSortOrderUpdate).SortOrder.OrderID == defaultSortOrderID
}) {
- return nil, errors.New("can't set default sort order to
last added with no added sort orders")
+ return errors.New("can't set default sort order to last
added with no added sort orders")
}
}
if defaultSortOrderID == b.defaultSortOrderID {
- return b, nil
+ return nil
}
if _, err := b.GetSortOrderByID(defaultSortOrderID); err != nil {
- return nil, fmt.Errorf("can't set default sort order to sort
order with id %d: %w", defaultSortOrderID, err)
+ return fmt.Errorf("can't set default sort order to sort order
with id %d: %w", defaultSortOrderID, err)
}
b.updates = append(b.updates,
NewSetDefaultSortOrderUpdate(defaultSortOrderID))
b.defaultSortOrderID = defaultSortOrderID
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetDefaultSpecID(defaultSpecID int)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetDefaultSpecID(defaultSpecID int) error {
lastUsed := false
if defaultSpecID == -1 {
if b.lastAddedPartitionID != nil {
lastUsed = true
defaultSpecID = *b.lastAddedPartitionID
} else {
- return nil, errors.New("can't set default spec to last
added with no added partition specs")
+ return errors.New("can't set default spec to last added
with no added partition specs")
}
}
if defaultSpecID == b.defaultSpecID {
- return b, nil
+ return nil
}
if _, err := b.GetSpecByID(defaultSpecID); err != nil {
- return nil, fmt.Errorf("can't set default spec to spec with id
%d: %w", defaultSpecID, err)
+ return fmt.Errorf("can't set default spec to spec with id %d:
%w", defaultSpecID, err)
}
if lastUsed {
@@ -486,43 +486,43 @@ func (b *MetadataBuilder) SetDefaultSpecID(defaultSpecID
int) (*MetadataBuilder,
}
b.defaultSpecID = defaultSpecID
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetFormatVersion(formatVersion int)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetFormatVersion(formatVersion int) error {
if formatVersion < b.formatVersion {
- return nil, fmt.Errorf("downgrading format version from %d to
%d is not allowed",
+ return fmt.Errorf("downgrading format version from %d to %d is
not allowed",
b.formatVersion, formatVersion)
}
if formatVersion > supportedTableFormatVersion {
- return nil, fmt.Errorf("unsupported format version %d",
formatVersion)
+ return fmt.Errorf("unsupported format version %d",
formatVersion)
}
if formatVersion == b.formatVersion {
- return b, nil
+ return nil
}
b.updates = append(b.updates,
NewUpgradeFormatVersionUpdate(formatVersion))
b.formatVersion = formatVersion
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetLoc(loc string) (*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetLoc(loc string) error {
if b.loc == loc {
- return b, nil
+ return nil
}
b.updates = append(b.updates, NewSetLocationUpdate(loc))
b.loc = loc
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetProperties(props iceberg.Properties)
(*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetProperties(props iceberg.Properties) error {
if len(props) == 0 {
- return b, nil
+ return nil
}
b.updates = append(b.updates, NewSetPropertiesUpdate(props))
@@ -532,7 +532,7 @@ func (b *MetadataBuilder) SetProperties(props
iceberg.Properties) (*MetadataBuil
maps.Copy(b.props, props)
}
- return b, nil
+ return nil
}
type setSnapshotRefOption func(*SnapshotRef) error
@@ -575,14 +575,14 @@ func (b *MetadataBuilder) SetSnapshotRef(
snapshotID int64,
refType RefType,
options ...setSnapshotRefOption,
-) (*MetadataBuilder, error) {
+) error {
ref := SnapshotRef{
SnapshotID: snapshotID,
SnapshotRefType: refType,
}
for _, opt := range options {
if err := opt(&ref); err != nil {
- return nil, fmt.Errorf("invalid snapshot ref option:
%w", err)
+ return fmt.Errorf("invalid snapshot ref option: %w",
err)
}
}
@@ -599,12 +599,12 @@ func (b *MetadataBuilder) SetSnapshotRef(
}
if existingRef, ok := b.refs[name]; ok && existingRef.Equals(ref) {
- return b, nil
+ return nil
}
snapshot, err := b.SnapshotByID(snapshotID)
if err != nil {
- return nil, fmt.Errorf("can't set snapshot ref %s to unknown
snapshot %d: %w", name, snapshotID, err)
+ return fmt.Errorf("can't set snapshot ref %s to unknown
snapshot %d: %w", name, snapshotID, err)
}
isAddedSnapshot := slices.ContainsFunc(b.updates, func(u Update) bool {
@@ -634,12 +634,12 @@ func (b *MetadataBuilder) SetSnapshotRef(
b.refs[name] = ref
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) RemoveSnapshotRef(name string) (*MetadataBuilder,
error) {
+func (b *MetadataBuilder) RemoveSnapshotRef(name string) error {
if _, found := b.refs[name]; !found {
- return nil, fmt.Errorf("snapshot ref not found: %s", name)
+ return fmt.Errorf("snapshot ref not found: %s", name)
}
if name == MainBranch {
@@ -650,18 +650,18 @@ func (b *MetadataBuilder) RemoveSnapshotRef(name string)
(*MetadataBuilder, erro
delete(b.refs, name)
b.updates = append(b.updates, NewRemoveSnapshotRefUpdate(name))
- return b, nil
+ return nil
}
-func (b *MetadataBuilder) SetUUID(uuid uuid.UUID) (*MetadataBuilder, error) {
+func (b *MetadataBuilder) SetUUID(uuid uuid.UUID) error {
if b.uuid == uuid {
- return b, nil
+ return nil
}
b.updates = append(b.updates, NewAssignUUIDUpdate(uuid))
b.uuid = uuid
- return b, nil
+ return nil
}
func (b *MetadataBuilder) SetLastUpdatedMS() *MetadataBuilder {
@@ -846,9 +846,9 @@ func (b *MetadataBuilder)
reuseOrCreateNewSchemaID(newSchema *iceberg.Schema) in
return newSchemaID
}
-func (b *MetadataBuilder) RemovePartitionSpecs(ints []int) (*MetadataBuilder,
error) {
+func (b *MetadataBuilder) RemovePartitionSpecs(ints []int) error {
if slices.Contains(ints, b.defaultSpecID) {
- return nil, fmt.Errorf("can't remove default partition spec
with id %d", b.defaultSpecID)
+ return fmt.Errorf("can't remove default partition spec with id
%d", b.defaultSpecID)
}
newSpecs := make([]iceberg.PartitionSpec, 0, len(b.specs)-len(ints))
@@ -868,7 +868,7 @@ func (b *MetadataBuilder) RemovePartitionSpecs(ints []int)
(*MetadataBuilder, er
b.updates = append(b.updates, NewRemoveSpecUpdate(ints))
}
- return b, nil
+ return nil
}
// maxBy returns the maximum value of extract(e) for all e in elems.
@@ -1409,48 +1409,39 @@ func NewMetadataWithUUID(sc *iceberg.Schema, partitions
*iceberg.PartitionSpec,
return nil, err
}
- _, err = builder.SetFormatVersion(formatVersion)
- if err != nil {
+ if err = builder.SetFormatVersion(formatVersion); err != nil {
return nil, err
}
- _, err = builder.SetUUID(tableUuid)
- if err != nil {
+ if err = builder.SetUUID(tableUuid); err != nil {
return nil, err
}
- _, err = builder.AddSortOrder(&reassignedIds.sortOrder, true)
- if err != nil {
+ if err = builder.AddSortOrder(&reassignedIds.sortOrder, true); err !=
nil {
return nil, err
}
- _, err = builder.SetDefaultSortOrderID(-1)
- if err != nil {
+ if err = builder.SetDefaultSortOrderID(-1); err != nil {
return nil, err
}
- _, err = builder.AddSchema(reassignedIds.schema)
- if err != nil {
+ if err = builder.AddSchema(reassignedIds.schema); err != nil {
return nil, err
}
- _, err = builder.SetCurrentSchemaID(-1)
- if err != nil {
+ if err = builder.SetCurrentSchemaID(-1); err != nil {
return nil, err
}
- _, err = builder.AddPartitionSpec(reassignedIds.partitionSpec, true)
- if err != nil {
+ if err = builder.AddPartitionSpec(reassignedIds.partitionSpec, true);
err != nil {
return nil, err
}
- _, err = builder.SetLoc(location)
- if err != nil {
+ if err = builder.SetLoc(location); err != nil {
return nil, err
}
- _, err = builder.SetProperties(props)
- if err != nil {
+ if err = builder.SetProperties(props); err != nil {
return nil, err
}
diff --git a/table/metadata_builder_internal_test.go
b/table/metadata_builder_internal_test.go
index 1fc900e4..0ff33869 100644
--- a/table/metadata_builder_internal_test.go
+++ b/table/metadata_builder_internal_test.go
@@ -68,24 +68,19 @@ func builderWithoutChanges(formatVersion int)
MetadataBuilder {
if err != nil {
panic(err)
}
- _, err = builder.SetFormatVersion(formatVersion)
- if err != nil {
+ if err = builder.SetFormatVersion(formatVersion); err != nil {
panic(err)
}
- _, err = builder.AddSortOrder(&sortOrder, true)
- if err != nil {
+ if err = builder.AddSortOrder(&sortOrder, true); err != nil {
panic(err)
}
- _, err = builder.AddSchema(&tableSchema)
- if err != nil {
+ if err = builder.AddSchema(&tableSchema); err != nil {
panic(err)
}
- _, err = builder.SetCurrentSchemaID(-1)
- if err != nil {
+ if err = builder.SetCurrentSchemaID(-1); err != nil {
panic(err)
}
- _, err = builder.AddPartitionSpec(&partitionSpec, true)
- if err != nil {
+ if err = builder.AddPartitionSpec(&partitionSpec, true); err != nil {
panic(err)
}
@@ -111,7 +106,7 @@ func TestAddRemovePartitionSpec(t *testing.T) {
iceberg.AddPartitionFieldBySourceID(3, "z",
iceberg.IdentityTransform{}, builder.schemaList[0], nil))
require.NoError(t, err)
- builderRef, err = builderRef.AddPartitionSpec(&addedSpec, false)
+ err = builderRef.AddPartitionSpec(&addedSpec, false)
require.NoError(t, err)
metadata, err := builderRef.Build()
require.NoError(t, err)
@@ -137,9 +132,8 @@ func TestAddRemovePartitionSpec(t *testing.T) {
newBuilder, err := MetadataBuilderFromBase(metadata)
require.NoError(t, err)
// Remove the spec
- newBuilderRef, err := newBuilder.RemovePartitionSpecs([]int{1})
- require.NoError(t, err)
- newBuild, err := newBuilderRef.Build()
+ require.NoError(t, newBuilder.RemovePartitionSpecs([]int{1}))
+ newBuild, err := newBuilder.Build()
require.NoError(t, err)
require.NotNil(t, newBuild)
require.Len(t, newBuilder.updates, 1)
@@ -157,22 +151,19 @@ func TestSetDefaultPartitionSpec(t *testing.T) {
iceberg.WithSpecID(10),
iceberg.AddPartitionFieldBySourceID(1, "y_bucket[2]",
iceberg.BucketTransform{NumBuckets: 2}, curSchema, nil))
require.NoError(t, err)
- builderRef, err := builder.AddPartitionSpec(&addedSpec, false)
- require.NoError(t, err)
- // Set the default partition spec
- builderRef, err = builderRef.SetDefaultSpecID(-1)
- require.NoError(t, err)
+ require.NoError(t, builder.AddPartitionSpec(&addedSpec, false))
+ require.NoError(t, builder.SetDefaultSpecID(-1))
id := 1001
expectedSpec, err := iceberg.NewPartitionSpecOpts(
iceberg.WithSpecID(1),
iceberg.AddPartitionFieldBySourceID(1, "y_bucket[2]",
iceberg.BucketTransform{NumBuckets: 2}, curSchema, &id))
require.NoError(t, err)
- require.True(t, builderRef.HasChanges())
- require.Equal(t, len(builderRef.updates), 2)
- require.True(t,
builderRef.updates[0].(*addPartitionSpecUpdate).Spec.Equals(expectedSpec))
- require.Equal(t, -1,
builderRef.updates[1].(*setDefaultSpecUpdate).SpecID)
- metadata, err := builderRef.Build()
+ require.True(t, builder.HasChanges())
+ require.Equal(t, len(builder.updates), 2)
+ require.True(t,
builder.updates[0].(*addPartitionSpecUpdate).Spec.Equals(expectedSpec))
+ require.Equal(t, -1, builder.updates[1].(*setDefaultSpecUpdate).SpecID)
+ metadata, err := builder.Build()
require.NoError(t, err)
require.NotNil(t, metadata)
@@ -195,18 +186,16 @@ func TestSetExistingDefaultPartitionSpec(t *testing.T) {
expectedSpec, err :=
iceberg.NewPartitionSpecOpts(iceberg.WithSpecID(1),
iceberg.AddPartitionFieldBySourceID(1, "y_bucket[2]",
iceberg.BucketTransform{NumBuckets: 2}, curSchema, &id))
require.NoError(t, err)
- builderRef, err := builder.AddPartitionSpec(&addedSpec, false)
- require.NoError(t, err)
+ require.NoError(t, builder.AddPartitionSpec(&addedSpec, false))
- builderRef, err = builderRef.SetDefaultSpecID(-1)
- require.NoError(t, err)
+ require.NoError(t, builder.SetDefaultSpecID(-1))
- require.True(t, builderRef.HasChanges())
- require.Len(t, builderRef.updates, 2)
- require.True(t,
builderRef.updates[0].(*addPartitionSpecUpdate).Spec.Equals(expectedSpec))
- require.Equal(t, -1,
builderRef.updates[1].(*setDefaultSpecUpdate).SpecID)
+ require.True(t, builder.HasChanges())
+ require.Len(t, builder.updates, 2)
+ require.True(t,
builder.updates[0].(*addPartitionSpecUpdate).Spec.Equals(expectedSpec))
+ require.Equal(t, -1, builder.updates[1].(*setDefaultSpecUpdate).SpecID)
- metadata, err := builderRef.Build()
+ metadata, err := builder.Build()
require.NoError(t, err)
require.NotNil(t, metadata)
require.Equal(t, 1, metadata.DefaultPartitionSpec())
@@ -217,14 +206,13 @@ func TestSetExistingDefaultPartitionSpec(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, newBuilder)
- newBuilderRef, err := newBuilder.SetDefaultSpecID(0)
- require.NoError(t, err)
+ require.NoError(t, newBuilder.SetDefaultSpecID(0))
- require.True(t, newBuilderRef.HasChanges(), "expected changes after
setting default spec")
- require.Len(t, newBuilderRef.updates, 1, "expected one update")
- require.Equal(t, 0,
newBuilderRef.updates[0].(*setDefaultSpecUpdate).SpecID, "expected default
partition spec to be set to 0")
+ require.True(t, newBuilder.HasChanges(), "expected changes after
setting default spec")
+ require.Len(t, newBuilder.updates, 1, "expected one update")
+ require.Equal(t, 0,
newBuilder.updates[0].(*setDefaultSpecUpdate).SpecID, "expected default
partition spec to be set to 0")
- newBuild, err := newBuilderRef.Build()
+ newBuild, err := newBuilder.Build()
require.NoError(t, err)
require.NotNil(t, newBuild)
require.Equal(t, 0, newBuild.DefaultPartitionSpec(), "expected default
partition spec to be set to 0")
@@ -253,12 +241,11 @@ func TestSetRef(t *testing.T) {
},
SchemaID: &schemaID,
}
- _, err := builder.AddSnapshot(&snapshot)
- require.NoError(t, err)
- _, err = builder.SetSnapshotRef(MainBranch, 10, BranchRef,
WithMinSnapshotsToKeep(10))
+
+ require.NoError(t, builder.AddSnapshot(&snapshot))
+ err := builder.SetSnapshotRef(MainBranch, 10, BranchRef,
WithMinSnapshotsToKeep(10))
require.ErrorContains(t, err, "can't set snapshot ref main to unknown
snapshot 10: snapshot with id 10 not found")
- _, err = builder.SetSnapshotRef(MainBranch, 1, BranchRef,
WithMinSnapshotsToKeep(10))
- require.NoError(t, err)
+ require.NoError(t, builder.SetSnapshotRef(MainBranch, 1, BranchRef,
WithMinSnapshotsToKeep(10)))
require.Len(t, builder.snapshotList, 1)
snap, err := builder.SnapshotByID(1)
require.NoError(t, err)
@@ -279,8 +266,7 @@ func TestAddPartitionSpecForV1RequiresSequentialIDs(t
*testing.T) {
iceberg.AddPartitionFieldBySourceID(3, "z",
iceberg.IdentityTransform{}, builder.CurrentSchema(), &id2))
require.NoError(t, err)
- _, err = builder.AddPartitionSpec(&addedSpec, false)
- require.ErrorContains(t, err, "v1 constraint: partition field IDs are
not sequential: expected 1001, got 1002")
+ require.ErrorContains(t, builder.AddPartitionSpec(&addedSpec, false),
"v1 constraint: partition field IDs are not sequential: expected 1001, got
1002")
}
func TestSetBranchSnapshotCreatesBranchIfNotExists(t *testing.T) {
@@ -304,10 +290,8 @@ func TestSetBranchSnapshotCreatesBranchIfNotExists(t
*testing.T) {
SchemaID: &schemaID,
}
- _, err := builder.AddSnapshot(&snapshot)
- require.NoError(t, err)
- _, err = builder.SetSnapshotRef("new_branch", 2, BranchRef)
- require.NoError(t, err)
+ require.NoError(t, builder.AddSnapshot(&snapshot))
+ require.NoError(t, builder.SetSnapshotRef("new_branch", 2, BranchRef))
meta, err := builder.Build()
require.NoError(t, err)
@@ -343,10 +327,8 @@ func TestCannotAddDuplicateSnapshotID(t *testing.T) {
},
SchemaID: &schemaID,
}
- _, err := builder.AddSnapshot(&snapshot)
- require.NoError(t, err)
- _, err = builder.AddSnapshot(&snapshot)
- require.ErrorContains(t, err, "can't add snapshot with id 2, already
exists")
+ require.NoError(t, builder.AddSnapshot(&snapshot))
+ require.ErrorContains(t, builder.AddSnapshot(&snapshot), "can't add
snapshot with id 2, already exists")
}
func TestRemoveMainSnapshotRef(t *testing.T) {
@@ -360,8 +342,7 @@ func TestRemoveMainSnapshotRef(t *testing.T) {
if _, ok := builder.refs[MainBranch]; !ok {
t.Fatal("expected main branch to exist")
}
- _, err = builder.RemoveSnapshotRef(MainBranch)
- require.NoError(t, err)
+ require.NoError(t, builder.RemoveSnapshotRef(MainBranch))
require.Nil(t, builder.currentSnapshotID)
meta, err = builder.Build()
require.NoError(t, err)
@@ -371,6 +352,5 @@ func TestRemoveMainSnapshotRef(t *testing.T) {
func TestDefaultSpecCannotBeRemoved(t *testing.T) {
builder := builderWithoutChanges(2)
- _, err := builder.RemovePartitionSpecs([]int{0})
- require.ErrorContains(t, err, "can't remove default partition spec with
id 0")
+ require.ErrorContains(t, builder.RemovePartitionSpecs([]int{0}), "can't
remove default partition spec with id 0")
}
diff --git a/table/metadata_internal_test.go b/table/metadata_internal_test.go
index ef1fc6ac..87c3fd55 100644
--- a/table/metadata_internal_test.go
+++ b/table/metadata_internal_test.go
@@ -757,11 +757,9 @@ func TestMetadataBuilderSetDefaultSpecIDLastPartition(t
*testing.T) {
assert.NoError(t, err)
partitionSpec := iceberg.NewPartitionSpecID(0)
- _, err = builder.AddPartitionSpec(&partitionSpec, false)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddPartitionSpec(&partitionSpec, false))
- _, err = builder.SetDefaultSpecID(-1)
- assert.NoError(t, err)
+ assert.NoError(t, builder.SetDefaultSpecID(-1))
assert.Equal(t, 0, builder.defaultSpecID)
}
@@ -769,22 +767,17 @@ func TestMetadataBuilderSetDefaultSpecIDLastPartition(t
*testing.T) {
func TestMetadataBuilderSetLastAddedSchema(t *testing.T) {
builder, err := NewMetadataBuilder()
assert.NoError(t, err)
- _, err = builder.SetFormatVersion(2)
- assert.NoError(t, err)
+ assert.NoError(t, builder.SetFormatVersion(2))
schema := iceberg.NewSchema(1,
iceberg.NestedField{ID: 1, Name: "foo", Type:
iceberg.StringType{}, Required: true},
)
- _, err = builder.AddSchema(schema)
- assert.NoError(t, err)
- _, err = builder.SetCurrentSchemaID(-1)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddSchema(schema))
+ assert.NoError(t, builder.SetCurrentSchemaID(-1))
partitionSpec := iceberg.NewPartitionSpecID(0)
- _, err = builder.AddPartitionSpec(&partitionSpec, false)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddPartitionSpec(&partitionSpec, false))
- _, err = builder.SetDefaultSpecID(-1)
- assert.NoError(t, err)
+ assert.NoError(t, builder.SetDefaultSpecID(-1))
meta, err := builder.Build()
assert.NoError(t, err)
@@ -795,25 +788,21 @@ func TestMetadataBuilderSetLastAddedSchema(t *testing.T) {
func TestMetadataBuilderSchemaIncreasingNumbering(t *testing.T) {
builder, err := NewMetadataBuilder()
assert.NoError(t, err)
- _, err = builder.SetFormatVersion(2)
- assert.NoError(t, err)
+ assert.NoError(t, builder.SetFormatVersion(2))
schema := iceberg.NewSchema(1,
iceberg.NestedField{ID: 1, Name: "foo", Type:
iceberg.StringType{}, Required: true},
)
- _, err = builder.AddSchema(schema)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddSchema(schema))
schema = iceberg.NewSchema(3,
iceberg.NestedField{ID: 3, Name: "foo", Type:
iceberg.StringType{}, Required: true},
)
- _, err = builder.AddSchema(schema)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddSchema(schema))
schema = iceberg.NewSchema(2,
iceberg.NestedField{ID: 4, Name: "foo", Type:
iceberg.StringType{}, Required: true},
)
- _, err = builder.AddSchema(schema)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddSchema(schema))
assert.Equal(t, 1, builder.schemaList[0].ID)
assert.Equal(t, 3, builder.schemaList[1].ID)
@@ -826,13 +815,11 @@ func TestMetadataBuilderReuseSchema(t *testing.T) {
schema := iceberg.NewSchema(1,
iceberg.NestedField{ID: 1, Name: "foo", Type:
iceberg.StringType{}, Required: true},
)
- _, err = builder.AddSchema(schema)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddSchema(schema))
schema2 := iceberg.NewSchema(15,
iceberg.NestedField{ID: 1, Name: "foo", Type:
iceberg.StringType{}, Required: true},
)
- _, err = builder.AddSchema(schema2)
- assert.NoError(t, err)
+ assert.NoError(t, builder.AddSchema(schema2))
assert.Equal(t, len(builder.schemaList), 1)
assert.Equal(t, *builder.lastAddedSchemaID, 1)
}
diff --git a/table/time_travel_test.go b/table/time_travel_test.go
index 0ef34d81..5dd92a1d 100644
--- a/table/time_travel_test.go
+++ b/table/time_travel_test.go
@@ -287,8 +287,7 @@ func createTestMetadata(snapshots []Snapshot, snapshotLog
[]SnapshotLogEntry) (M
// Add snapshots if provided
for _, snapshot := range snapshots {
- builder, err = builder.AddSnapshot(&snapshot)
- if err != nil {
+ if err = builder.AddSnapshot(&snapshot); err != nil {
return nil, err
}
}
diff --git a/table/updates.go b/table/updates.go
index f91bd487..dd867aab 100644
--- a/table/updates.go
+++ b/table/updates.go
@@ -153,9 +153,7 @@ func NewAssignUUIDUpdate(uuid uuid.UUID) *assignUUIDUpdate {
}
func (u *assignUUIDUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetUUID(u.UUID)
-
- return err
+ return builder.SetUUID(u.UUID)
}
type upgradeFormatVersionUpdate struct {
@@ -173,9 +171,7 @@ func NewUpgradeFormatVersionUpdate(formatVersion int)
*upgradeFormatVersionUpdat
}
func (u *upgradeFormatVersionUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetFormatVersion(u.FormatVersion)
-
- return err
+ return builder.SetFormatVersion(u.FormatVersion)
}
type addSchemaUpdate struct {
@@ -193,9 +189,7 @@ func NewAddSchemaUpdate(schema *iceberg.Schema)
*addSchemaUpdate {
}
func (u *addSchemaUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.AddSchema(u.Schema)
-
- return err
+ return builder.AddSchema(u.Schema)
}
type setCurrentSchemaUpdate struct {
@@ -213,9 +207,7 @@ func NewSetCurrentSchemaUpdate(id int)
*setCurrentSchemaUpdate {
}
func (u *setCurrentSchemaUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetCurrentSchemaID(u.SchemaID)
-
- return err
+ return builder.SetCurrentSchemaID(u.SchemaID)
}
type addPartitionSpecUpdate struct {
@@ -236,9 +228,7 @@ func NewAddPartitionSpecUpdate(spec *iceberg.PartitionSpec,
initial bool) *addPa
}
func (u *addPartitionSpecUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.AddPartitionSpec(u.Spec, u.initial)
-
- return err
+ return builder.AddPartitionSpec(u.Spec, u.initial)
}
type setDefaultSpecUpdate struct {
@@ -256,9 +246,7 @@ func NewSetDefaultSpecUpdate(id int) *setDefaultSpecUpdate {
}
func (u *setDefaultSpecUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetDefaultSpecID(u.SpecID)
-
- return err
+ return builder.SetDefaultSpecID(u.SpecID)
}
type addSortOrderUpdate struct {
@@ -279,9 +267,7 @@ func NewAddSortOrderUpdate(sortOrder *SortOrder, initial
bool) *addSortOrderUpda
}
func (u *addSortOrderUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.AddSortOrder(u.SortOrder, u.initial)
-
- return err
+ return builder.AddSortOrder(u.SortOrder, u.initial)
}
type setDefaultSortOrderUpdate struct {
@@ -299,9 +285,7 @@ func NewSetDefaultSortOrderUpdate(id int)
*setDefaultSortOrderUpdate {
}
func (u *setDefaultSortOrderUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetDefaultSortOrderID(u.SortOrderID)
-
- return err
+ return builder.SetDefaultSortOrderID(u.SortOrderID)
}
type addSnapshotUpdate struct {
@@ -318,9 +302,7 @@ func NewAddSnapshotUpdate(snapshot *Snapshot)
*addSnapshotUpdate {
}
func (u *addSnapshotUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.AddSnapshot(u.Snapshot)
-
- return err
+ return builder.AddSnapshot(u.Snapshot)
}
type setSnapshotRefUpdate struct {
@@ -366,14 +348,12 @@ func (u *setSnapshotRefUpdate) Apply(builder
*MetadataBuilder) error {
opts = append(opts,
WithMinSnapshotsToKeep(u.MinSnapshotsToKeep))
}
- _, err := builder.SetSnapshotRef(
+ return builder.SetSnapshotRef(
u.RefName,
u.SnapshotID,
u.RefType,
opts...,
)
-
- return err
}
type setLocationUpdate struct {
@@ -390,9 +370,7 @@ func NewSetLocationUpdate(loc string) Update {
}
func (u *setLocationUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetLoc(u.Location)
-
- return err
+ return builder.SetLoc(u.Location)
}
type setPropertiesUpdate struct {
@@ -410,9 +388,7 @@ func NewSetPropertiesUpdate(updates iceberg.Properties)
*setPropertiesUpdate {
}
func (u *setPropertiesUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.SetProperties(u.Updates)
-
- return err
+ return builder.SetProperties(u.Updates)
}
type removePropertiesUpdate struct {
@@ -431,9 +407,7 @@ func NewRemovePropertiesUpdate(removals []string)
*removePropertiesUpdate {
}
func (u *removePropertiesUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.RemoveProperties(u.Removals)
-
- return err
+ return builder.RemoveProperties(u.Removals)
}
type removeSnapshotsUpdate struct {
@@ -451,9 +425,7 @@ func NewRemoveSnapshotsUpdate(ids []int64)
*removeSnapshotsUpdate {
}
func (u *removeSnapshotsUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.RemoveSnapshots(u.SnapshotIDs)
-
- return err
+ return builder.RemoveSnapshots(u.SnapshotIDs)
}
func (u *removeSnapshotsUpdate) PostCommit(ctx context.Context, preTable
*Table, postTable *Table) error {
@@ -546,9 +518,7 @@ func NewRemoveSnapshotRefUpdate(ref string)
*removeSnapshotRefUpdate {
}
func (u *removeSnapshotRefUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.RemoveSnapshotRef(u.RefName)
-
- return err
+ return builder.RemoveSnapshotRef(u.RefName)
}
type removeSpecUpdate struct {
@@ -566,9 +536,7 @@ func NewRemoveSpecUpdate(specIds []int) *removeSpecUpdate {
}
func (u *removeSpecUpdate) Apply(builder *MetadataBuilder) error {
- _, err := builder.RemovePartitionSpecs(u.SpecIds)
-
- return err
+ return builder.RemovePartitionSpecs(u.SpecIds)
}
type removeSchemasUpdate struct {
