Uh oh!
There was an error while loading. Please reload this page.
Add rename_view to REST Catalog - #2149
Conversation
rambleraptor
commented
Jul 1, 2025
@Fokko@kevinjqliu mind taking a look when you can? thanks! |
f4e2a61 to
756ee0bCompareUh oh!
There was an error while loading. Please reload this page.
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
rambleraptor
commented
Apr 28, 2026
@geruh looks like you've been going through the View PRs. Mind taking a look at this one? Follows the others near-identically. |
rambleraptor
commented
May 27, 2026
@ebyhr can you take a look? You've been doing a lot of work around views lately. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
rambleraptor
commented
May 29, 2026
@kevinjqliu@Fokko@geruh mind taking a look? This will help with all of the View work that's been done. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
abnobdoss
left a comment
There was a problem hiding this comment.
Looks good! The only concern I have is the point you highlighted in the PR description of the discrepancy between rename_table returning a Table but rename_view returning None. It sounds like the right fix would be to change rename_table to return None to be aligned to the spec. Since that is a breaking change I imagine we likely would have to wait till 1.0.0 to align those functions.
rambleraptor
commented
Jun 21, 2026
I agree that |
rambleraptor
commented
Jun 22, 2026
@kevinjqliu mind taking a look? This should be good to go! |
| @retry(**_RETRY_ARGS) | ||
| def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None: | ||
| self._check_endpoint(Capability.V1_RENAME_VIEW) | ||
| payload = { |
There was a problem hiding this comment.
self._split_identifier_for_json is still called twice per namespace.
I think we can move the payload after checking namespace_exists, for example
source = self._split_identifier_for_json(from_identifier)
destination = self._split_identifier_for_json(to_identifier)
# Ensure that namespaces exist on source and destination.
source_namespace = source["namespace"]
if not self.namespace_exists(source_namespace):
raise NoSuchNamespaceError(f"Source namespace does not exist: {source_namespace}")
destination_namespace = destination["namespace"]
if not self.namespace_exists(destination_namespace):
raise NoSuchNamespaceError(f"Destination namespace does not exist: {destination_namespace}")
payload = {
"source": source,
"destination": destination,
}
Rationale for this change
As part of #818, this adds
rename_viewsupport to the Iceberg REST Catalog.The REST Catalog spec specifies that both
rename_viewandrename_tabledo not return anything. Currently,rename_tablereturns the table. I'm happy to change that API if we want, but it would technically be a breaking change.Are these changes tested?
Added unit tests.
Are there any user-facing changes?
rename_viewsupport to Iceberg REST Catalog.