This is an automated email from the ASF dual-hosted git repository.
jmclean pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new b6e6a233ac [#8161] Fix removeCatalog to respect the ignoreIfNotExists
flag… (#8237)
b6e6a233ac is described below
commit b6e6a233acc50945e72f96114a1d280c0c7c47fd
Author: Surya B <[email protected]>
AuthorDate: Mon Aug 25 18:37:37 2025 +0530
[#8161] Fix removeCatalog to respect the ignoreIfNotExists flag… (#8237)
# What changes were proposed in this pull request?
- fixes spotless Apply CI failure on previous PR
- Updated `removeCatalog` implementation to respect the
`ignoreIfNotExists` flag.
- If the catalog does not exist and `ignoreIfNotExists = true`, the
method now exits silently.
- If the catalog does not exist and `ignoreIfNotExists = false`, a
`CatalogException` is thrown.
- Wrapped underlying errors in `CatalogException` for consistency.
# Why are the changes needed?
- The previous implementation ignored the `ignoreIfNotExists` flag.
- This led to unexpected exceptions even when users explicitly indicated
they wanted missing catalogs to be ignored.
- Fixing this ensures more predictable behavior and aligns with the
contract of the API.
- Fix: #8161
# Does this PR introduce any user-facing change?
- **Yes.**
- Users who set `ignoreIfNotExists = true` will no longer see
unnecessary exceptions if the catalog is missing.
- Behavior is now consistent with expected API semantics.
---------
Co-authored-by: Justin Mclean <[email protected]>
---
.../connector/store/GravitinoCatalogStore.java | 13 ++-
.../connector/store/TestGravitinoCatalogStore.java | 97 ++++++++++++++++++++++
2 files changed, 108 insertions(+), 2 deletions(-)
diff --git
a/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/store/GravitinoCatalogStore.java
b/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/store/GravitinoCatalogStore.java
index 726cf6aa6b..e1e95a3ef7 100644
---
a/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/store/GravitinoCatalogStore.java
+++
b/flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/store/GravitinoCatalogStore.java
@@ -66,11 +66,20 @@ public class GravitinoCatalogStore extends
AbstractCatalogStore {
catalogFactory.gravitinoCatalogProvider(),
gravitinoProperties);
}
-
+ /**
+ * Removes the specified catalog.
+ *
+ * @param catalogName name of the catalog to remove
+ * @param ignoreIfNotExists if true, ignore when the catalog does not exist
+ * @throws CatalogException if the catalog cannot be removed
+ */
@Override
public void removeCatalog(String catalogName, boolean ignoreIfNotExists)
throws CatalogException {
try {
- gravitinoCatalogManager.dropCatalog(catalogName);
+ boolean isDropped = gravitinoCatalogManager.dropCatalog(catalogName);
+ if (!ignoreIfNotExists && !isDropped) {
+ throw new CatalogException(String.format("Failed to remove the
catalog: %s", catalogName));
+ }
} catch (Exception e) {
throw new CatalogException(String.format("Failed to remove the catalog:
%s", catalogName), e);
}
diff --git
a/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/store/TestGravitinoCatalogStore.java
b/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/store/TestGravitinoCatalogStore.java
new file mode 100644
index 0000000000..228d03e5e9
--- /dev/null
+++
b/flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/store/TestGravitinoCatalogStore.java
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.flink.connector.store;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+import org.apache.gravitino.flink.connector.catalog.GravitinoCatalogManager;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestGravitinoCatalogStore {
+ private GravitinoCatalogManager gravitinoCatalogMockManager;
+ private GravitinoCatalogStore gravitinoCatalogStore;
+
+ @Before
+ public void setupCatalogStore() {
+ gravitinoCatalogMockManager = mock(GravitinoCatalogManager.class);
+ gravitinoCatalogStore = new
GravitinoCatalogStore(gravitinoCatalogMockManager);
+ }
+
+ @Test
+ public void testRemoveCatalog_whenCatalogExists_shouldSucceed() {
+ String catalogName = "testCatalog";
+
when(gravitinoCatalogMockManager.dropCatalog(catalogName)).thenReturn(true);
+ try {
+ gravitinoCatalogStore.removeCatalog(catalogName, false);
+ } catch (Exception e) {
+ fail("Expected no exception, but got: " + e.getMessage());
+ }
+ verify(gravitinoCatalogMockManager).dropCatalog(catalogName);
+ }
+
+ @Test
+ public void
testRemoveCatalog_whenCatalogNotExists_ignoreFlagTrue_shouldNotThrow() {
+ String catalogName = "missingCatalog";
+
when(gravitinoCatalogMockManager.dropCatalog(catalogName)).thenReturn(false);
+ try {
+ gravitinoCatalogStore.removeCatalog(catalogName, true);
+ } catch (Exception e) {
+ fail("Expected no exception, but got: " + e.getMessage());
+ }
+ verify(gravitinoCatalogMockManager).dropCatalog(catalogName);
+ }
+
+ @Test
+ public void
testRemoveCatalog_whenCatalogNotExists_ignoreFlagFalse_shouldThrow() {
+ String catalogName = "missingCatalog";
+
when(gravitinoCatalogMockManager.dropCatalog(catalogName)).thenReturn(false);
+ try {
+ gravitinoCatalogStore.removeCatalog(catalogName, false);
+ fail("Expected CatalogException to be thrown");
+ } catch (CatalogException e) {
+ assertTrue(
+ "Expected failure message to contain 'Failed to remove the
catalog:'",
+ e.getMessage().contains("Failed to remove the catalog:"));
+ }
+ verify(gravitinoCatalogMockManager).dropCatalog(catalogName);
+ }
+
+ @Test
+ public void testRemoveCatalog_UnexpectedException_shouldThrow() {
+ String catalogName = "errorCatalog";
+ when(gravitinoCatalogMockManager.dropCatalog(catalogName))
+ .thenThrow(new RuntimeException("UnexpectedErrorOccurred"));
+ try {
+ gravitinoCatalogStore.removeCatalog(catalogName, false);
+ fail("Expected CatalogException to be thrown");
+ } catch (CatalogException e) {
+ assertTrue(
+ "Expected failure message to contain 'Failed to remove the
catalog:'",
+ e.getMessage().contains("Failed to remove the catalog:"));
+ assertTrue("Expected cause to be RuntimeException", e.getCause()
instanceof RuntimeException);
+ }
+ verify(gravitinoCatalogMockManager).dropCatalog(catalogName);
+ }
+}
