PR comments

Author: rambleraptor

pyiceberg/catalog/__init__.py

@@ -754,6 +754,7 @@ def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Id
 
         Raises:
             NoSuchViewError: If a view with the name does not exist.
+            ViewAlreadyExistsError: If the target view already exists.
         """
 
     @staticmethod

pyiceberg/catalog/rest/__init__.py

@@ -1508,14 +1508,15 @@ def drop_view(self, identifier: str | Identifier) -> None:
 
     @retry(**_RETRY_ARGS)
     def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
+        self._check_endpoint(Capability.V1_RENAME_VIEW)
         payload = {
             "source": self._split_identifier_for_json(from_identifier),
             "destination": self._split_identifier_for_json(to_identifier),
         }
 
         # Ensure source and destination namespaces exist before rename.
         source_namespace = self._split_identifier_for_json(from_identifier)["namespace"]
-        dest_namespace = self._split_identifier_for_path(to_identifier)["namespace"]
+        dest_namespace = self._split_identifier_for_json(to_identifier)["namespace"]
 
         if not self.namespace_exists(source_namespace):
             raise NoSuchNamespaceError(f"Source namespace does not exist: {source_namespace}")

tests/catalog/test_rest.py

@@ -110,6 +110,7 @@
     Capability.V1_VIEW_EXISTS,
     Capability.V1_REGISTER_VIEW,
     Capability.V1_DELETE_VIEW,
+    Capability.V1_RENAME_VIEW,
     Capability.V1_SUBMIT_TABLE_SCAN_PLAN,
     Capability.V1_TABLE_SCAN_PLAN_TASKS,
 ]
@@ -3314,4 +3315,4 @@ def test_rename_view_destination_namespace_does_not_exist(rest_mock: Mocker) ->
     catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
     with pytest.raises(NoSuchNamespaceError) as exc_info:
         catalog.rename_view(from_identifier, to_identifier)
-    assert "Destination namespace does not exist: non_existent_namespace" in str(exc_info.value)
+    assert "Destination namespace does not exist: ('non_existent_namespace',)" in str(exc_info.value)

tests/integration/test_catalog.py

@@ -670,6 +670,26 @@ def test_rest_drop_view(
 
 
 @pytest.mark.integration
+def test_rest_rename_view(
+    rest_catalog: RestCatalog, example_view_metadata_v1: dict[str, Any], database_name: str, view_name: str
+) -> None:
+    from_identifier = (database_name, view_name)
+    to_identifier = (database_name, f"{view_name}_renamed")
+
+    rest_catalog.create_namespace_if_not_exists(database_name)
+    view = View(from_identifier, ViewMetadata.model_validate(example_view_metadata_v1))
+
+    rest_catalog.create_view(from_identifier, view.schema(), view.current_version())
+    assert rest_catalog.view_exists(from_identifier)
+
+    rest_catalog.rename_view(from_identifier, to_identifier)
+
+    assert not rest_catalog.view_exists(from_identifier)
+    assert rest_catalog.view_exists(to_identifier)
+
+
+@pytest.mark.integration
+@pytest.mark.skip(reason="Requires Iceberg REST Fixtures 1.11.x")
 def test_rest_custom_namespace_separator(rest_catalog: RestCatalog, table_schema_simple: Schema) -> None:
     """
     Tests that the REST catalog correctly picks up the namespace-separator from the config endpoint.