[#1237] fix(trino): Fix dropping iceberg schema failure (#1333)

### What changes were proposed in this pull request? Instead of catching exceptions when dropping schemas fails, throw them. ### Why are the changes needed? We can't determine whether the schema drop was successful or not based on its return value. Fix: #1237 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Existing UTs and ITs. Co-authored-by: Qi Yu <[email protected]>
apache · Jan 4, 2024 · b17edcd · b17edcd
1 parent 839f5c7
commit b17edcd
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 2 deletions.
diff --git a/...-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoConnectorIT.java b/...-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoConnectorIT.java
@@ -793,6 +793,7 @@ void testIcebergTableAndSchemaCreatedByTrino() {
   @Order(15)
   void testIcebergCatalogCreatedByGravitino() throws InterruptedException {
     String catalogName = GravitinoITUtils.genRandomName("iceberg_catalog").toLowerCase();
+    String schemaName = GravitinoITUtils.genRandomName("iceberg_catalog").toLowerCase();
     GravitinoMetaLake createdMetalake = client.loadMetalake(NameIdentifier.of(metalakeName));
     String[] command = {
       "mysql",
@@ -834,6 +835,42 @@ void testIcebergCatalogCreatedByGravitino() throws InterruptedException {
 
     String data = containerSuite.getTrinoContainer().executeQuerySQL(sql).get(0).get(0);
     Assertions.assertEquals(metalakeName + "." + catalogName, data);
+
+    catalog
+        .asSchemas()
+        .createSchema(
+            NameIdentifier.of(metalakeName, catalogName, schemaName),
+            "Created by gravitino client",
+            ImmutableMap.<String, String>builder().build());
+
+    sql =
+        String.format("show schemas in \"%s.%s\" like '%s'", metalakeName, catalogName, schemaName);
+    Assertions.assertTrue(checkTrinoHasLoaded(sql, 30));
+
+    final String sql1 =
+        String.format("drop schema \"%s.%s\".%s cascade", metalakeName, catalogName, schemaName);
+    // Will fail because the iceberg catalog does not support cascade drop
+    containerSuite.getTrinoContainer().executeUpdateSQL(sql1);
+
+    final String sql2 =
+        String.format("show schemas in \"%s.%s\" like '%s'", metalakeName, catalogName, schemaName);
+    success = checkTrinoHasLoaded(sql2, 30);
+    if (!success) {
+      Assertions.fail("Trino fail to load catalogs created by gravitino: " + sql2);
+    }
+
+    // Do not support the cascade drop
+    success =
+        catalog
+            .asSchemas()
+            .dropSchema(NameIdentifier.of(metalakeName, catalogName, schemaName), true);
+    Assertions.assertFalse(success);
+    final String sql3 =
+        String.format("show schemas in \"%s.%s\" like '%s'", metalakeName, catalogName, schemaName);
+    success = checkTrinoHasLoaded(sql3, 30);
+    if (!success) {
+      Assertions.fail("Trino fail to load catalogs created by gravitino: " + sql);
+    }
   }
 
   @Test

diff --git a/.../main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java b/.../main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java
@@ -149,8 +149,14 @@ public void createSchema(GravitinoSchema schema) {
 
   public void dropSchema(String schemaName, boolean cascade) {
     try {
-      schemaCatalog.dropSchema(
-          NameIdentifier.ofSchema(metalake.name(), catalogName, schemaName), cascade);
+      boolean success =
+          schemaCatalog.dropSchema(
+              NameIdentifier.ofSchema(metalake.name(), catalogName, schemaName), cascade);
+
+      if (!success) {
+        throw new TrinoException(GRAVITINO_SCHEMA_NOT_EXISTS, "Drop schema failed");
+      }
+
     } catch (NonEmptySchemaException e) {
       throw new TrinoException(GRAVITINO_SCHEMA_NOT_EMPTY, "Schema does not empty", e);
     }