From 0cdd9adc15464bd1de1e644bf1588a10e28459ce Mon Sep 17 00:00:00 2001 From: ryan Date: Sat, 29 Jun 2024 19:52:34 -0700 Subject: [PATCH 1/3] fix update namespace properties bug --- pyiceberg/catalog/sql.py | 15 ++++++++++----- tests/catalog/test_sql.py | 2 ++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pyiceberg/catalog/sql.py b/pyiceberg/catalog/sql.py index ff7831d77f..fafb5ef367 100644 --- a/pyiceberg/catalog/sql.py +++ b/pyiceberg/catalog/sql.py @@ -673,11 +673,16 @@ def update_namespace_properties( IcebergNamespaceProperties.property_key.in_(set(updates.keys())), ) session.execute(delete_stmt) - insert_stmt = insert(IcebergNamespaceProperties) - for property_key, property_value in updates.items(): - insert_stmt = insert_stmt.values( - catalog_name=self.name, namespace=namespace_str, property_key=property_key, property_value=property_value - ) + insert_stmt_values = [ + { + "catalog_name": self.name, + "namespace": namespace_str, + "property_key": property_key, + "property_value": property_value, + } + for property_key, property_value in updates.items() + ] + insert_stmt = insert(IcebergNamespaceProperties).values(insert_stmt_values) session.execute(insert_stmt) session.commit() return properties_update_summary diff --git a/tests/catalog/test_sql.py b/tests/catalog/test_sql.py index 24adfb88ab..ec7b9ace12 100644 --- a/tests/catalog/test_sql.py +++ b/tests/catalog/test_sql.py @@ -1173,6 +1173,8 @@ def test_update_namespace_properties(catalog: SqlCatalog, namespace: str) -> Non else: assert k in update_report.removed assert "updated test description" == catalog.load_namespace_properties(namespace)["comment"] + assert "5" == catalog.load_namespace_properties(namespace)["test_property5"] + assert "4" == catalog.load_namespace_properties(namespace)["test_property4"] @pytest.mark.parametrize( From 411d19860aad0089005179cfc21036871568acda Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 30 Jun 2024 09:06:15 -0700 Subject: [PATCH 2/3] resolve comments --- pyiceberg/catalog/sql.py | 8 ++++---- tests/catalog/test_sql.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pyiceberg/catalog/sql.py b/pyiceberg/catalog/sql.py index fafb5ef367..d3cd8a9464 100644 --- a/pyiceberg/catalog/sql.py +++ b/pyiceberg/catalog/sql.py @@ -675,10 +675,10 @@ def update_namespace_properties( session.execute(delete_stmt) insert_stmt_values = [ { - "catalog_name": self.name, - "namespace": namespace_str, - "property_key": property_key, - "property_value": property_value, + IcebergNamespaceProperties.catalog_name: self.name, + IcebergNamespaceProperties.namespace: namespace_str, + IcebergNamespaceProperties.property_key: property_key, + IcebergNamespaceProperties.property_value: property_value, } for property_key, property_value in updates.items() ] diff --git a/tests/catalog/test_sql.py b/tests/catalog/test_sql.py index ec7b9ace12..6e99d564b8 100644 --- a/tests/catalog/test_sql.py +++ b/tests/catalog/test_sql.py @@ -1165,16 +1165,16 @@ def test_update_namespace_properties(catalog: SqlCatalog, namespace: str) -> Non updates = {"test_property4": "4", "test_property5": "5", "comment": "updated test description"} catalog.create_namespace(namespace, test_properties) update_report = catalog.update_namespace_properties(namespace, removals, updates) - for k in updates.keys(): + updated_properties = catalog.load_namespace_properties(namespace) + for k, v in updates.items(): assert k in update_report.updated + assert k in updated_properties + assert v == updated_properties[k] for k in removals: if k == "should_not_removed": assert k in update_report.missing else: assert k in update_report.removed - assert "updated test description" == catalog.load_namespace_properties(namespace)["comment"] - assert "5" == catalog.load_namespace_properties(namespace)["test_property5"] - assert "4" == catalog.load_namespace_properties(namespace)["test_property4"] @pytest.mark.parametrize( From db858566aab9394b509d72133443ea07efc3ebeb Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 30 Jun 2024 16:55:41 -0700 Subject: [PATCH 3/3] resolve comment --- tests/catalog/test_sql.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/catalog/test_sql.py b/tests/catalog/test_sql.py index 6e99d564b8..eae0c5ea4b 100644 --- a/tests/catalog/test_sql.py +++ b/tests/catalog/test_sql.py @@ -1165,16 +1165,19 @@ def test_update_namespace_properties(catalog: SqlCatalog, namespace: str) -> Non updates = {"test_property4": "4", "test_property5": "5", "comment": "updated test description"} catalog.create_namespace(namespace, test_properties) update_report = catalog.update_namespace_properties(namespace, removals, updates) - updated_properties = catalog.load_namespace_properties(namespace) - for k, v in updates.items(): + for k in updates.keys(): assert k in update_report.updated - assert k in updated_properties - assert v == updated_properties[k] for k in removals: if k == "should_not_removed": assert k in update_report.missing else: assert k in update_report.removed + assert catalog.load_namespace_properties(namespace) == { + "comment": "updated test description", + "test_property4": "4", + "test_property5": "5", + "location": f"{warehouse_location}/{namespace}.db", + } @pytest.mark.parametrize(