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 ec3a60fbc test(catalog): assert unsupported namespace properties
instead of skipping (#2237)
ec3a60fbc is described below
commit ec3a60fbc6e407bddf0641e353339a0ceb5ab474
Author: Richie Bachala <[email protected]>
AuthorDate: Tue Oct 20 12:50:20 2026 -0700
test(catalog): assert unsupported namespace properties instead of skipping
(#2237)
Catalogs that set SupportsNamespaceProperties to false now run
NamespacePropertiesNotSupported in place of the property-write tests.
It asserts that CreateNamespace with properties and
UpdateNamespaceProperties
return iceberg.ErrNotImplemented and that the rejected calls change nothing,
mirroring RenameTableNotSupported.
The Hadoop catalog now wraps both errors in iceberg.ErrNotImplemented so
callers can detect the unsupported case with errors.Is.
Signed-off-by: Richie Bachala <[email protected]>
---
catalog/catalogtest/catalogtest.go | 27 +++++++++++--------
catalog/catalogtest/namespaces.go | 54 ++++++++++++++++++++++----------------
catalog/hadoop/hadoop.go | 4 +--
catalog/hadoop/hadoop_test.go | 6 ++---
4 files changed, 53 insertions(+), 38 deletions(-)
diff --git a/catalog/catalogtest/catalogtest.go
b/catalog/catalogtest/catalogtest.go
index 22b25ae08..d71819a78 100644
--- a/catalog/catalogtest/catalogtest.go
+++ b/catalog/catalogtest/catalogtest.go
@@ -84,10 +84,11 @@ type Config struct {
NewCatalog func(t *testing.T) catalog.Catalog
// SupportsNamespaceProperties reports whether the catalog can store
- // properties against a namespace. Tests that write properties are
skipped
- // when it is false, as they are for the Hadoop catalog, whose
namespaces
- // are directories with nowhere to keep them. Loading is required of
every
- // catalog and is tested either way.
+ // properties against a namespace. When it is false, as it is for the
+ // Hadoop catalog, whose namespaces are directories with nowhere to keep
+ // them, the tests that write properties are replaced by one asserting
that
+ // writing them returns iceberg.ErrNotImplemented and changes nothing.
+ // Loading is required of every catalog and is tested either way.
SupportsNamespaceProperties bool
// SupportsRenameTable reports whether the catalog can rename tables.
@@ -129,14 +130,18 @@ func RunCatalogTests(t *testing.T, cfg Config) {
t.Run("DropNamespaceNotEmpty", func(t *testing.T) {
testDropNamespaceNotEmpty(t, cfg) })
t.Run("ListNamespaces", func(t *testing.T) { testListNamespaces(t, cfg)
})
- t.Run("CreateNamespaceWithProperties", func(t *testing.T) {
testCreateNamespaceWithProperties(t, cfg) })
t.Run("LoadNamespaceProperties", func(t *testing.T) {
testLoadNamespaceProperties(t, cfg) })
- t.Run("SetNamespaceProperties", func(t *testing.T) {
testSetNamespaceProperties(t, cfg) })
- t.Run("UpdateNamespaceProperties", func(t *testing.T) {
testUpdateNamespaceProperties(t, cfg) })
- t.Run("UpdateAndSetNamespaceProperties", func(t *testing.T) {
testUpdateAndSetNamespaceProperties(t, cfg) })
- t.Run("RemoveNamespaceProperties", func(t *testing.T) {
testRemoveNamespaceProperties(t, cfg) })
- t.Run("SetNamespacePropertiesNamespaceDoesNotExist", func(t *testing.T)
{ testSetNamespacePropertiesNamespaceDoesNotExist(t, cfg) })
- t.Run("RemoveNamespacePropertiesNamespaceDoesNotExist", func(t
*testing.T) { testRemoveNamespacePropertiesNamespaceDoesNotExist(t, cfg) })
+ if cfg.SupportsNamespaceProperties {
+ t.Run("CreateNamespaceWithProperties", func(t *testing.T) {
testCreateNamespaceWithProperties(t, cfg) })
+ t.Run("SetNamespaceProperties", func(t *testing.T) {
testSetNamespaceProperties(t, cfg) })
+ t.Run("UpdateNamespaceProperties", func(t *testing.T) {
testUpdateNamespaceProperties(t, cfg) })
+ t.Run("UpdateAndSetNamespaceProperties", func(t *testing.T) {
testUpdateAndSetNamespaceProperties(t, cfg) })
+ t.Run("RemoveNamespaceProperties", func(t *testing.T) {
testRemoveNamespaceProperties(t, cfg) })
+ t.Run("SetNamespacePropertiesNamespaceDoesNotExist", func(t
*testing.T) { testSetNamespacePropertiesNamespaceDoesNotExist(t, cfg) })
+ t.Run("RemoveNamespacePropertiesNamespaceDoesNotExist", func(t
*testing.T) { testRemoveNamespacePropertiesNamespaceDoesNotExist(t, cfg) })
+ } else {
+ t.Run("NamespacePropertiesNotSupported", func(t *testing.T) {
testNamespacePropertiesNotSupported(t, cfg) })
+ }
}
// testBasicCreateTable asserts that a newly created table is visible to the
diff --git a/catalog/catalogtest/namespaces.go
b/catalog/catalogtest/namespaces.go
index c4cbcc315..b49dde99d 100644
--- a/catalog/catalogtest/namespaces.go
+++ b/catalog/catalogtest/namespaces.go
@@ -168,22 +168,44 @@ func testListNamespaces(t *testing.T, cfg Config) {
assert.Contains(t, namespaces, second, "remaining namespace should
still be listed")
}
-// skipWithoutNamespaceProperties exempts catalogs that cannot store namespace
-// properties from the tests that write them.
-func skipWithoutNamespaceProperties(t *testing.T, cfg Config) {
- t.Helper()
-
- if !cfg.SupportsNamespaceProperties {
- t.Skip("catalog does not support namespace properties")
- }
+// testNamespacePropertiesNotSupported asserts that a catalog which cannot
store
+// namespace properties rejects writing them with iceberg.ErrNotImplemented and
+// that the rejected call changes nothing: creating with properties must not
+// leave a namespace behind, and a rejected update must not alter the
+// properties already loaded for the namespace.
+func testNamespacePropertiesNotSupported(t *testing.T, cfg Config) {
+ ctx := context.Background()
+ cat := cfg.NewCatalog(t)
+ namespace, _ := newIdentifiers()
+
+ err := cat.CreateNamespace(ctx, namespace, iceberg.Properties{"prop":
"val"})
+ require.ErrorIs(t, err, iceberg.ErrNotImplemented)
+
+ exists, err := cat.CheckNamespaceExists(ctx, namespace)
+ require.NoError(t, err)
+ require.False(t, exists, "rejected create should not leave a namespace
behind")
+
+ require.NoError(t, cat.CreateNamespace(ctx, namespace, nil))
+ t.Cleanup(func() { _ = cat.DropNamespace(ctx, namespace) })
+
+ before, err := cat.LoadNamespaceProperties(ctx, namespace)
+ require.NoError(t, err)
+
+ _, err = cat.UpdateNamespaceProperties(ctx, namespace, nil,
iceberg.Properties{"prop": "val"})
+ require.ErrorIs(t, err, iceberg.ErrNotImplemented)
+
+ _, err = cat.UpdateNamespaceProperties(ctx, namespace,
[]string{"location"}, nil)
+ require.ErrorIs(t, err, iceberg.ErrNotImplemented)
+
+ after, err := cat.LoadNamespaceProperties(ctx, namespace)
+ require.NoError(t, err)
+ assert.Equal(t, before, after, "rejected updates should leave
properties unchanged")
}
// testCreateNamespaceWithProperties asserts that properties passed to
// CreateNamespace are readable afterwards. A catalog may add properties of its
// own, so only the created one is checked.
func testCreateNamespaceWithProperties(t *testing.T, cfg Config) {
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
@@ -219,8 +241,6 @@ func testLoadNamespaceProperties(t *testing.T, cfg Config) {
// testSetNamespaceProperties asserts that properties set on an existing
// namespace are reported as updated and are readable afterwards.
func testSetNamespaceProperties(t *testing.T, cfg Config) {
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
@@ -242,8 +262,6 @@ func testSetNamespaceProperties(t *testing.T, cfg Config) {
// testUpdateNamespaceProperties asserts that setting a property that already
// has a value overwrites it.
func testUpdateNamespaceProperties(t *testing.T, cfg Config) {
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
@@ -266,8 +284,6 @@ func testUpdateNamespaceProperties(t *testing.T, cfg
Config) {
// testUpdateAndSetNamespaceProperties asserts that a single update may both
// overwrite an existing property and add a new one.
func testUpdateAndSetNamespaceProperties(t *testing.T, cfg Config) {
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
@@ -291,8 +307,6 @@ func testUpdateAndSetNamespaceProperties(t *testing.T, cfg
Config) {
// testRemoveNamespaceProperties asserts that a removed property is gone and
// that the properties left alone survive.
func testRemoveNamespaceProperties(t *testing.T, cfg Config) {
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
@@ -318,8 +332,6 @@ func testRemoveNamespaceProperties(t *testing.T, cfg
Config) {
// testSetNamespacePropertiesNamespaceDoesNotExist asserts that setting
// properties on a namespace that was never created is an error.
func testSetNamespacePropertiesNamespaceDoesNotExist(t *testing.T, cfg Config)
{
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
@@ -331,8 +343,6 @@ func testSetNamespacePropertiesNamespaceDoesNotExist(t
*testing.T, cfg Config) {
// testRemoveNamespacePropertiesNamespaceDoesNotExist asserts that removing
// properties from a namespace that was never created is an error.
func testRemoveNamespacePropertiesNamespaceDoesNotExist(t *testing.T, cfg
Config) {
- skipWithoutNamespaceProperties(t, cfg)
-
ctx := context.Background()
cat := cfg.NewCatalog(t)
namespace, _ := newIdentifiers()
diff --git a/catalog/hadoop/hadoop.go b/catalog/hadoop/hadoop.go
index ecc718aa6..3232fbee8 100644
--- a/catalog/hadoop/hadoop.go
+++ b/catalog/hadoop/hadoop.go
@@ -937,7 +937,7 @@ func (c *Catalog) CreateNamespace(_ context.Context, ns
table.Identifier, props
}
if len(props) > 0 {
- return errors.New("hadoop catalog: namespace properties are not
supported")
+ return fmt.Errorf("%w: hadoop catalog: namespace properties are
not supported", iceberg.ErrNotImplemented)
}
// Raise an error if the namespace already exists
@@ -1113,7 +1113,7 @@ func (c *Catalog) LoadNamespaceProperties(_
context.Context, ns table.Identifier
}
func (c *Catalog) UpdateNamespaceProperties(_ context.Context, _
table.Identifier, _ []string, _ iceberg.Properties)
(catalog.PropertiesUpdateSummary, error) {
- return catalog.PropertiesUpdateSummary{}, errors.New("hadoop catalog:
UpdateNamespaceProperties not yet implemented")
+ return catalog.PropertiesUpdateSummary{}, fmt.Errorf("%w: hadoop
catalog: namespace properties are not supported", iceberg.ErrNotImplemented)
}
// checkedMkdirAll is a helper function that checks
diff --git a/catalog/hadoop/hadoop_test.go b/catalog/hadoop/hadoop_test.go
index 93d1b9e74..3baa3ede0 100644
--- a/catalog/hadoop/hadoop_test.go
+++ b/catalog/hadoop/hadoop_test.go
@@ -936,7 +936,7 @@ func (s *HadoopCatalogTestSuite)
TestCreateNamespaceNestedParentMissing() {
func (s *HadoopCatalogTestSuite) TestCreateNamespaceWithProperties() {
err := s.cat.CreateNamespace(context.Background(), []string{"ns"},
iceberg.Properties{"key": "val"})
- s.Require().Error(err)
+ s.Require().ErrorIs(err, iceberg.ErrNotImplemented)
s.Contains(err.Error(), "properties are not supported")
}
@@ -1131,8 +1131,8 @@ func (s *HadoopCatalogTestSuite)
TestLoadNamespacePropertiesFileNotDir() {
func (s *HadoopCatalogTestSuite) TestUpdateNamespacePropertiesUnsupported() {
_, err := s.cat.UpdateNamespaceProperties(context.Background(),
[]string{"ns"}, nil, nil)
- s.Require().Error(err)
- s.Contains(err.Error(), "not yet implemented")
+ s.Require().ErrorIs(err, iceberg.ErrNotImplemented)
+ s.Contains(err.Error(), "properties are not supported")
}
func (s *HadoopCatalogTestSuite) TestDropNamespaceWithRegularFilesOnly() {
