REST: Don't URL-encode view names in create_view and register_view bodies by kevinjqliu · Pull Request #4151 · apache/iceberg-python

kevinjqliu · 2026-10-25T00:14:54Z

Rationale for this change

Same kind of bug as #4112, but for views. create_view and register_view put the URL-encoded view name in the JSON body, so my view gets created as my%20view. Use the raw name instead, like create_table and register_table.

Also added an integration test that round-trips names with spaces, +&=, and non-ASCII through the REST fixture.

Are these changes tested?

Yes, both fail on main:

pytest tests/catalog/test_rest.py: passed
pytest tests/integration/test_catalog.py -m integration -k test_rest_identifiers_with_special_characters: passed

Are there any user-facing changes?

Yes, bug fix for view names that need URL encoding.

AI Disclosure: Assisted by GitHub Copilot.

kevinjqliu · 2026-10-25T00:16:09Z



+@pytest.mark.integration
+def test_rest_identifiers_with_special_characters(


integration test that checks special character handling on relevant endpoints

kevinjqliu · 2026-10-25T00:28:31Z


        request = CreateViewRequest(
-            name=namespace_and_view["view"],
+            name=self._identifier_to_validated_tuple(identifier)[-1],


use _identifier_to_validated_tuple for request bodies so names aren't escaped, and escape only in URL paths.

mirrors CreateTableRequest

iceberg-python/pyiceberg/catalog/rest/__init__.py

Lines 1230 to 1238 in 12d4a9a

request = CreateTableRequest(

name=self._identifier_to_validated_tuple(identifier)[-1],

location=location,

table_schema=fresh_schema,

partition_spec=fresh_partition_spec,

write_order=fresh_sort_order,

stage_create=stage_create,

properties=properties,

)

Idea (not necessarily for this PR): what if Identifier still acted like a tuple but had named methods on it as well? I really don't like the -1 here, since it's not always intuitive why we're doing that.

Could you add a comment explaining what you're trying to do?

kevinjqliu · 2026-10-25T00:35:31Z

+        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=namespace_and_view["namespace"]),


follows register_table

iceberg-python/pyiceberg/catalog/rest/__init__.py

Lines 1352 to 1360 in 12d4a9a

namespace_and_table = self._split_identifier_for_path(identifier)

request = RegisterTableRequest(

name=self._identifier_to_validated_tuple(identifier)[-1],

metadata_location=metadata_location,

overwrite=overwrite,

)

serialized_json = request.model_dump_json().encode(UTF8)

response = self._session.post(

self.url(Endpoints.register_table, namespace=namespace_and_table["namespace"]),

kevinjqliu · 2026-10-25T00:44:35Z

The bug was using the wrong helper in the request body. The body should not be escaped, the URL should be. We should probably do some refactoring here (esp. with the naming).

Helper Escaped? Returns for ("ns", "my view") Use for

_identifier_to_validated_tuple(identifier) No ("ns", "my view") JSON body, error messages. Raises NoSuchIdentifierError if there's no namespace

_split_identifier_for_path(identifier, kind) Yes {"namespace": "ns", "view": "my%20view"} URL path only

Copilot

🟢 Approval recommended

The focused fix preserves path encoding, covers both affected request bodies with regression tests, and has no identified blocking issues.

No open issues 🎉

What changed in this PR

Fixes REST view creation and registration so names containing special characters remain unchanged in JSON request bodies, while URL paths retain encoding.

Changes:

Uses raw view names in create_view and register_view payloads.
Adds regression tests for both payloads and integration coverage for special-character identifiers.

File	Description
`tests/integration/test_catalog.py`	Adds REST round-trip checks for namespaces, tables, and views with special characters.
`tests/catalog/test_rest.py`	Verifies create and register view requests preserve raw names in JSON bodies.
`pyiceberg/catalog/rest/__init__.py`	Separates raw view names from encoded paths and uses readable identifiers in conflict errors.

_{Review effort: Balanced}

kevinjqliu · 2026-10-25T00:48:36Z

+        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]
+        # TODO: Cover register_view once View exposes its metadata location (#4088)


we should add register_view here, but its not yet merged

added register_view here after rebasing #4088

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>

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>

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>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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>

Register the created view under a second name that needs URL encoding, now that View exposes its metadata location (apache#4088). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

kevinjqliu · 2026-10-25T03:54:25Z

+        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])


