Retry on NoSuchNamespaceException not found in rename table for rest catalog #12159
Conversation
huan233usc
changed the title
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
Outdated
Show resolved
Hide resolved
| if (name().equals(to.namespace().level(0)) && to.namespace().length() > 1) { | ||
| Namespace namespace = | ||
| Namespace.of(Arrays.copyOfRange(to.namespace().levels(), 1, to.namespace().length())); | ||
| TableIdentifier toWithStrippingCatalog = TableIdentifier.of(namespace, to.name()); |
There was a problem hiding this comment.
| TableIdentifier toWithStrippingCatalog = TableIdentifier.of(namespace, to.name()); | |
| TableIdentifier toWithoutCatalog = TableIdentifier.of(namespace, to.name()); |
| @@ -428,7 +429,19 @@ public void renameTable(SessionContext context, TableIdentifier from, TableIdent | |||
| Endpoint.check(endpoints, Endpoint.V1_RENAME_TABLE); | |||
| checkIdentifierIsValid(from); | |||
| checkIdentifierIsValid(to); | |||
| try { | |||
| renameInternal(context, from, to); | |||
There was a problem hiding this comment.
can you please add some tests to CatalogTests and to ViewCatalogTests?
|
Updated PR with comments, working on resolving conflict |
| @@ -2506,6 +2506,29 @@ public void testNamespaceExistsViaHEADRequest() { | |||
| any()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void renameTableNamespaceWithCatalogName() { | |||
There was a problem hiding this comment.
please move this to CatalogTests so that we can test the same behavior with different catalog implementations
There was a problem hiding this comment.
Done, only RestCatalog(maybe Hive also?) handles this case for now.
| @@ -244,6 +244,33 @@ public void viewExistsViaHEADRequest() { | |||
| any()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void renameViewNamespaceHavingCatalogName() { | |||
There was a problem hiding this comment.
| public void renameViewNamespaceHavingCatalogName() { | |
| public void renameViewNamespaceWithCatalogName() { |
please move this to ViewCatalogTests so that we can test the same behavior with different catalog implementations
| if (requiresNamespaceCreate()) { | ||
| catalog().createNamespace(from.namespace()); | ||
| } | ||
| assertThat(catalog().viewExists(from)).as("View should not exist").isFalse(); |
There was a problem hiding this comment.
nit: please add a newline right before this line
| @@ -298,6 +294,37 @@ public void testTableRename() { | |||
| assertThat(validationCatalog.tableExists(renamedIdent)).as("New name should exist").isTrue(); | |||
| } | |||
|
|
|||
| @TestTemplate | |||
| public void testTableRenameInMultiLevelNameSpace() { | |||
There was a problem hiding this comment.
| public void testTableRenameInMultiLevelNameSpace() { | |
| public void testTableRenameWithNestedNamespace() { |
|
@huan233usc can you rebase your branch against latest |
|
Done |
Remove catalog name during rename
|
@huan233usc thanks for your patience on the PR here, I haven't forgotten about it but I need to explore how we can potentially fix this on the Spark side. @amogh-jahagirdar could you in the meantime also take a look at this please? |
This PR attempts to fix the issue mentioned in #11154.
On NoSuchNamespaceException, we retry rename table with stripping the catalog name if the first level in table identifier is catalog name