mkroll-db commented on code in PR #18203:
URL: https://github.com/apache/iceberg/pull/18203#discussion_r4102994720
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java:
##########
@@ -297,6 +312,60 @@ protected String defaultWarehouseLocation(TableIdentifier
table) {
return SLASH.join(defaultNamespaceLocation(table.namespace()),
tableLocation);
}
+ @Override
+ public Table unregisterTable(TableIdentifier identifier) {
+ Preconditions.checkArgument(
+ identifier != null && isValidIdentifier(identifier), "Invalid
identifier: %s", identifier);
+
+ TableMetadata initialMetadata = newTableOps(identifier).current();
+ if (initialMetadata == null) {
+ throw new NoSuchTableException("Table does not exist: %s", identifier);
+ }
+
+ AtomicReference<Table> unregistered = new AtomicReference<>();
+ Tasks.foreach(identifier)
+ .retry(initialMetadata.propertyAsInt(COMMIT_NUM_RETRIES,
COMMIT_NUM_RETRIES_DEFAULT))
+ .exponentialBackoff(
+ initialMetadata.propertyAsInt(
+ COMMIT_MIN_RETRY_WAIT_MS, COMMIT_MIN_RETRY_WAIT_MS_DEFAULT),
+ initialMetadata.propertyAsInt(
+ COMMIT_MAX_RETRY_WAIT_MS, COMMIT_MAX_RETRY_WAIT_MS_DEFAULT),
+ initialMetadata.propertyAsInt(
+ COMMIT_TOTAL_RETRY_TIME_MS,
COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT),
+ 2.0 /* exponential */)
+ .onlyRetryOn(CommitFailedException.class)
+ .run(tableIdentifier ->
unregistered.set(unregisterTableOnce(tableIdentifier)));
Review Comment:
Initially I had a `foreach` (endless)-loop which I didn't like. I then
looked at the code and found that `exponentialBackoff` is used which was much
better.
BUT I agree it's overkill. I removed it in
d07d8c6c9e78b5d38a12c557c6ace758e7a3aa6b
##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -488,6 +491,24 @@ public static void dropTable(Catalog catalog,
TableIdentifier ident) {
}
}
+ public static UnregisterTableResponse unregisterTable(Catalog catalog,
TableIdentifier ident) {
+ if (MetadataTableType.from(ident.name()) != null) {
+ throw new NoSuchTableException("Table does not exist: %s", ident);
+ }
Review Comment:
Double checked and `unregister` already handles this scenario, so it makes
no sense to keep it.
I removed it in d07d8c6c9e78b5d38a12c557c6ace758e7a3aa6b
--
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]
