danielcweeks commented on code in PR #16584:
URL: https://github.com/apache/iceberg/pull/16584#discussion_r3501818885
##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -488,6 +490,27 @@ public static void dropTable(Catalog catalog,
TableIdentifier ident) {
}
}
+ public static UnregisterTableResponse unregisterTable(Catalog catalog,
TableIdentifier ident) {
+ // capture the last metadata before dropping so it can be returned for
re-registration
+ Table table = catalog.loadTable(ident);
+ if (!(table instanceof BaseTable)) {
+ throw new IllegalStateException("Cannot wrap catalog that does not
produce BaseTable");
+ }
+
+ TableMetadata metadata = ((BaseTable) table).operations().current();
+
+ // unregister without removing the underlying data and metadata files
+ boolean dropped = catalog.dropTable(ident, false /* do not purge */);
+ if (!dropped) {
+ throw new NoSuchTableException("Table does not exist: %s", ident);
Review Comment:
This doesn't look quite right. `!dropped` could also mean that you weren't
allowed to drop the table. By the time we get to this point, we already have
the table metadata and it was looked up. I would use a generic error and say
something like "Unregister failed. Table <id> not dropped."
I get the java doc says that `false = table did not exist` but that's
actually not quite a complete statement of the behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
