Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pyiceberg/catalog/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,18 @@ def create_view(
ViewAlreadyExistsError: If a view with the name already exists.
"""

@abstractmethod
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
"""Rename a fully classified view name.

Args:
from_identifier (str | Identifier): Existing view identifier.
to_identifier (str | Identifier): New view identifier.

Raises:
NoSuchViewError: If a view with the name does not exist.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add ViewAlreadyExistsError?

"""

@staticmethod
def identifier_to_tuple(identifier: str | Identifier) -> Identifier:
"""Parse an identifier to a tuple.
Expand Down
3 changes: 3 additions & 0 deletions pyiceberg/catalog/bigquery_metastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError

@override
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
raise NotImplementedError

def load_namespace_properties(self, namespace: str | Identifier) -> Properties:
dataset_name = self.identifier_to_database(namespace)

Expand Down
4 changes: 4 additions & 0 deletions pyiceberg/catalog/dynamodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,10 @@ def view_exists(self, identifier: str | Identifier) -> bool:
def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError

@override
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
raise NotImplementedError

def _get_iceberg_table_item(self, database_name: str, table_name: str) -> dict[str, Any]:
try:
return self._get_dynamo_item(identifier=f"{database_name}.{table_name}", namespace=database_name)
Expand Down
4 changes: 4 additions & 0 deletions pyiceberg/catalog/glue.py
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,10 @@ def view_exists(self, identifier: str | Identifier) -> bool:
def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError

@override
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
raise NotImplementedError

@staticmethod
def __is_iceberg_table(table: "TableTypeDef") -> bool:
return table.get("Parameters", {}).get(TABLE_TYPE, "").lower() == ICEBERG
Expand Down
4 changes: 4 additions & 0 deletions pyiceberg/catalog/hive.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,10 @@ def view_exists(self, identifier: str | Identifier) -> bool:
def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError

@override
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
raise NotImplementedError

def _create_lock_request(self, database_name: str, table_name: str) -> LockRequest:
lock_component: LockComponent = LockComponent(
level=LockLevel.TABLE, type=LockType.EXCLUSIVE, dbname=database_name, tablename=table_name, isTransactional=True
Expand Down
4 changes: 4 additions & 0 deletions pyiceberg/catalog/noop.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,7 @@ def create_view(
@override
def load_view(self, identifier: str | Identifier) -> View:
raise NotImplementedError

@override
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
raise NotImplementedError
25 changes: 25 additions & 0 deletions pyiceberg/catalog/rest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ class Endpoints:
register_view: str = "namespaces/{namespace}/register-view"
drop_view: str = "namespaces/{namespace}/views/{view}"
view_exists: str = "namespaces/{namespace}/views/{view}"
rename_view: str = "views/rename"
plan_table_scan: str = "namespaces/{namespace}/tables/{table}/plan"
fetch_scan_tasks: str = "namespaces/{namespace}/tables/{table}/tasks"

Expand Down Expand Up @@ -187,6 +188,7 @@ class Capability:
V1_VIEW_EXISTS = Endpoint(http_method=HttpMethod.HEAD, path=f"{API_PREFIX}/{Endpoints.view_exists}")
V1_REGISTER_VIEW = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.register_view}")
V1_DELETE_VIEW = Endpoint(http_method=HttpMethod.DELETE, path=f"{API_PREFIX}/{Endpoints.drop_view}")
V1_RENAME_VIEW = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.rename_view}")
V1_SUBMIT_TABLE_SCAN_PLAN = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.plan_table_scan}")
V1_TABLE_SCAN_PLAN_TASKS = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.fetch_scan_tasks}")

Expand Down Expand Up @@ -216,6 +218,7 @@ class Capability:
Capability.V1_LIST_VIEWS,
Capability.V1_LOAD_VIEW,
Capability.V1_DELETE_VIEW,
Capability.V1_RENAME_VIEW,
)
)

Expand Down Expand Up @@ -1471,6 +1474,28 @@ def drop_view(self, identifier: str | Identifier) -> None:
except HTTPError as exc:
_handle_non_200_response(exc, {404: NoSuchViewError})

@retry(**_RETRY_ARGS)
def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is self._check_endpoint(Capability.V1_RENAME_VIEW) missing intentionally?

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"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use _split_identifier_for_path (not _split_identifier_for_json)?


if not self.namespace_exists(source_namespace):
raise NoSuchNamespaceError(f"Source namespace does not exist: {source_namespace}")
if not self.namespace_exists(dest_namespace):
raise NoSuchNamespaceError(f"Destination namespace does not exist: {dest_namespace}")

