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


##########
api/src/main/java/org/apache/gravitino/SupportsCatalogs.java:
##########
@@ -108,12 +110,67 @@ Catalog alterCatalog(String catalogName, CatalogChange... 
changes)
       throws NoSuchCatalogException, IllegalArgumentException;
 
   /**
-   * Drop a catalog with specified identifier.
+   * Drop a catalog with specified identifier. Please make sure:
    *
-   * @param catalogName the name of the catalog.
-   * @return True if the catalog was dropped, false otherwise.
+   * <ul>
+   *   <li>There is no schema in the catalog. Otherwise, a {@link 
NonEmptyEntityException} will be
+   *       thrown.
+   *   <li>The method {@link #inactivateCatalog(NameIdentifier)} has been 
called before dropping the
+   *       catalog. Otherwise, a {@link EntityInUseException} will be thrown.
+   * </ul>
+   *
+   * It is equivalent to calling {@code dropCatalog(ident, false)}.
+   *
+   * @param ident the identifier of the catalog.
+   * @return True if the catalog was dropped, false if the catalog does not 
exist.
+   * @throws NonEmptyEntityException If the catalog is not empty.
+   * @throws EntityInUseException If the catalog is in use.
+   */
+  boolean dropCatalog(NameIdentifier ident) throws NonEmptyEntityException, 
EntityInUseException;

Review Comment:
   We should use String catalog name, not catalog name identifier.



##########
api/src/main/java/org/apache/gravitino/SupportsCatalogs.java:
##########
@@ -108,12 +110,67 @@ Catalog alterCatalog(String catalogName, CatalogChange... 
changes)
       throws NoSuchCatalogException, IllegalArgumentException;
 
   /**
-   * Drop a catalog with specified identifier.
+   * Drop a catalog with specified identifier. Please make sure:
    *
-   * @param catalogName the name of the catalog.
-   * @return True if the catalog was dropped, false otherwise.
+   * <ul>
+   *   <li>There is no schema in the catalog. Otherwise, a {@link 
NonEmptyEntityException} will be
+   *       thrown.
+   *   <li>The method {@link #inactivateCatalog(NameIdentifier)} has been 
called before dropping the
+   *       catalog. Otherwise, a {@link EntityInUseException} will be thrown.
+   * </ul>
+   *
+   * It is equivalent to calling {@code dropCatalog(ident, false)}.
+   *
+   * @param ident the identifier of the catalog.
+   * @return True if the catalog was dropped, false if the catalog does not 
exist.
+   * @throws NonEmptyEntityException If the catalog is not empty.
+   * @throws EntityInUseException If the catalog is in use.
+   */
+  boolean dropCatalog(NameIdentifier ident) throws NonEmptyEntityException, 
EntityInUseException;
+
+  /**
+   * Drop a catalog with specified identifier. If the force flag is true, it 
will:
+   *
+   * <ul>
+   *   <li>Cascade drop all sub-entities (schemas, tables, etc.) of the 
catalog in Gravitino store.
+   *   <li>Drop the catalog even if it is in use.
+   *   <li>External resources (e.g. database, table, etc.) associated with 
sub-entities will not be
+   *       dropped unless it is managed (such as managed fileset).
+   * </ul>
+   *
+   * @param ident The identifier of the catalog.
+   * @param force Whether to force the drop.
+   * @return True if the catalog was dropped, false if the catalog does not 
exist.
+   * @throws NonEmptyEntityException If the catalog is not empty and force is 
false.
+   * @throws EntityInUseException If the catalog is in use and force is false.
+   */
+  boolean dropCatalog(NameIdentifier ident, boolean force)
+      throws NonEmptyEntityException, EntityInUseException;
+
+  /**
+   * Activate a catalog. If the catalog is already active, this method does 
nothing.
+   *
+   * @param ident The identifier of the catalog.
+   * @throws NoSuchCatalogException If the catalog does not exist.
+   */
+  void activateCatalog(NameIdentifier ident) throws NoSuchCatalogException;
+
+  /**
+   * Inactivate a catalog. If the catalog is already inactive, this method 
does nothing. Once a
+   * catalog is inactivated:
+   *
+   * <ul>
+   *   <li>It can only be listed, loaded, dropped, or activated.
+   *   <li>Any other operations on the catalog will throw an {@link
+   *       org.apache.gravitino.exceptions.NonInUseEntityException}.
+   *   <li>Any operation on the sub-entities (schemas, tables, etc.) will 
throw an {@link
+   *       org.apache.gravitino.exceptions.NonInUseEntityException}.
+   * </ul>
+   *
+   * @param ident The identifier of the catalog.
+   * @throws NoSuchCatalogException If the catalog does not exist.
    */
-  boolean dropCatalog(String catalogName);
+  void inactivateCatalog(NameIdentifier ident) throws NoSuchCatalogException;

Review Comment:
   deactivate, not inactivate.



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