From fe2ec489a4b2823a22861c4dc23985f7f0f95ce7 Mon Sep 17 00:00:00 2001 From: Qi Yu Date: Thu, 19 Oct 2023 18:01:59 +0800 Subject: [PATCH] [#555] improvement(hive): Improve error message about altering an unknown 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. --- .../gravitino/catalog/hive/HiveCatalogOperations.java | 5 +---- .../gravitino/catalog/CatalogOperationDispatcher.java | 3 +++ .../integration/test/catalog/hive/CatalogHiveIT.java | 11 +++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java index 325585e97a..5e6ed46a9b 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java +++ b/catalogs/catalog-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 index 8549d8ff4a..420c743910 100644 --- 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 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/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/hive/CatalogHiveIT.java b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/hive/CatalogHiveIT.java index 4cfd3502c1..4ce583b3d4 100644 --- a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/hive/CatalogHiveIT.java +++ b/integration-test/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();