response = self._session.post(self.url(Endpoints.rename_view), json=payload)
try:
response.raise_for_status()
except HTTPError as exc:
_handle_non_200_response(exc, {404: NoSuchViewError, 409: ViewAlreadyExistsError})

def close(self) -> None:
"""Close the catalog and release Session connection adapters.

Expand Down
3 changes: 3 additions & 0 deletions pyiceberg/catalog/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -784,3 +784,6 @@ def close(self) -> None:
"""
if hasattr(self, "engine"):
self.engine.dispose()

def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None:
raise NotImplementedError
119 changes: 119 additions & 0 deletions tests/catalog/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3167,3 +3167,122 @@ def test_load_table_without_storage_credentials(
)
assert actual.metadata.model_dump() == expected.metadata.model_dump()
assert actual == expected

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an integration test? I've added initial tests in #3406.


def test_rename_view_204(rest_mock: Mocker) -> None:
from_identifier = ("some_namespace", "old_view")
to_identifier = ("some_namespace", "new_view")
rest_mock.head(
f"{TEST_URI}v1/namespaces/some_namespace",
status_code=200,
request_headers=TEST_HEADERS,
)
rest_mock.post(
f"{TEST_URI}v1/views/rename",
json={
"source": {"namespace": ["some_namespace"], "name": "old_view"},
"destination": {"namespace": ["some_namespace"], "name": "new_view"},
},
status_code=204,
request_headers=TEST_HEADERS,
)
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
catalog.rename_view(from_identifier, to_identifier)
assert (
rest_mock.last_request.text == """{"source": {"namespace": ["some_namespace"], "name": "old_view"}, """
""""destination": {"namespace": ["some_namespace"], "name": "new_view"}}"""
)


def test_rename_view_404(rest_mock: Mocker) -> None:
from_identifier = ("some_namespace", "non_existent_view")
to_identifier = ("some_namespace", "new_view")
rest_mock.head(
f"{TEST_URI}v1/namespaces/some_namespace",
status_code=200,
request_headers=TEST_HEADERS,
)
rest_mock.post(
f"{TEST_URI}v1/views/rename",
json={
"error": {
"message": "View does not exist: some_namespace.non_existent_view",
"type": "NoSuchViewException",
"code": 404,
}
},
status_code=404,
request_headers=TEST_HEADERS,
)
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
with pytest.raises(NoSuchViewError) as exc_info:
catalog.rename_view(from_identifier, to_identifier)
assert "View does not exist: some_namespace.non_existent_view" in str(exc_info.value)


def test_rename_view_409(rest_mock: Mocker) -> None:
from_identifier = ("some_namespace", "old_view")
to_identifier = ("some_namespace", "existing_view")
rest_mock.head(
f"{TEST_URI}v1/namespaces/some_namespace",
status_code=200,
request_headers=TEST_HEADERS,
)
rest_mock.post(
f"{TEST_URI}v1/views/rename",
json={
"error": {
"message": "View already exists: some_namespace.existing_view",
"type": "ViewAlreadyExistsException",
"code": 409,
}
},
status_code=409,
request_headers=TEST_HEADERS,
)
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
with pytest.raises(ViewAlreadyExistsError) as exc_info:
catalog.rename_view(from_identifier, to_identifier)
assert "View already exists: some_namespace.existing_view" in str(exc_info.value)


def test_rename_view_source_namespace_does_not_exist(rest_mock: Mocker) -> None:
from_identifier = ("non_existent_namespace", "old_view")
to_identifier = ("some_namespace", "new_view")

rest_mock.head(
f"{TEST_URI}v1/namespaces/non_existent_namespace",
status_code=404,
request_headers=TEST_HEADERS,
)
rest_mock.head(
f"{TEST_URI}v1/namespaces/some_namespace",
status_code=200,
request_headers=TEST_HEADERS,
)
Comment on lines +3258 to +3262
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This mock seems redundant because the catalog raises an exception before verifying the target namespace. No requested change, I suppose you put it intentionally.


catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
with pytest.raises(NoSuchNamespaceError) as exc_info:
catalog.rename_view(from_identifier, to_identifier)
assert "Source namespace does not exist: ('non_existent_namespace',)" in str(exc_info.value)


def test_rename_view_destination_namespace_does_not_exist(rest_mock: Mocker) -> None:
from_identifier = ("some_namespace", "old_view")
to_identifier = ("non_existent_namespace", "new_view")

rest_mock.head(
f"{TEST_URI}v1/namespaces/some_namespace",
status_code=200,
request_headers=TEST_HEADERS,
)
rest_mock.head(
f"{TEST_URI}v1/namespaces/non_existent_namespace",
status_code=404,
request_headers=TEST_HEADERS,
)

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)