[#555] improvement(hive): Improve error message about altering an unk…

…nown table (#556) ### What changes were proposed in this pull request? Change the exception class from `RuntimeException` to `NoSuchTableException` when altering an unknown table. ### Why are the changes needed? Improving error messages to be more accurate and readable. Fix: #555 ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Add test int IT.
apache · Oct 19, 2023 · fe2ec48 · fe2ec48
1 parent bdb2ab0
commit fe2ec48
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 4 deletions.
diff --git a/...talog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java b/...talog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java
@@ -661,9 +661,6 @@ public Table alterTable(NameIdentifier tableIdent, TableChange... changes)
       LOG.info("Altered Hive table {} in Hive Metastore", tableIdent.name());
       return HiveTable.fromHiveTable(alteredHiveTable);
 
-    } catch (NoSuchObjectException e) {
-      throw new NoSuchTableException(
-          String.format("Hive table does not exist: %s in Hive Metastore", tableIdent.name()), e);
     } catch (TException | InterruptedException e) {
       if (e.getMessage().contains("types incompatible with the existing columns")) {
         throw new IllegalArgumentException(
@@ -677,7 +674,7 @@ public Table alterTable(NameIdentifier tableIdent, TableChange... changes)
       }
       throw new RuntimeException(
           "Failed to alter Hive table " + tableIdent.name() + " in Hive metastore", e);
-    } catch (IllegalArgumentException e) {
+    } catch (IllegalArgumentException | NoSuchTableException e) {
       throw e;
     } catch (Exception e) {
       throw new RuntimeException(e);

diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java
@@ -607,6 +607,9 @@ private <R, E1 extends Throwable, E2 extends Throwable> R doWithCatalog(
       } else if (ex2.isInstance(throwable)) {
         throw ex2.cast(throwable);
       }
+      if (RuntimeException.class.isAssignableFrom(throwable.getClass())) {
+        throw (RuntimeException) throwable;
+      }
 
       throw new RuntimeException(throwable);
     }

diff --git a/...t/src/test/java/com/datastrato/gravitino/integration/test/catalog/hive/CatalogHiveIT.java b/...t/src/test/java/com/datastrato/gravitino/integration/test/catalog/hive/CatalogHiveIT.java
@@ -34,6 +34,7 @@
 import com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata;
 import com.datastrato.gravitino.client.GravitinoMetaLake;
 import com.datastrato.gravitino.dto.rel.ColumnDTO;
+import com.datastrato.gravitino.exceptions.NoSuchTableException;
 import com.datastrato.gravitino.integration.test.util.AbstractIT;
 import com.datastrato.gravitino.integration.test.util.GravitinoITUtils;
 import com.datastrato.gravitino.rel.Distribution;
@@ -580,6 +581,16 @@ private void assertTableEquals(
     Assertions.assertEquals(partitionKeys, hivePartitionKeys);
   }
 
+  @Test
+  void testAlterUnknownTable() {
+    NameIdentifier identifier = NameIdentifier.of(metalakeName, catalogName, schemaName, "unknown");
+    Assertions.assertThrows(
+        NoSuchTableException.class,
+        () -> {
+          catalog.asTableCatalog().alterTable(identifier, TableChange.updateComment("new_comment"));
+        });
+  }
+
   @Test
   public void testAlterHiveTable() throws TException, InterruptedException {
     ColumnDTO[] columns = createColumns();