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 941d301d feat(metadata): prevent reserved table properties from being
set / removed (#572)
941d301d is described below
commit 941d301dfafcf9c2155a360fcfd3f3e0388bbb92
Author: Tobias Pütz <tob...@minio.io>
AuthorDate: Mon Sep 22 22:07:43 2025 +0200
feat(metadata): prevent reserved table properties from being set / removed
(#572)
---
catalog/sql/sql_test.go | 4 +--
table/metadata.go | 16 ++++++++--
table/metadata_builder_internal_test.go | 54 +++++++++++++++++++++++++++++++++
table/metadata_internal_test.go | 2 +-
table/properties.go | 25 +++++++++++++++
table/table_test.go | 18 +++++------
6 files changed, 104 insertions(+), 15 deletions(-)
diff --git a/catalog/sql/sql_test.go b/catalog/sql/sql_test.go
index 2581486c..b4b061a1 100644
--- a/catalog/sql/sql_test.go
+++ b/catalog/sql/sql_test.go
@@ -386,7 +386,7 @@ func (s *SqliteCatalogTestSuite) TestCreateV1Table() {
ns := catalog.NamespaceFromIdent(tt.tblID)
s.Require().NoError(tt.cat.CreateNamespace(context.Background(), ns, nil))
tbl, err := tt.cat.CreateTable(context.Background(), tt.tblID,
tableSchemaNested,
-
catalog.WithProperties(iceberg.Properties{"format-version": "1"}))
+
catalog.WithProperties(iceberg.Properties{table.PropertyFormatVersion: "1"}))
s.Require().NoError(err)
s.FileExists(strings.TrimPrefix(tbl.MetadataLocation(),
"file://"))
@@ -441,7 +441,7 @@ func (s *SqliteCatalogTestSuite)
TestCreateDuplicatedTable() {
ns := catalog.NamespaceFromIdent(tt.tblID)
s.Require().NoError(tt.cat.CreateNamespace(context.Background(), ns, nil))
_, err := tt.cat.CreateTable(context.Background(), tt.tblID,
tableSchemaNested,
-
catalog.WithProperties(iceberg.Properties{"format-version": "1"}))
+
catalog.WithProperties(iceberg.Properties{table.PropertyFormatVersion: "1"}))
s.Require().NoError(err)
_, err = tt.cat.CreateTable(context.Background(), tt.tblID,
tableSchemaNested)
diff --git a/table/metadata.go b/table/metadata.go
index bae44694..f59f11a3 100644
--- a/table/metadata.go
+++ b/table/metadata.go
@@ -415,7 +415,11 @@ func (b *MetadataBuilder) RemoveProperties(keys []string)
error {
if len(keys) == 0 {
return nil
}
-
+ for _, reserved := range ReservedProperties {
+ if slices.Contains(keys, reserved) {
+ return fmt.Errorf("can't remove reserved property %s",
reserved)
+ }
+ }
b.updates = append(b.updates, NewRemovePropertiesUpdate(keys))
for _, key := range keys {
delete(b.props, key)
@@ -543,6 +547,12 @@ func (b *MetadataBuilder) SetProperties(props
iceberg.Properties) error {
return nil
}
+ for _, key := range ReservedProperties {
+ if _, ok := props[key]; ok {
+ return fmt.Errorf("can't set reserved property %s", key)
+ }
+ }
+
b.updates = append(b.updates, NewSetPropertiesUpdate(props))
if b.props == nil {
b.props = props
@@ -1443,12 +1453,12 @@ func NewMetadataWithUUID(sc *iceberg.Schema, partitions
*iceberg.PartitionSpec,
var err error
formatVersion := DefaultFormatVersion
if props != nil {
- verStr, ok := props["format-version"]
+ verStr, ok := props[PropertyFormatVersion]
if ok {
if formatVersion, err = strconv.Atoi(verStr); err !=
nil {
formatVersion = DefaultFormatVersion
}
- delete(props, "format-version")
+ delete(props, PropertyFormatVersion)
}
}
diff --git a/table/metadata_builder_internal_test.go
b/table/metadata_builder_internal_test.go
index 0c733110..3c5fc985 100644
--- a/table/metadata_builder_internal_test.go
+++ b/table/metadata_builder_internal_test.go
@@ -442,3 +442,57 @@ func TestDefaultSpecCannotBeRemoved(t *testing.T) {
require.ErrorContains(t, builder.RemovePartitionSpecs([]int{0}), "can't
remove default partition spec with id 0")
}
+
+func TestSetReservedPropertiesFails(t *testing.T) {
+ builder := builderWithoutChanges(2)
+
+ // Test that setting non-reserved properties works
+ err := builder.SetProperties(iceberg.Properties{
+ "custom-property": "value1",
+ "another-custom-property": "value2",
+ })
+ require.NoError(t, err)
+ require.True(t, builder.HasChanges())
+
+ // Test setting each reserved property individually
+ for _, reserved := range ReservedProperties {
+ err := builder.SetProperties(iceberg.Properties{reserved:
"some-value"})
+ require.ErrorContains(t, err, "can't set reserved property
"+reserved)
+ }
+
+ // Test setting multiple properties where one is reserved
+ err = builder.SetProperties(iceberg.Properties{
+ "custom-property": "allowed",
+ PropertyCurrentSnapshotId: "12345",
+ "another-custom-property": "also-allowed",
+ })
+ require.ErrorContains(t, err, "can't set reserved property
"+PropertyCurrentSnapshotId)
+}
+
+func TestRemoveReservedPropertiesFails(t *testing.T) {
+ builder := builderWithoutChanges(2)
+
+ // Test removing each reserved property individually
+ for _, reserved := range ReservedProperties {
+ err := builder.RemoveProperties([]string{reserved})
+ require.ErrorContains(t, err, "can't remove reserved property
"+reserved)
+ }
+
+ // Test removing multiple properties where one is reserved
+ err := builder.RemoveProperties([]string{
+ "custom-property",
+ PropertyUuid,
+ "another-custom-property",
+ })
+ require.ErrorContains(t, err, "can't remove reserved property
"+PropertyUuid)
+
+ // Add some custom properties first, then test that removing
non-reserved properties works
+ require.NoError(t, builder.SetProperties(iceberg.Properties{
+ "custom-property": "value1",
+ "another-custom-property": "value2",
+ }))
+
+ err = builder.RemoveProperties([]string{"custom-property"})
+ require.NoError(t, err)
+ require.True(t, builder.HasChanges())
+}
diff --git a/table/metadata_internal_test.go b/table/metadata_internal_test.go
index 78bc5fe0..6863b201 100644
--- a/table/metadata_internal_test.go
+++ b/table/metadata_internal_test.go
@@ -560,7 +560,7 @@ func TestNewMetadataWithExplicitV1Format(t *testing.T) {
}},
}
- actual, err := NewMetadata(schema, &partitionSpec, sortOrder,
"s3://some_v1_location/", iceberg.Properties{"format-version": "1"})
+ actual, err := NewMetadata(schema, &partitionSpec, sortOrder,
"s3://some_v1_location/", iceberg.Properties{PropertyFormatVersion: "1"})
require.NoError(t, err)
expectedSchema := iceberg.NewSchemaWithIdentifiers(0, []int{2},
diff --git a/table/properties.go b/table/properties.go
index bb3c2b08..8ef3d026 100644
--- a/table/properties.go
+++ b/table/properties.go
@@ -85,3 +85,28 @@ const (
MaxRefAgeMsKey = "max-ref-age-ms"
MaxRefAgeMsDefault = math.MaxInt
)
+
+// Reserved properties
+const (
+ PropertyFormatVersion = "format-version"
+ PropertyUuid = "uuid"
+ PropertySnapshotCount = "snapshot-count"
+ PropertyCurrentSnapshotId = "current-snapshot-id"
+ PropertyCurrentSnapshotSummary = "current-snapshot-summary"
+ PropertyCurrentSnapshotTimestamp = "current-snapshot-timestamp"
+ PropertyCurrentSchema = "current-schema"
+ PropertyDefaultPartitionSpec = "default-partition-spec"
+ PropertyDefaultSortOrder = "default-sort-order"
+)
+
+var ReservedProperties = [9]string{
+ PropertyFormatVersion,
+ PropertyUuid,
+ PropertySnapshotCount,
+ PropertyCurrentSnapshotId,
+ PropertyCurrentSnapshotSummary,
+ PropertyCurrentSnapshotTimestamp,
+ PropertyCurrentSchema,
+ PropertyDefaultPartitionSpec,
+ PropertyDefaultSortOrder,
+}
diff --git a/table/table_test.go b/table/table_test.go
index 93050048..694e860b 100644
--- a/table/table_test.go
+++ b/table/table_test.go
@@ -318,7 +318,7 @@ func (t *TableWritingTestSuite) writeParquet(fio
iceio.WriteFileIO, filePath str
func (t *TableWritingTestSuite) createTable(identifier table.Identifier,
formatVersion int, spec iceberg.PartitionSpec, sc *iceberg.Schema) *table.Table
{
meta, err := table.NewMetadata(sc, &spec, table.UnsortedSortOrder,
- t.location, iceberg.Properties{"format-version":
strconv.Itoa(formatVersion)})
+ t.location, iceberg.Properties{table.PropertyFormatVersion:
strconv.Itoa(formatVersion)})
t.Require().NoError(err)
return table.New(
@@ -820,7 +820,7 @@ func (t *TableWritingTestSuite) TestReplaceDataFiles() {
ident := table.Identifier{"default", "replace_data_files_v" +
strconv.Itoa(t.formatVersion)}
meta, err := table.NewMetadata(t.tableSchemaPromotedTypes,
iceberg.UnpartitionedSpec,
- table.UnsortedSortOrder, t.location,
iceberg.Properties{"format-version": strconv.Itoa(t.formatVersion)})
+ table.UnsortedSortOrder, t.location,
iceberg.Properties{table.PropertyFormatVersion: strconv.Itoa(t.formatVersion)})
t.Require().NoError(err)
ctx := context.Background()
@@ -902,7 +902,7 @@ func (t *TableWritingTestSuite) TestExpireSnapshots() {
ident := table.Identifier{"default", "replace_data_files_v" +
strconv.Itoa(t.formatVersion)}
meta, err := table.NewMetadata(t.tableSchemaPromotedTypes,
iceberg.UnpartitionedSpec,
- table.UnsortedSortOrder, t.location,
iceberg.Properties{"format-version": strconv.Itoa(t.formatVersion)})
+ table.UnsortedSortOrder, t.location,
iceberg.Properties{table.PropertyFormatVersion: strconv.Itoa(t.formatVersion)})
t.Require().NoError(err)
ctx := context.Background()
@@ -1152,7 +1152,7 @@ func (t *TableWritingTestSuite) TestMergeManifests() {
table.ParquetCompressionKey: "snappy",
table.ManifestMergeEnabledKey: "true",
table.ManifestMinMergeCountKey: "1",
- "format-version":
strconv.Itoa(t.formatVersion),
+ table.PropertyFormatVersion:
strconv.Itoa(t.formatVersion),
}, tableSchema())
tblB := t.createTableWithProps(table.Identifier{"default",
"merge_manifest_b"},
@@ -1161,14 +1161,14 @@ func (t *TableWritingTestSuite) TestMergeManifests() {
table.ManifestMergeEnabledKey: "true",
table.ManifestMinMergeCountKey: "1",
table.ManifestTargetSizeBytesKey: "1",
- "format-version":
strconv.Itoa(t.formatVersion),
+ table.PropertyFormatVersion:
strconv.Itoa(t.formatVersion),
}, tableSchema())
tblC := t.createTableWithProps(table.Identifier{"default",
"merge_manifest_c"},
iceberg.Properties{
table.ParquetCompressionKey: "snappy",
table.ManifestMinMergeCountKey: "1",
- "format-version":
strconv.Itoa(t.formatVersion),
+ table.PropertyFormatVersion:
strconv.Itoa(t.formatVersion),
}, tableSchema())
arrTable := arrowTableWithNull()
@@ -1300,7 +1300,7 @@ func TestNullableStructRequiredField(t *testing.T) {
require.NoError(t, cat.CreateNamespace(t.Context(),
table.Identifier{"testing"}, nil))
tbl, err := cat.CreateTable(t.Context(), table.Identifier{"testing",
"nullable_struct_required_field"}, sc,
- catalog.WithProperties(iceberg.Properties{"format-version":
"2"}),
+
catalog.WithProperties(iceberg.Properties{table.PropertyFormatVersion: "2"}),
catalog.WithLocation("file://"+loc))
require.NoError(t, err)
require.NotNil(t, tbl)
@@ -1397,7 +1397,7 @@ func (t *TableWritingTestSuite)
TestDeleteOldMetadataLogsErrorOnFileNotFound() {
ident := table.Identifier{"default", "file_v" +
strconv.Itoa(t.formatVersion)}
meta, err := table.NewMetadata(t.tableSchemaPromotedTypes,
iceberg.UnpartitionedSpec,
- table.UnsortedSortOrder, t.location,
iceberg.Properties{"format-version": strconv.Itoa(t.formatVersion),
"write.metadata.delete-after-commit.enabled": "true"})
+ table.UnsortedSortOrder, t.location,
iceberg.Properties{table.PropertyFormatVersion: strconv.Itoa(t.formatVersion),
"write.metadata.delete-after-commit.enabled": "true"})
t.Require().NoError(err)
tbl := table.New(ident, meta, t.getMetadataLoc(), func(ctx
context.Context) (iceio.IO, error) {
@@ -1443,7 +1443,7 @@ func (t *TableWritingTestSuite)
TestDeleteOldMetadataNoErrorLogsOnFileFound() {
}
ident := table.Identifier{"default", "file_v" +
strconv.Itoa(t.formatVersion)}
- meta, err := table.NewMetadata(t.tableSchemaPromotedTypes,
iceberg.UnpartitionedSpec, table.UnsortedSortOrder, t.location,
iceberg.Properties{"format-version": strconv.Itoa(t.formatVersion),
"write.metadata.delete-after-commit.enabled": "true"})
+ meta, err := table.NewMetadata(t.tableSchemaPromotedTypes,
iceberg.UnpartitionedSpec, table.UnsortedSortOrder, t.location,
iceberg.Properties{table.PropertyFormatVersion: strconv.Itoa(t.formatVersion),
"write.metadata.delete-after-commit.enabled": "true"})
t.Require().NoError(err)
tbl := table.New(
