Conversation
Co-authored-by: Daniel Weeks <daniel.c.weeks@gmail.com>
Co-authored-by: Marco Kroll <marco.kroll@databricks.com>
Co-authored-by: Marco Kroll <marco.kroll@databricks.com>
Co-authored-by: Daniel Weeks <daniel.c.weeks@gmail.com>
| mutationHeaders, | ||
| ErrorHandlers.tableErrorHandler()); | ||
| StaticTableOperations ops = new StaticTableOperations(response.metadata(), io); | ||
| return new BaseTable(ops, fullTableName(identifier)); |
There was a problem hiding this comment.
I don't think we want to include the fullTableName. This should be the name component of the table without the namespace. At this point the table is unregistered, so it does not live in a namespace.
|
Thanks for getting this going again @mkroll-db! This fell off my radar. |
| } | ||
|
|
||
| @Test | ||
| public void testUnregisterRetriesAfterConcurrentCommit() { |
There was a problem hiding this comment.
These two tests don't make sense to me. We should just be testing the unregister functionality in isolation. I don't think concurrent commit is any different from just validating that the table no longer exists (loading the table doesn't start any sort of transaction, so this is more performative than practical)
|
|
||
| assertThatThrownBy(() -> unregisteringCatalog.unregisterTable(tableIdentifier)) | ||
| .isInstanceOf(CommitFailedException.class) | ||
| .hasMessage("Cannot unregister table db.table: metadata location changed concurrently"); |
There was a problem hiding this comment.
There's currently no way to determine that the metadata location changed, so this test doesn't make sense.
| String.format( | ||
| "{\n" | ||
| + " \"metadata-location\" : \"metadata-location\",\n" | ||
| + " \"metadata\" : {\n" |
There was a problem hiding this comment.
I don't feel like we need all of the table metadata hard coded like this. We're round tripping the request/response object parser, not the table metadata parser. We should be able to reuse the table metadata parser and simply this a lot.
|
@rambleraptor I'm fine in closing the current PR and reviving yours. I just wanted to ensure that we continue working on it. It fell off my radar first so. |
This branch has not been deployed
3 participants
Adds catalog-level
unregisterTablesupport introduced in #16400 in the REST catalog fixture and compatibility kit.Unregister removes a table from the catalog without deleting its data or metadata files.
It returns a read-only table containing the last registered metadata. This can be used to register the table (with another catalog) again.
Disclaimer
Parts of the code were generated with AI.
This is a reopen of #16584 from @rambleraptor