two views here because one is from create_view, another from register_review

kevinjqliu commented Oct 25, 2026

View reviewed changes

kevinjqliu changed the title ~~REST: Send raw view name in create_view and register_view~~ REST: Don't URL-encode view names in create_view and register_view bodies Oct 25, 2026

kevinjqliu marked this pull request as ready for review October 25, 2026 00:46

Copilot AI balanced review requested due to automatic review settings October 25, 2026 00:46

Copilot started reviewing on behalf of kevinjqliu October 25, 2026 00:47

View session

Copilot AI reviewed Oct 25, 2026

View reviewed changes

kevinjqliu commented Oct 25, 2026

View reviewed changes

kevinjqliu and others added 6 commits October 24, 2026 20:49

Clarify why % is omitted from REST identifier integration test

83e6196

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Keep _split_identifier_for_path for the register_view path

4677321

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>

Cover register_view in REST identifier integration test

6af5063

Register the created view under a second name that needs URL encoding, now that View exposes its metadata location (apache#4088). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

kevinjqliu force-pushed the kevinjqliu-fix-rest-view-name-encoding branch from acc4f8c to 6af5063 Compare October 25, 2026 03:52

kevinjqliu commented Oct 25, 2026

View reviewed changes

rambleraptor approved these changes Oct 25, 2026

View reviewed changes

kevinjqliu requested review from Fokko and geruh October 25, 2026 03:55

geruh approved these changes Oct 25, 2026

View reviewed changes

kevinjqliu enabled auto-merge October 25, 2026 04:05

kevinjqliu added this pull request to the merge queue Oct 25, 2026

Merged via the queue into apache:main with commit c8502b3 Oct 25, 2026
21 checks passed

kevinjqliu deleted the kevinjqliu-fix-rest-view-name-encoding branch October 25, 2026 04:29

Navigation Menu

Repository navigation

REST: Don't URL-encode view names in create_view and register_view bodies - #4151

REST: Don't URL-encode view names in create_view and register_view bodies#4151
kevinjqliu merged 6 commits into
apache:mainfrom
kevinjqliu:kevinjqliu-fix-rest-view-name-encoding

kevinjqliu commented Oct 25, 2026 •

edited

Loading

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

rambleraptor Oct 25, 2026

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

kevinjqliu Oct 25, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants



		@pytest.mark.integration
		def test_rest_identifiers_with_special_characters(

	request = CreateTableRequest(
	name=self._identifier_to_validated_tuple(identifier)[-1],
	location=location,
	table_schema=fresh_schema,
	partition_spec=fresh_partition_spec,
	write_order=fresh_sort_order,
	stage_create=stage_create,
	properties=properties,
	)

	namespace_and_table = self._split_identifier_for_path(identifier)
	request = RegisterTableRequest(
	name=self._identifier_to_validated_tuple(identifier)[-1],
	metadata_location=metadata_location,
	overwrite=overwrite,
	)
	serialized_json = request.model_dump_json().encode(UTF8)
	response = self._session.post(
	self.url(Endpoints.register_table, namespace=namespace_and_table["namespace"]),

Helper	Escaped?	Returns for `("ns", "my view")`	Use for
`_identifier_to_validated_tuple(identifier)`	No	`("ns", "my view")`	JSON body, error messages. Raises `NoSuchIdentifierError` if there's no namespace
`_split_identifier_for_path(identifier, kind)`	Yes	`{"namespace": "ns", "view": "my%20view"}`	URL path only

Conversation

kevinjqliu commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kevinjqliu commented Oct 25, 2026 •

edited

Loading