jerryshao commented on code in PR #5067:
URL: https://github.com/apache/gravitino/pull/5067#discussion_r1804235049


##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -562,38 +627,44 @@ public Catalog alterCatalog(NameIdentifier ident, 
CatalogChange... changes)
     }
   }
 
-  /**
-   * Drops (deletes) the catalog with the specified identifier.
-   *
-   * @param ident The identifier of the catalog to drop.
-   * @return {@code true} if the catalog was successfully dropped, {@code 
false} otherwise.
-   */
   @Override
-  public boolean dropCatalog(NameIdentifier ident) {
-    // There could be a race issue that someone is using the catalog while we 
are dropping it.
-    catalogCache.invalidate(ident);
-
+  public boolean dropCatalog(NameIdentifier ident, boolean force)
+      throws NonEmptyEntityException, EntityInUseException {
     try {
+      boolean catalogInUse = catalogInUse(store, ident);
+      if (catalogInUse && !force) {
+        throw new EntityInUseException(
+            "Catalog %s is in use, please deactivate it first or use force 
option", ident);
+      }
+
+      List<SchemaEntity> schemas =
+          store.list(
+              Namespace.of(ident.namespace().level(0), ident.name()),
+              SchemaEntity.class,
+              EntityType.SCHEMA);
       CatalogEntity catalogEntity = store.get(ident, EntityType.CATALOG, 
CatalogEntity.class);
-      if (catalogEntity.getProvider().equals("kafka")) {
-        // Kafka catalog needs to cascade drop the default schema
-        List<SchemaEntity> schemas =
-            store.list(
-                Namespace.of(ident.namespace().level(0), ident.name()),
-                SchemaEntity.class,
-                EntityType.SCHEMA);
-        // If there is only one schema, it must be the default schema, because 
we don't allow to
-        // drop the default schema.
-        if (schemas.size() == 1) {
-          return store.delete(ident, EntityType.CATALOG, true);
-        }
+
+      if (!schemas.isEmpty()
+          && !force
+          && (!catalogEntity.getProvider().equals("kafka") || schemas.size() > 
1)) {

Review Comment:
   The expression here is difficult to understand, maybe we can use multiple 
`if` to describe the exception message for different cases.



-- 
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]

Reply via email to