From 9d358839e376daf69c418e357bb04cbb0febe652 Mon Sep 17 00:00:00 2001 From: edson duarte Date: Thu, 27 Jun 2024 22:58:24 -0300 Subject: [PATCH 1/3] Raise error on nonexistent namespace on sql.py --- pyiceberg/catalog/sql.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyiceberg/catalog/sql.py b/pyiceberg/catalog/sql.py index ff7831d77f..aa7a44bbc5 100644 --- a/pyiceberg/catalog/sql.py +++ b/pyiceberg/catalog/sql.py @@ -556,6 +556,8 @@ def drop_namespace(self, namespace: Union[str, Identifier]) -> None: ) ) session.commit() + else: + raise NoSuchNamespaceError(f"Namespace does not exist: {namespace}") def list_tables(self, namespace: Union[str, Identifier]) -> List[Identifier]: """List tables under the given namespace in the catalog. From 5bcf85ebbcd170c52d23f26032325f50382ca740 Mon Sep 17 00:00:00 2001 From: edson duarte Date: Thu, 27 Jun 2024 23:11:11 -0300 Subject: [PATCH 2/3] Simplify namespace check --- pyiceberg/catalog/sql.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pyiceberg/catalog/sql.py b/pyiceberg/catalog/sql.py index aa7a44bbc5..b58d1c94ef 100644 --- a/pyiceberg/catalog/sql.py +++ b/pyiceberg/catalog/sql.py @@ -543,22 +543,22 @@ def drop_namespace(self, namespace: Union[str, Identifier]) -> None: NoSuchNamespaceError: If a namespace with the given name does not exist. NamespaceNotEmptyError: If the namespace is not empty. """ - if self._namespace_exists(namespace): - namespace_str = Catalog.namespace_to_string(namespace) - if tables := self.list_tables(namespace): - raise NamespaceNotEmptyError(f"Namespace {namespace_str} is not empty. {len(tables)} tables exist.") - - with Session(self.engine) as session: - session.execute( - delete(IcebergNamespaceProperties).where( - IcebergNamespaceProperties.catalog_name == self.name, - IcebergNamespaceProperties.namespace == namespace_str, - ) - ) - session.commit() - else: + if not self._namespace_exists(namespace): raise NoSuchNamespaceError(f"Namespace does not exist: {namespace}") + namespace_str = Catalog.namespace_to_string(namespace) + if tables := self.list_tables(namespace): + raise NamespaceNotEmptyError(f"Namespace {namespace_str} is not empty. {len(tables)} tables exist.") + + with Session(self.engine) as session: + session.execute( + delete(IcebergNamespaceProperties).where( + IcebergNamespaceProperties.catalog_name == self.name, + IcebergNamespaceProperties.namespace == namespace_str, + ) + ) + session.commit() + def list_tables(self, namespace: Union[str, Identifier]) -> List[Identifier]: """List tables under the given namespace in the catalog. From 5d2c48da31b1af52260dfffe8265bf1d516a7433 Mon Sep 17 00:00:00 2001 From: edson duarte Date: Fri, 28 Jun 2024 08:56:18 -0300 Subject: [PATCH 3/3] Add test case --- tests/catalog/test_sql.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/catalog/test_sql.py b/tests/catalog/test_sql.py index 24adfb88ab..7534a115bf 100644 --- a/tests/catalog/test_sql.py +++ b/tests/catalog/test_sql.py @@ -1093,6 +1093,18 @@ def test_drop_namespace(catalog: SqlCatalog, table_schema_nested: Schema, table_ assert namespace not in catalog.list_namespaces() +@pytest.mark.parametrize( + "catalog", + [ + lazy_fixture("catalog_memory"), + lazy_fixture("catalog_sqlite"), + ], +) +def test_drop_non_existing_namespaces(catalog: SqlCatalog) -> None: + with pytest.raises(NoSuchNamespaceError): + catalog.drop_namespace("does_not_exist") + + @pytest.mark.parametrize( "catalog", [