Skip to content

[Iceberg]Support rename view on Rest catalog and Nessie catalog#25202

Merged
hantangwangd merged 1 commit intoprestodb:masterfrom
hantangwangd:support_rename_view_on_rest_and_nessie
May 31, 2025
Merged

[Iceberg]Support rename view on Rest catalog and Nessie catalog#25202
hantangwangd merged 1 commit intoprestodb:masterfrom
hantangwangd:support_rename_view_on_rest_and_nessie

Conversation

@hantangwangd
Copy link
Member

@hantangwangd hantangwangd commented May 26, 2025

Description

Through PR #23749 we support ALTER VIEW RENAME TO statement, and support rename view for Iceberg connector configured with Hive catalog. This PR enables catalogs like REST and NESSIE that implements interface ViewCatalog to support rename view as well.

Motivation and Context

Support rename view for Iceberg connector on as many catalog types as possible

Impact

Iceberg connector configured with REST and NESSIE catalogs can now support rename view

Test Plan

  • Make the existing test cases for rename views to cover REST and NESSIE as well

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== RELEASE NOTES ==

Iceberg Connector Changes
* Add support of ``rename view`` for Iceberg connector when configured with ``REST`` and ``NESSIE``

@hantangwangd hantangwangd marked this pull request as ready for review May 26, 2025 23:17
@hantangwangd hantangwangd requested review from a team and ZacBlanco as code owners May 26, 2025 23:17
@hantangwangd hantangwangd requested a review from jaystarshot May 26, 2025 23:17
((ViewCatalog) catalog).renameView(
toIcebergTableIdentifier(source, catalogFactory.isNestedNamespaceEnabled()),
toIcebergTableIdentifier(target, catalogFactory.isNestedNamespaceEnabled()));
}
Copy link
Member

@agrawalreetika agrawalreetika May 27, 2025

Choose a reason for hiding this comment

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

One question here, do we need to catch NoSuchNamespaceException here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think there is no need to check and catch NoSuchNamespaceException here again because the check is already done in RenameViewTask. See the comment in IcebergHiveMetadata here. Do you think this makes sense?

Copy link
Member

Choose a reason for hiding this comment

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

So as per Iceberg API it could throw these 3 exception here. And as I checked earlier, RenameViewTask checks for first two?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your detailed message. After re-checking the code, I think we needn't consider the situation that would throw a NoSuchNamespaceException, since we do not allow view rename cross schemas or catalogs, see here.

Copy link
Member

@agrawalreetika agrawalreetika left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Just one question.

Copy link
Member

@agrawalreetika agrawalreetika left a comment

Choose a reason for hiding this comment

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

Thanks for the change. LGTM

@hantangwangd hantangwangd merged commit 783c58e into prestodb:master May 31, 2025
105 checks passed
@hantangwangd hantangwangd deleted the support_rename_view_on_rest_and_nessie branch May 31, 2025 00:11
@prestodb-ci prestodb-ci mentioned this pull request Jul 28, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants