From 9fbb1e8ea34d21ec61e17a1d2336113d0b90b248 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 16:10:48 -0700 Subject: [PATCH 1/6] REST: Send raw view name in create_view and register_view bodies create_view and register_view took the view name from _split_identifier_for_path, which URL-quotes it for use as a path segment, and put it in the JSON request body. A view named "my view" was created as "my%20view". Use the raw name, as create_table and register_table already do. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pyiceberg/catalog/rest/__init__.py | 12 ++++----- tests/catalog/test_rest.py | 41 +++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/pyiceberg/catalog/rest/__init__.py b/pyiceberg/catalog/rest/__init__.py index 97b2e23505..58452b1e91 100644 --- a/pyiceberg/catalog/rest/__init__.py +++ b/pyiceberg/catalog/rest/__init__.py @@ -1312,7 +1312,7 @@ def create_view( location = location.rstrip("/") request = CreateViewRequest( - name=namespace_and_view["view"], + name=self._identifier_to_validated_tuple(identifier)[-1], location=location, view_schema=fresh_schema, view_version=view_version, @@ -1825,16 +1825,14 @@ def view_exists(self, identifier: str | Identifier) -> bool: @override def register_view(self, identifier: str | Identifier, metadata_location: str) -> View: self._check_endpoint(Capability.V1_REGISTER_VIEW) - namespace_and_view = self._split_identifier_for_path(identifier, IdentifierKind.VIEW) - namespace = namespace_and_view["namespace"] - view = namespace_and_view["view"] + identifier_tuple = self._identifier_to_validated_tuple(identifier) if self.table_exists(identifier): - raise TableAlreadyExistsError(f"Table {namespace}.{view} already exists") + raise TableAlreadyExistsError(f"Table {'.'.join(identifier_tuple)} already exists") - request = RegisterViewRequest(name=view, metadata_location=metadata_location) + request = RegisterViewRequest(name=identifier_tuple[-1], metadata_location=metadata_location) serialized_json = request.model_dump_json().encode(UTF8) response = self._session.post( - self.url(Endpoints.register_view, namespace=namespace), + self.url(Endpoints.register_view, namespace=self._encode_namespace_path(identifier_tuple[:-1])), data=serialized_json, ) try: diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py index b1b46e4673..7ec18bfe5e 100644 --- a/tests/catalog/test_rest.py +++ b/tests/catalog/test_rest.py @@ -23,7 +23,7 @@ from contextlib import contextmanager from typing import Any, cast from unittest import mock -from urllib.parse import parse_qs, urlsplit +from urllib.parse import parse_qs, quote, urlsplit import pytest from requests import Request @@ -1805,6 +1805,27 @@ def test_create_view_200(rest_mock: Mocker, table_schema_simple: Schema, example assert actual.config == example_view_metadata_rest_json["config"] +def test_create_view_name_is_not_encoded_in_body( + rest_mock: Mocker, table_schema_simple: Schema, example_view_metadata_rest_json: dict[str, Any] +) -> None: + rest_mock.post( + f"{TEST_URI}v1/namespaces/fokko/views", + json=example_view_metadata_rest_json, + status_code=200, + request_headers=TEST_HEADERS, + ) + catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) + view_version = ViewVersion( + schema_id=1, + summary={"engine-name": "spark", "engineVersion": "3.3"}, + representations=[], + default_namespace=["default"], + ) + for view_name in ["my view", "dept/view", "100%", "\u65e5\u672c"]: + catalog.create_view(identifier=("fokko", view_name), schema=table_schema_simple, view_version=view_version) + assert rest_mock.last_request.json()["name"] == view_name + + def test_create_view_409( rest_mock: Mocker, table_schema_simple: Schema, @@ -2817,6 +2838,24 @@ def test_register_view_200(rest_mock: Mocker, example_view_metadata_rest_json: d assert actual.config == example_view_metadata_rest_json["config"] +def test_register_view_name_is_not_encoded_in_body(rest_mock: Mocker, example_view_metadata_rest_json: dict[str, Any]) -> None: + rest_mock.post( + f"{TEST_URI}v1/namespaces/default/register-view", + json=example_view_metadata_rest_json, + status_code=200, + request_headers=TEST_HEADERS, + ) + catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) + for view_name in ["my view", "dept/view", "100%", "\u65e5\u672c"]: + rest_mock.head( + f"{TEST_URI}v1/namespaces/default/tables/{quote(view_name, safe='')}", + status_code=404, + request_headers=TEST_HEADERS, + ) + catalog.register_view(identifier=("default", view_name), metadata_location="s3://warehouse/database/view/metadata.json") + assert rest_mock.last_request.json()["name"] == view_name + + def test_register_view_409_view(rest_mock: Mocker) -> None: rest_mock.head( f"{TEST_URI}v1/namespaces/default/tables/registered_view", From abb82bfbc70a82f26421eed2d8ee76a4612a4ff3 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 16:16:27 -0700 Subject: [PATCH 2/6] Test REST identifiers that need URL encoding against the REST fixture Round-trip namespace, table, and view names containing spaces, reserved URL characters, and non-ASCII text through path, query, and body parameters. This catches encoding mistakes like the double-encoded list_namespaces parent and the encoded view name in create_view. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/integration/test_catalog.py | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/integration/test_catalog.py b/tests/integration/test_catalog.py index 3765b296e0..f304650e4a 100644 --- a/tests/integration/test_catalog.py +++ b/tests/integration/test_catalog.py @@ -719,6 +719,42 @@ def test_rest_drop_view( assert not rest_catalog.view_exists(identifier) +@pytest.mark.integration +def test_rest_identifiers_with_special_characters( + rest_catalog: RestCatalog, table_schema_simple: Schema, example_view_metadata_v1: dict[str, Any] +) -> None: + """Names that need URL encoding must round-trip unchanged through path, query, and body parameters.""" + # "%" is omitted because the REST fixture rejects "%25" in URL paths with a 400 + for name in ["with space", "a+b&c=d", "\u65e5\u672c"]: + parent = (f"{name}_{uuid.uuid4().hex}",) + namespace = (*parent, name) + table_identifier = (*namespace, f"table {name}") + view_identifier = (*namespace, f"view {name}") + + rest_catalog.create_namespace(parent) + rest_catalog.create_namespace(namespace, properties={"key": name}) + assert parent in rest_catalog.list_namespaces() + assert rest_catalog.list_namespaces(parent) == [namespace] + assert rest_catalog.namespace_exists(namespace) + assert rest_catalog.load_namespace_properties(namespace)["key"] == name + + rest_catalog.create_table(table_identifier, table_schema_simple) + assert rest_catalog.table_exists(table_identifier) + assert rest_catalog.list_tables(namespace) == [table_identifier] + assert rest_catalog.load_table(table_identifier).name() == table_identifier + + view = View(view_identifier, ViewMetadata.model_validate(example_view_metadata_v1)) + rest_catalog.create_view(view_identifier, view.schema(), view.current_version()) + assert rest_catalog.view_exists(view_identifier) + assert rest_catalog.list_views(namespace) == [view_identifier] + + rest_catalog.drop_view(view_identifier) + rest_catalog.drop_table(table_identifier) + rest_catalog.drop_namespace(namespace) + rest_catalog.drop_namespace(parent) + assert not rest_catalog.namespace_exists(parent) + + @pytest.mark.integration def test_rest_custom_namespace_separator(rest_catalog: RestCatalog, table_schema_simple: Schema) -> None: """ From 3309a1878f80840c7edd1781d25ead6206c5c4f2 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 17:20:49 -0700 Subject: [PATCH 3/6] Cover commit_table in REST identifier integration test commit_table is the only caller of the TableIdentifier branch of _split_identifier_for_path, so table updates had no coverage for names that need URL encoding. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/integration/test_catalog.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_catalog.py b/tests/integration/test_catalog.py index f304650e4a..2884d6fa82 100644 --- a/tests/integration/test_catalog.py +++ b/tests/integration/test_catalog.py @@ -741,7 +741,11 @@ def test_rest_identifiers_with_special_characters( rest_catalog.create_table(table_identifier, table_schema_simple) assert rest_catalog.table_exists(table_identifier) assert rest_catalog.list_tables(namespace) == [table_identifier] - assert rest_catalog.load_table(table_identifier).name() == table_identifier + table = rest_catalog.load_table(table_identifier) + assert table.name() == table_identifier + with table.transaction() as tx: + tx.set_properties(key=name) + assert rest_catalog.load_table(table_identifier).properties["key"] == name view = View(view_identifier, ViewMetadata.model_validate(example_view_metadata_v1)) rest_catalog.create_view(view_identifier, view.schema(), view.current_version()) From 83e619668d0a4b853c81496ee335fb6b37710851 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 17:22:21 -0700 Subject: [PATCH 4/6] Clarify why % is omitted from REST identifier integration test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/integration/test_catalog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_catalog.py b/tests/integration/test_catalog.py index 2884d6fa82..f25da53f8d 100644 --- a/tests/integration/test_catalog.py +++ b/tests/integration/test_catalog.py @@ -724,7 +724,7 @@ def test_rest_identifiers_with_special_characters( rest_catalog: RestCatalog, table_schema_simple: Schema, example_view_metadata_v1: dict[str, Any] ) -> None: """Names that need URL encoding must round-trip unchanged through path, query, and body parameters.""" - # "%" is omitted because the REST fixture rejects "%25" in URL paths with a 400 + # "%" is omitted because the REST fixture's Jetty rejects "%25" in URL paths ("400 Ambiguous URI path encoding") for name in ["with space", "a+b&c=d", "\u65e5\u672c"]: parent = (f"{name}_{uuid.uuid4().hex}",) namespace = (*parent, name) From 46773215b103e872e9145c4637e78a72e7ec88f1 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 17:31:15 -0700 Subject: [PATCH 5/6] Keep _split_identifier_for_path for the register_view path Matches create_view and register_table: path from _split_identifier_for_path, raw name for the body. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pyiceberg/catalog/rest/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyiceberg/catalog/rest/__init__.py b/pyiceberg/catalog/rest/__init__.py index 58452b1e91..e115289bde 100644 --- a/pyiceberg/catalog/rest/__init__.py +++ b/pyiceberg/catalog/rest/__init__.py @@ -1825,6 +1825,7 @@ def view_exists(self, identifier: str | Identifier) -> bool: @override def register_view(self, identifier: str | Identifier, metadata_location: str) -> View: self._check_endpoint(Capability.V1_REGISTER_VIEW) + namespace_and_view = self._split_identifier_for_path(identifier, IdentifierKind.VIEW) identifier_tuple = self._identifier_to_validated_tuple(identifier) if self.table_exists(identifier): raise TableAlreadyExistsError(f"Table {'.'.join(identifier_tuple)} already exists") @@ -1832,7 +1833,7 @@ def register_view(self, identifier: str | Identifier, metadata_location: str) -> request = RegisterViewRequest(name=identifier_tuple[-1], metadata_location=metadata_location) serialized_json = request.model_dump_json().encode(UTF8) response = self._session.post( - self.url(Endpoints.register_view, namespace=self._encode_namespace_path(identifier_tuple[:-1])), + self.url(Endpoints.register_view, namespace=namespace_and_view["namespace"]), data=serialized_json, ) try: From 6af506307a7aef5cdf96d4bbdbd37808bb42fa2a Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 17:47:54 -0700 Subject: [PATCH 6/6] Cover register_view in REST identifier integration test Register the created view under a second name that needs URL encoding, now that View exposes its metadata location (#4088). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/integration/test_catalog.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_catalog.py b/tests/integration/test_catalog.py index f25da53f8d..decd0e11c5 100644 --- a/tests/integration/test_catalog.py +++ b/tests/integration/test_catalog.py @@ -730,6 +730,7 @@ def test_rest_identifiers_with_special_characters( namespace = (*parent, name) table_identifier = (*namespace, f"table {name}") view_identifier = (*namespace, f"view {name}") + registered_view_identifier = (*namespace, f"registered view {name}") rest_catalog.create_namespace(parent) rest_catalog.create_namespace(namespace, properties={"key": name}) @@ -748,10 +749,16 @@ def test_rest_identifiers_with_special_characters( assert rest_catalog.load_table(table_identifier).properties["key"] == name view = View(view_identifier, ViewMetadata.model_validate(example_view_metadata_v1)) - rest_catalog.create_view(view_identifier, view.schema(), view.current_version()) + created_view = rest_catalog.create_view(view_identifier, view.schema(), view.current_version()) assert rest_catalog.view_exists(view_identifier) assert rest_catalog.list_views(namespace) == [view_identifier] + assert created_view.metadata_location is not None + rest_catalog.register_view(registered_view_identifier, created_view.metadata_location) + assert rest_catalog.view_exists(registered_view_identifier) + assert sorted(rest_catalog.list_views(namespace)) == sorted([view_identifier, registered_view_identifier]) + + rest_catalog.drop_view(registered_view_identifier) rest_catalog.drop_view(view_identifier) rest_catalog.drop_table(table_identifier) rest_catalog.drop_namespace(namespace)