Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hantangwangd merged 1 commit into from
May 31, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,18 @@ public void dropView(ConnectorSession session, SchemaTableName viewName)
((ViewCatalog) catalog).dropView(toIcebergTableIdentifier(viewName, catalogFactory.isNestedNamespaceEnabled()));
}

@Override
public void renameView(ConnectorSession session, SchemaTableName source, SchemaTableName target)
{
Catalog catalog = catalogFactory.getCatalog(session);
if (!(catalog instanceof ViewCatalog)) {
throw new PrestoException(NOT_SUPPORTED, "This connector does not support renaming views");
}
((ViewCatalog) catalog).renameView(
toIcebergTableIdentifier(source, catalogFactory.isNestedNamespaceEnabled()),
toIcebergTableIdentifier(target, catalogFactory.isNestedNamespaceEnabled()));
}
Copy link
Copy Markdown
Member

@agrawalreetika agrawalreetika May 27, 2025 •
edited
Loading

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
Copy Markdown
Member Author

@hantangwangd hantangwangd May 27, 2025

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
Copy Markdown
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.

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

Copy link
Copy Markdown
Member Author

@hantangwangd hantangwangd May 27, 2025

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.


private void verifyAndPopulateViews(View view, SchemaTableName schemaTableName, String viewData, ImmutableMap.Builder<SchemaTableName, ConnectorViewDefinition> views)
{
views.put(schemaTableName, new ConnectorViewDefinition(
Expand Down
27 changes: 27 additions & 0 deletions presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergDistributedQueries.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,31 @@ public void testStringFilters()
assertQuery("SELECT count(*) FROM test_varcharn_filter WHERE shipmode = 'AIR '", "VALUES (0)");
assertQuery("SELECT count(*) FROM test_varcharn_filter WHERE shipmode = 'NONEXIST'", "VALUES (0)");
}

@Test
public void testRenameView()
{
skipTestUnless(supportsViews());
assertQuerySucceeds("CREATE TABLE iceberg_test_table (_string VARCHAR, _integer INTEGER)");
assertUpdate("CREATE VIEW test_view_to_be_renamed AS SELECT * FROM iceberg_test_table");
assertUpdate("ALTER VIEW IF EXISTS test_view_to_be_renamed RENAME TO test_view_renamed");
assertUpdate("CREATE VIEW test_view2_to_be_renamed AS SELECT * FROM iceberg_test_table");
assertUpdate("ALTER VIEW test_view2_to_be_renamed RENAME TO test_view2_renamed");
assertQuerySucceeds("SELECT * FROM test_view_renamed");
assertQuerySucceeds("SELECT * FROM test_view2_renamed");
assertUpdate("DROP VIEW test_view_renamed");
assertUpdate("DROP VIEW test_view2_renamed");
assertUpdate("DROP TABLE iceberg_test_table");
}

@Test
public void testRenameViewIfNotExists()
{
String catalog = getSession().getCatalog().get();
String schema = getSession().getSchema().get();
skipTestUnless(supportsViews());
assertQueryFails("ALTER VIEW test_rename_view_not_exist RENAME TO test_renamed_view_not_exist",
format("line 1:1: View '%s.%s.test_rename_view_not_exist' does not exist", catalog, schema));
assertQuerySucceeds("ALTER VIEW IF EXISTS test_rename_view_not_exist RENAME TO test_renamed_view_not_exist");
}
}
24 changes: 0 additions & 24 deletions presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergMetadataListing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,28 +154,4 @@ public void testTableValidation()
assertQuerySucceeds("SELECT * FROM iceberg.test_schema.iceberg_table1");
assertQueryFails("SELECT * FROM iceberg.test_schema.hive_table", "Not an Iceberg table: test_schema.hive_table");
}

@Test
public void testRenameView()
{
assertQuerySucceeds("CREATE SCHEMA iceberg.test_rename_view_schema");
assertQuerySucceeds("CREATE TABLE iceberg.test_rename_view_schema.iceberg_test_table (_string VARCHAR, _integer INTEGER)");
assertUpdate("CREATE VIEW iceberg.test_rename_view_schema.test_view_to_be_renamed AS SELECT * FROM iceberg.test_rename_view_schema.iceberg_test_table");
assertUpdate("ALTER VIEW IF EXISTS iceberg.test_rename_view_schema.test_view_to_be_renamed RENAME TO iceberg.test_rename_view_schema.test_view_renamed");
assertUpdate("CREATE VIEW iceberg.test_rename_view_schema.test_view2_to_be_renamed AS SELECT * FROM iceberg.test_rename_view_schema.iceberg_test_table");
assertUpdate("ALTER VIEW iceberg.test_rename_view_schema.test_view2_to_be_renamed RENAME TO iceberg.test_rename_view_schema.test_view2_renamed");
assertQuerySucceeds("SELECT * FROM iceberg.test_rename_view_schema.test_view_renamed");
assertQuerySucceeds("SELECT * FROM iceberg.test_rename_view_schema.test_view2_renamed");
assertUpdate("DROP VIEW iceberg.test_rename_view_schema.test_view_renamed");
assertUpdate("DROP VIEW iceberg.test_rename_view_schema.test_view2_renamed");
assertUpdate("DROP TABLE iceberg.test_rename_view_schema.iceberg_test_table");
assertQuerySucceeds("DROP SCHEMA IF EXISTS iceberg.test_rename_view_schema");
}

@Test
public void testRenameViewIfNotExists()
{
assertQueryFails("ALTER VIEW iceberg.test_schema.test_rename_view_not_exist RENAME TO iceberg.test_schema.test_renamed_view_not_exist", "line 1:1: View 'iceberg.test_schema.test_rename_view_not_exist' does not exist");
assertQuerySucceeds("ALTER VIEW IF EXISTS iceberg.test_schema.test_rename_view_not_exist RENAME TO iceberg.test_schema.test_renamed_view_not_exist");
}
}
Loading