You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ke...@apache.org on 2025/06/12 21:47:23 UTC

(iceberg-python) branch main updated: fix: glue drop_namespace to check non-iceberg tables (#2083)

This is an automated email from the ASF dual-hosted git repository.

kevinjqliu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git


The following commit(s) were added to refs/heads/main by this push:
     new 47613d3f fix: glue drop_namespace to check non-iceberg tables (#2083)
47613d3f is described below

commit 47613d3fb0474f186dc37374d6ed4883bf1ae2c0
Author: Drew Gallardo <im...@gmail.com>
AuthorDate: Thu Jun 12 14:47:17 2025 -0700

    fix: glue drop_namespace to check non-iceberg tables (#2083)
    
    # Rationale for this change
    
    This PR is adding a check for all tables in the GlueCatalog instead of
    just iceberg tables.
    
    GlueCatalog allows users to store other table formats under the same
    database. Using the catalog's `list_tables()` call filters out
    non-Iceberg table types, which avoids the proper Iceberg
    `NamespaceNotEmptyError` and instead returns a generic Glue SDK error.
    
    This change aligns with the behavior in the java implementation:
    -
    https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java#L538-L571
    
    # Are these changes tested?
    
    Tested locally with Glue Catalog
    
    # Are there any user-facing changes?
    
    Yes new exception message when a `drop_namespace()` call is made against
    a database with non-iceberg tables.
---
 pyiceberg/catalog/glue.py  | 14 ++++++++++----
 tests/catalog/test_glue.py | 12 ++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pyiceberg/catalog/glue.py b/pyiceberg/catalog/glue.py
index adfcc910..25e7d3f7 100644
--- a/pyiceberg/catalog/glue.py
+++ b/pyiceberg/catalog/glue.py
@@ -687,13 +687,19 @@ class GlueCatalog(MetastoreCatalog):
         """
         database_name = self.identifier_to_database(namespace, NoSuchNamespaceError)
         try:
-            table_list = self.list_tables(namespace=database_name)
-        except NoSuchNamespaceError as e:
+            table_list_response = self.glue.get_tables(DatabaseName=database_name)
+            table_list = table_list_response["TableList"]
+        except self.glue.exceptions.EntityNotFoundException as e:
             raise NoSuchNamespaceError(f"Database does not exist: {database_name}") from e
 
         if len(table_list) > 0:
-            raise NamespaceNotEmptyError(f"Database {database_name} is not empty")
-
+            first_table = table_list[0]
+            if self.__is_iceberg_table(first_table):
+                raise NamespaceNotEmptyError(f"Cannot drop namespace {database_name} because it still contains Iceberg tables")
+            else:
+                raise NamespaceNotEmptyError(
+                    f"Cannot drop namespace {database_name} because it still contains non-Iceberg tables"
+                )
         self.glue.delete_database(Name=database_name)
 
     def list_tables(self, namespace: Union[str, Identifier]) -> List[Identifier]:
diff --git a/tests/catalog/test_glue.py b/tests/catalog/test_glue.py
index df1734eb..399c9e8b 100644
--- a/tests/catalog/test_glue.py
+++ b/tests/catalog/test_glue.py
@@ -548,6 +548,18 @@ def test_drop_non_empty_namespace(
         test_catalog.drop_namespace(database_name)
 
 
+@mock_aws
+def test_drop_namespace_that_contains_non_iceberg_tables(
+    _bucket_initialize: None, moto_endpoint_url: str, table_schema_nested: Schema, database_name: str, table_name: str
+) -> None:
+    test_catalog = GlueCatalog("glue", **{"s3.endpoint": moto_endpoint_url, "warehouse": f"s3://{BUCKET_NAME}/"})
+    test_catalog.create_namespace(namespace=database_name)
+    test_catalog.glue.create_table(DatabaseName=database_name, TableInput={"Name": "hive_table"})
+
+    with pytest.raises(NamespaceNotEmptyError):
+        test_catalog.drop_namespace(database_name)
+
+
 @mock_aws
 def test_drop_non_exist_namespace(_bucket_initialize: None, moto_endpoint_url: str, database_name: str) -> None:
     test_catalog = GlueCatalog("glue", **{"s3.endpoint": moto_endpoint_url})