Rearrange try/catch blocks in HiveMetastoreTableOperations

The only retryable failures in this block are the `replaceTable` call and the CommitFailedException already thrown if the metadata location has been updated. Exceptions thrown by checkState, or because the table does not exist should not retry.
trinodb · May 16, 2022 · ccb884d · ccb884d
1 parent 0f71007
commit ccb884d
Showing 1 changed file with 17 additions and 18 deletions.
diff --git a/...eberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/HiveMetastoreTableOperations.java b/...eberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/HiveMetastoreTableOperations.java
@@ -70,27 +70,26 @@ protected void commitToExistingTable(TableMetadata base, TableMetadata metadata)
                 database,
                 tableName);
         try {
-            Table table;
-            try {
-                Table currentTable = fromMetastoreApiTable(thriftMetastore.getTable(identity, database, tableName)
-                        .orElseThrow(() -> new TableNotFoundException(getSchemaTableName())));
+            Table currentTable = fromMetastoreApiTable(thriftMetastore.getTable(identity, database, tableName)
+                    .orElseThrow(() -> new TableNotFoundException(getSchemaTableName())));
 
-                checkState(currentMetadataLocation != null, "No current metadata location for existing table");
-                String metadataLocation = currentTable.getParameters().get(METADATA_LOCATION_PROP);
-                if (!currentMetadataLocation.equals(metadataLocation)) {
-                    throw new CommitFailedException("Metadata location [%s] is not same as table metadata location [%s] for %s",
-                            currentMetadataLocation, metadataLocation, getSchemaTableName());
-                }
+            checkState(currentMetadataLocation != null, "No current metadata location for existing table");
+            String metadataLocation = currentTable.getParameters().get(METADATA_LOCATION_PROP);
+            if (!currentMetadataLocation.equals(metadataLocation)) {
+                throw new CommitFailedException("Metadata location [%s] is not same as table metadata location [%s] for %s",
+                        currentMetadataLocation, metadataLocation, getSchemaTableName());
+            }
 
-                table = Table.builder(currentTable)
-                        .setDataColumns(toHiveColumns(metadata.schema().columns()))
-                        .withStorage(storage -> storage.setLocation(metadata.location()))
-                        .setParameter(METADATA_LOCATION_PROP, newMetadataLocation)
-                        .setParameter(PREVIOUS_METADATA_LOCATION_PROP, currentMetadataLocation)
-                        .build();
+            Table table = Table.builder(currentTable)
+                    .setDataColumns(toHiveColumns(metadata.schema().columns()))
+                    .withStorage(storage -> storage.setLocation(metadata.location()))
+                    .setParameter(METADATA_LOCATION_PROP, newMetadataLocation)
+                    .setParameter(PREVIOUS_METADATA_LOCATION_PROP, currentMetadataLocation)
+                    .build();
 
-                // todo privileges should not be replaced for an alter
-                PrincipalPrivileges privileges = table.getOwner().map(MetastoreUtil::buildInitialPrivilegeSet).orElse(NO_PRIVILEGES);
+            // todo privileges should not be replaced for an alter
+            PrincipalPrivileges privileges = table.getOwner().map(MetastoreUtil::buildInitialPrivilegeSet).orElse(NO_PRIVILEGES);
+            try {
                 metastore.replaceTable(database, tableName, table, privileges);
             }
             catch (RuntimeException e) {