(iceberg-go) branch main updated: fix(glue): reject dropping non-empty namespaces (#1520)

[ze...@apache.org]

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 dc30f413c fix(glue): reject dropping non-empty namespaces (#1520)
dc30f413c is described below

commit dc30f413cc19d22fb22d463f909fee7981d144e8
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Fri Jul 24 20:08:24 2026 +0200

    fix(glue): reject dropping non-empty namespaces (#1520)
    
    ## Summary
    
    - check a Glue database for tables before dropping its namespace
    - return `catalog.ErrNamespaceNotEmpty` when any table exists
    - avoid deleting the database when Glue cannot determine whether it is
    empty
    
    The check includes both Iceberg and non-Iceberg tables because deleting
    a Glue database makes all tables in that database inaccessible.
    
    ## Testing
    
    - `go test ./catalog/glue`
---
 catalog/glue/glue.go      | 19 +++++++++++
 catalog/glue/glue_test.go | 84 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/catalog/glue/glue.go b/catalog/glue/glue.go
index 144e7277d..4957464de 100644
--- a/catalog/glue/glue.go
+++ b/catalog/glue/glue.go
@@ -685,6 +685,25 @@ func (c *Catalog) DropNamespace(ctx context.Context, namespace table.Identifier)
 		return err
 	}
 
+	// Glue has no conditional database delete, so a concurrent table creation can still race this check.
+	tables, err := c.glueSvc.GetTables(ctx, &glue.GetTablesInput{
+		CatalogId:    c.catalogId,
+		DatabaseName: aws.String(databaseName),
+		MaxResults:   aws.Int32(1),
+	})
+	if err != nil {
+		return fmt.Errorf("failed to list tables in namespace %s: %w", databaseName, err)
+	}
+	if len(tables.TableList) > 0 {
+		tableType := "non-Iceberg"
+		if strings.EqualFold(tables.TableList[0].Parameters[tableParamTableType], glueTypeIceberg) {
+			tableType = "Iceberg"
+		}
+
+		return fmt.Errorf("%w: cannot drop namespace %s because it still contains %s tables",
+			catalog.ErrNamespaceNotEmpty, databaseName, tableType)
+	}
+
 	params := &glue.DeleteDatabaseInput{CatalogId: c.catalogId, Name: aws.String(databaseName)}
 	_, err = c.glueSvc.DeleteDatabase(ctx, params)
 	if err != nil {
diff --git a/catalog/glue/glue_test.go b/catalog/glue/glue_test.go
index 1cf9a5cbc..d2e0d9cd0 100644
--- a/catalog/glue/glue_test.go
+++ b/catalog/glue/glue_test.go
@@ -892,12 +892,12 @@ func TestGlueCreateNamespace(t *testing.T) {
 }
 
 func TestGlueDropNamespace(t *testing.T) {
-	assert := require.New(t)
-
 	mockGlueSvc := &mockGlueClient{}
+	catalogID := aws.String("test_catalog")
 
 	mockGlueSvc.On("GetDatabase", mock.Anything, &glue.GetDatabaseInput{
-		Name: aws.String("test_namespace"),
+		CatalogId: catalogID,
+		Name:      aws.String("test_namespace"),
 	}, mock.Anything).Return(&glue.GetDatabaseOutput{
 		Database: &types.Database{
 			Name: aws.String("test_namespace"),
@@ -907,16 +907,88 @@ func TestGlueDropNamespace(t *testing.T) {
 		},
 	}, nil).Once()
 
+	mockGlueSvc.On("GetTables", mock.Anything, &glue.GetTablesInput{
+		CatalogId:    catalogID,
+		DatabaseName: aws.String("test_namespace"),
+		MaxResults:   aws.Int32(1),
+	}, mock.Anything).Return(&glue.GetTablesOutput{}, nil).Once()
+
 	mockGlueSvc.On("DeleteDatabase", mock.Anything, &glue.DeleteDatabaseInput{
-		Name: aws.String("test_namespace"),
+		CatalogId: catalogID,
+		Name:      aws.String("test_namespace"),
 	}, mock.Anything).Return(&glue.DeleteDatabaseOutput{}, nil).Once()
 
 	glueCatalog := &Catalog{
-		glueSvc: mockGlueSvc,
+		glueSvc:   mockGlueSvc,
+		catalogId: catalogID,
 	}
 
 	err := glueCatalog.DropNamespace(context.TODO(), DatabaseIdentifier("test_namespace"))
-	assert.NoError(err)
+	require.NoError(t, err)
+	mockGlueSvc.AssertExpectations(t)
+}
+
+func TestGlueDropNamespaceNotEmpty(t *testing.T) {
+	tests := []struct {
+		name            string
+		tableType       string
+		expectedMessage string
+	}{
+		{name: "iceberg table", tableType: glueTypeIceberg, expectedMessage: "still contains Iceberg tables"},
+		{name: "non-Iceberg table", tableType: glueTableType, expectedMessage: "still contains non-Iceberg tables"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockGlueSvc := &mockGlueClient{}
+
+			mockGlueSvc.On("GetDatabase", mock.Anything, &glue.GetDatabaseInput{
+				Name: aws.String("test_namespace"),
+			}, mock.Anything).Return(&glue.GetDatabaseOutput{
+				Database: &types.Database{Name: aws.String("test_namespace")},
+			}, nil).Once()
+			mockGlueSvc.On("GetTables", mock.Anything, &glue.GetTablesInput{
+				DatabaseName: aws.String("test_namespace"),
+				MaxResults:   aws.Int32(1),
+			}, mock.Anything).Return(&glue.GetTablesOutput{
+				TableList: []types.Table{{
+					Name:       aws.String("test_table"),
+					Parameters: map[string]string{tableParamTableType: tt.tableType},
+				}},
+			}, nil).Once()
+
+			glueCatalog := &Catalog{glueSvc: mockGlueSvc}
+			err := glueCatalog.DropNamespace(context.TODO(), DatabaseIdentifier("test_namespace"))
+
+			require.ErrorIs(t, err, catalog.ErrNamespaceNotEmpty)
+			require.ErrorContains(t, err, tt.expectedMessage)
+			mockGlueSvc.AssertNotCalled(t, "DeleteDatabase", mock.Anything, mock.Anything, mock.Anything)
+			mockGlueSvc.AssertExpectations(t)
+		})
+	}
+}
+
+func TestGlueDropNamespaceGetTablesError(t *testing.T) {
+	mockGlueSvc := &mockGlueClient{}
+	errGetTables := errors.New("get tables failed")
+
+	mockGlueSvc.On("GetDatabase", mock.Anything, &glue.GetDatabaseInput{
+		Name: aws.String("test_namespace"),
+	}, mock.Anything).Return(&glue.GetDatabaseOutput{
+		Database: &types.Database{Name: aws.String("test_namespace")},
+	}, nil).Once()
+	mockGlueSvc.On("GetTables", mock.Anything, &glue.GetTablesInput{
+		DatabaseName: aws.String("test_namespace"),
+		MaxResults:   aws.Int32(1),
+	}, mock.Anything).Return(&glue.GetTablesOutput{}, errGetTables).Once()
+
+	glueCatalog := &Catalog{glueSvc: mockGlueSvc}
+	err := glueCatalog.DropNamespace(context.TODO(), DatabaseIdentifier("test_namespace"))
+
+	require.ErrorIs(t, err, errGetTables)
+	require.ErrorContains(t, err, "failed to list tables in namespace test_namespace")
+	mockGlueSvc.AssertNotCalled(t, "DeleteDatabase", mock.Anything, mock.Anything, mock.Anything)
+	mockGlueSvc.AssertExpectations(t)
 }
 
 func TestGlueCheckNamespaceExists(t *testing.T) {