jerryshao commented on code in PR #5067:
URL: https://github.com/apache/gravitino/pull/5067#discussion_r1804897176
##########
core/src/main/java/org/apache/gravitino/listener/CatalogEventDispatcher.java:
##########
@@ -169,4 +173,17 @@ public void testConnection(
// TODO: Support event dispatching for testConnection
dispatcher.testConnection(ident, type, provider, comment, properties);
}
+
+ @Override
+ public void enableCatalog(NameIdentifier ident)
+ throws NoSuchCatalogException, CatalogNotInUseException {
+ // todo: support activate catalog event
Review Comment:
Will you do this in a different PR?
##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -562,38 +617,45 @@ 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, CatalogInUseException {
try {
+ boolean catalogInUse = catalogInUse(store, ident);
+ if (catalogInUse && !force) {
+ throw new CatalogInUseException(
+ "Catalog %s is in use, please disable 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) {
+ // the Kafka catalog is special, it includes a default schema
+ if (!catalogEntity.getProvider().equals("kafka") || schemas.size() >
1) {
+ throw new NonEmptyEntityException(
+ "Catalog %s has schemas, please drop them first or use force
option", ident);
}
}
- return store.delete(ident, EntityType.CATALOG);
- } catch (NoSuchEntityException e) {
+
+ // If reached here, it implies that the catalog is not in use or force
is true.
+ if (catalogInUse) {
+ // force is true, disable the catalog first
+ disableCatalog(ident);
+ }
Review Comment:
Since the catalog will be deleted, I don't think it is necessary to do the
disable here.
##########
api/src/main/java/org/apache/gravitino/SupportsCatalogs.java:
##########
@@ -108,12 +111,71 @@ Catalog alterCatalog(String catalogName, CatalogChange...
changes)
throws NoSuchCatalogException, IllegalArgumentException;
/**
- * Drop a catalog with specified identifier.
+ * Drop a catalog with specified name. Please make sure:
+ *
+ * <ul>
+ * <li>There is no schema in the catalog. Otherwise, a {@link
NonEmptyEntityException} will be
+ * thrown.
+ * <li>The method {@link #disableCatalog(String)} has been called before
dropping the catalog.
+ * Otherwise, a {@link CatalogInUseException} will be thrown.
+ * </ul>
+ *
+ * It is equivalent to calling {@code dropCatalog(ident, false)}.
*
* @param catalogName the name of the catalog.
- * @return True if the catalog was dropped, false otherwise.
+ * @return True if the catalog was dropped, false if the catalog does not
exist.
+ * @throws NonEmptyEntityException If the catalog is not empty.
+ * @throws CatalogInUseException If the catalog is in use.
+ */
+ default boolean dropCatalog(String catalogName)
+ throws NonEmptyEntityException, CatalogInUseException {
+ return dropCatalog(catalogName, false);
+ }
+
+ /**
+ * Drop a catalog with specified name. 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>
+ *
+ * If the force flag is false, it is equivalent to calling {@link
#dropCatalog(String)}.
+ *
+ * @param catalogName 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 CatalogInUseException If the catalog is in use and force is false.
+ */
+ boolean dropCatalog(String catalogName, boolean force)
+ throws NonEmptyEntityException, CatalogInUseException;
+
+ /**
+ * Enable a catalog. If the catalog is already enable, this method does
nothing.
Review Comment:
"If the catalog is already enabled..."
--
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]
