This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 2ce6bdbb46 [#14272] fix(lance): refuse catalog-level DropNamespace on
non-lakehouse catalogs (#14273)
2ce6bdbb46 is described below
commit 2ce6bdbb46807c81cae38892e981fad82e27c36d
Author: Nevin Zheng <[email protected]>
AuthorDate: Tue Oct 27 19:42:52 2026 -0700
[#14272] fix(lance): refuse catalog-level DropNamespace on non-lakehouse
catalogs (#14273)
### What changes were proposed in this pull request?
- **`GravitinoLanceNameSpaceOperations.dropCatalog`** now loads the
catalog and applies the lakehouse
scope check before calling the backing `dropCatalog`. A non-lakehouse
catalog is refused with the same
`NamespaceNotFoundException` ("Catalog is not a lakehouse catalog:
<name>") that describe and exists
return, and the backing drop is never called.
- A missing catalog keeps its current handling (`FAIL` gives "Catalog
not found", `SKIP` succeeds) and
is no longer passed to the backing drop.
- **`GravitinoLanceNamespaceWrapper`:** the check in
`loadAndValidateLakehouseCatalog` moves into a
package-private `validateLakehouseCatalog`, so both paths throw the same
error.
### Why are the changes needed?
**The catalog-level drop was the only Lance operation without the
lakehouse scope check.** A Lance client
could drop any catalog in the metalake, Hive or Iceberg included,
although Lance cannot list or describe
those catalogs. The default `RESTRICT` request also reached the catalog
hook's privilege cleanup before
the drop was refused as in use (that ordering is fixed separately in
#14265).
Fix: #14272
### Does this PR introduce _any_ user-facing change?
Yes. Lance `DropNamespace` on a one-level id that names a non-lakehouse
catalog now returns
NamespaceNotFound (404) in every mode and behavior, and the catalog is
left unchanged. Lakehouse
catalogs and missing catalogs behave as before.
### How was this patch tested?
New cases in `TestGravitinoLanceNameSpaceOperations`, with a real
wrapper over a stubbed
`CatalogOperator`, so the production scope check runs:
- **Refused:** non-lakehouse catalog with no behavior, `RESTRICT` and
`CASCADE`; every non-lakehouse
kind (Iceberg, MySQL, fileset, and the `lakehouse-generic` provider on
the wrong type); `SKIP` mode;
the error matches describe.
- **Guards:** a lakehouse catalog still drops (`RESTRICT` →
`force=false`, `CASCADE` → `true`); a
lakehouse catalog in use keeps its error; a missing catalog keeps its
behavior.
Results:
- **On `main`:** 10 of the 17 `TestGravitinoLanceNameSpaceOperations`
cases fail (the non-lakehouse catalog is dropped); the 7 guards pass.
- **With the fix:** all 17 pass, as do the other Lance tests in
`lance-common` and `lance-rest-server`
(`TestGravitinoLanceNamespaceWrapper`,
`TestGravitinoLanceTableOperations` and others).
`./gradlew -PskipITs :lance:lance-common:test
:lance:lance-rest-server:test --tests 'org.apache.gravitino.lance.*'`
Co-authored-by: Qi Yu <[email protected]>
---
.../GravitinoLanceNameSpaceOperations.java | 29 ++--
.../gravitino/GravitinoLanceNamespaceWrapper.java | 14 +-
.../TestGravitinoLanceNameSpaceOperations.java | 166 +++++++++++++++++++++
3 files changed, 199 insertions(+), 10 deletions(-)
diff --git
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNameSpaceOperations.java
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNameSpaceOperations.java
index 2ca476ed9b..17b9b198b0 100644
---
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNameSpaceOperations.java
+++
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNameSpaceOperations.java
@@ -361,20 +361,23 @@ public class GravitinoLanceNameSpaceOperations implements
LanceNamespaceOperatio
private DropNamespaceResponse dropCatalog(
String catalogName, DropMode mode, DropBehavior behavior) {
+ // Only a lakehouse catalog is a Lance namespace. Check the scope before
the drop, as describe
+ // and exists do, so that a Lance request can never drop (or start
dropping) another catalog.
+ Catalog catalog;
+ try {
+ catalog = namespaceWrapper.loadCatalog(catalogName);
+ } catch (NoSuchCatalogException e) {
+ return catalogNotFound(catalogName, mode);
+ }
+ namespaceWrapper.validateLakehouseCatalog(catalogName, catalog);
+
try {
boolean dropped = namespaceWrapper.dropCatalog(catalogName, behavior ==
DropBehavior.CASCADE);
if (dropped) {
return new DropNamespaceResponse();
- } else {
- // Catalog did not exist
- if (mode == DropMode.FAIL) {
- throw new NamespaceNotFoundException(
- "Catalog not found: " + catalogName,
- CommonUtil.formatCurrentStackTrace(),
- catalogName);
- }
- return new DropNamespaceResponse(); // SKIP mode
}
+ // Catalog did not exist
+ return catalogNotFound(catalogName, mode);
} catch (NonEmptyCatalogException e) {
throw new InvalidInputException(
String.format("Catalog %s is not empty", catalogName),
@@ -393,6 +396,14 @@ public class GravitinoLanceNameSpaceOperations implements
LanceNamespaceOperatio
}
}
+ private DropNamespaceResponse catalogNotFound(String catalogName, DropMode
mode) {
+ if (mode == DropMode.FAIL) {
+ throw new NamespaceNotFoundException(
+ "Catalog not found: " + catalogName,
CommonUtil.formatCurrentStackTrace(), catalogName);
+ }
+ return new DropNamespaceResponse(); // SKIP mode
+ }
+
private DropNamespaceResponse dropSchema(
String catalogName, String schemaName, DropMode mode, DropBehavior
behavior) {
try {
diff --git
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java
index d6880ce320..b36ac200ef 100644
---
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java
+++
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNamespaceWrapper.java
@@ -205,13 +205,25 @@ public class GravitinoLanceNamespaceWrapper extends
NamespaceWrapper {
throw new NamespaceNotFoundException(
"Catalog not found: " + catalogName,
CommonUtil.formatCurrentStackTrace(), catalogName);
}
+ validateLakehouseCatalog(catalogName, catalog);
+ return catalog;
+ }
+
+ /**
+ * Rejects a catalog that is not a lakehouse catalog. Lance only exposes
lakehouse catalogs as
+ * namespaces, so any other catalog is reported as a missing namespace.
+ *
+ * @param catalogName the catalog name used in the error
+ * @param catalog the loaded catalog
+ * @throws NamespaceNotFoundException if the catalog is not a lakehouse
catalog
+ */
+ void validateLakehouseCatalog(String catalogName, Catalog catalog) {
if (!isLakehouseCatalog(catalog)) {
throw new NamespaceNotFoundException(
"Catalog is not a lakehouse catalog: " + catalogName,
CommonUtil.formatCurrentStackTrace(),
catalogName);
}
- return catalog;
}
String[] listSchemas(Catalog catalog) throws NoSuchCatalogException {
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceNameSpaceOperations.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceNameSpaceOperations.java
index 51629e4478..078049bb04 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceNameSpaceOperations.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceNameSpaceOperations.java
@@ -18,19 +18,32 @@
*/
package org.apache.gravitino.lance.common.ops.gravitino;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.regex.Pattern;
import org.apache.gravitino.Catalog;
+import org.apache.gravitino.exceptions.CatalogInUseException;
+import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.lance.namespace.errors.InvalidInputException;
import org.lance.namespace.errors.NamespaceNotFoundException;
+import org.lance.namespace.model.DropNamespaceResponse;
import org.lance.namespace.model.ListNamespacesResponse;
import org.mockito.Mockito;
class TestGravitinoLanceNameSpaceOperations {
private static final String DELIMITER = Pattern.quote(".");
+ private static final String LAKEHOUSE_PROVIDER = "lakehouse-generic";
@Test
void testListNamespacesOnSchemaReturnsEmptyList() {
@@ -91,4 +104,157 @@ class TestGravitinoLanceNameSpaceOperations {
() -> operations.listNamespaces("bogus_catalog", DELIMITER, null,
null));
Assertions.assertEquals("Catalog not found: bogus_catalog",
exception.getMessage());
}
+
+ @ParameterizedTest
+ @NullSource
+ @ValueSource(strings = {"restrict", "cascade"})
+ void testDropCatalogRefusesNonLakehouseCatalog(String behavior) {
+ // A Hive catalog is invisible to Lance list, describe and exists, so
Lance must not drop it.
+ // The default (null) behavior is RESTRICT, which would otherwise reach
the catalog hook's
+ // privilege cleanup before the drop is refused as "in use".
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ Catalog catalog = mockCatalog(Catalog.Type.RELATIONAL, "hive");
+ when(operator.loadCatalog("sales")).thenReturn(catalog);
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ NamespaceNotFoundException exception =
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.dropNamespace("sales", DELIMITER, "fail",
behavior));
+
+ Assertions.assertEquals("Catalog is not a lakehouse catalog: sales",
exception.getMessage());
+ Assertions.assertEquals("sales", exception.getInstance());
+ verify(operator, never()).dropCatalog(anyString(), anyBoolean());
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "RELATIONAL, lakehouse-iceberg",
+ "RELATIONAL, jdbc-mysql",
+ "FILESET, fileset",
+ "FILESET, lakehouse-generic",
+ "MODEL, lakehouse-generic"
+ })
+ void testDropCatalogRefusesEveryNonLakehouseKind(Catalog.Type type, String
provider) {
+ // Both the type and the provider must match. The right provider on the
wrong type is refused.
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ Catalog catalog = mockCatalog(type, provider);
+ when(operator.loadCatalog("other")).thenReturn(catalog);
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.dropNamespace("other", DELIMITER, "fail", "cascade"));
+ verify(operator, never()).dropCatalog(anyString(), anyBoolean());
+ }
+
+ @Test
+ void testDropCatalogRefusesNonLakehouseCatalogInSkipMode() {
+ // SKIP only tolerates a missing catalog. An existing non-lakehouse
catalog is still refused,
+ // as it is for a schema-level drop under a non-lakehouse catalog.
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ Catalog catalog = mockCatalog(Catalog.Type.RELATIONAL, "hive");
+ when(operator.loadCatalog("sales")).thenReturn(catalog);
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.dropNamespace("sales", DELIMITER, "skip", "cascade"));
+ verify(operator, never()).dropCatalog(anyString(), anyBoolean());
+ }
+
+ @Test
+ void testDropCatalogErrorMatchesDescribeForNonLakehouseCatalog() {
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ Catalog catalog = mockCatalog(Catalog.Type.RELATIONAL, "hive");
+ when(operator.loadCatalog("sales")).thenReturn(catalog);
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ NamespaceNotFoundException describeException =
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.describeNamespace("sales", DELIMITER));
+ Exception dropException =
+ Assertions.assertThrows(
+ Exception.class, () -> operations.dropNamespace("sales",
DELIMITER, null, null));
+
+ Assertions.assertEquals(describeException.getClass(),
dropException.getClass());
+ Assertions.assertEquals(describeException.getMessage(),
dropException.getMessage());
+ Assertions.assertEquals(
+ describeException.getInstance(),
+ ((NamespaceNotFoundException) dropException).getInstance());
+ }
+
+ @ParameterizedTest
+ @CsvSource({"restrict, false", "cascade, true"})
+ void testDropLakehouseCatalogStillDrops(String behavior, boolean
expectedForce) {
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ Catalog catalog = mockCatalog(Catalog.Type.RELATIONAL, LAKEHOUSE_PROVIDER);
+ when(operator.loadCatalog("lance_cat")).thenReturn(catalog);
+ when(operator.dropCatalog("lance_cat", expectedForce)).thenReturn(true);
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ DropNamespaceResponse response =
+ operations.dropNamespace("lance_cat", DELIMITER, "fail", behavior);
+
+ Assertions.assertNotNull(response);
+ verify(operator).dropCatalog("lance_cat", expectedForce);
+ }
+
+ @Test
+ void testDropLakehouseCatalogInUseKeepsExistingError() {
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ Catalog catalog = mockCatalog(Catalog.Type.RELATIONAL, LAKEHOUSE_PROVIDER);
+ when(operator.loadCatalog("lance_cat")).thenReturn(catalog);
+ when(operator.dropCatalog("lance_cat", false))
+ .thenThrow(new CatalogInUseException("Catalog %s is in use",
"lance_cat"));
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ InvalidInputException exception =
+ Assertions.assertThrows(
+ InvalidInputException.class,
+ () -> operations.dropNamespace("lance_cat", DELIMITER, null,
null));
+ Assertions.assertEquals("Catalog lance_cat is in use",
exception.getMessage());
+ }
+
+ @Test
+ void testDropMissingCatalogKeepsExistingBehavior() {
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator = mockOperator();
+ when(operator.loadCatalog("bogus"))
+ .thenThrow(new NoSuchCatalogException("Catalog %s does not exist",
"bogus"));
+ when(operator.dropCatalog(anyString(), anyBoolean())).thenReturn(false);
+ GravitinoLanceNameSpaceOperations operations = newOperations(operator);
+
+ NamespaceNotFoundException exception =
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.dropNamespace("bogus", DELIMITER, "fail",
"restrict"));
+ Assertions.assertEquals("Catalog not found: bogus",
exception.getMessage());
+ Assertions.assertEquals("bogus", exception.getInstance());
+
+ Assertions.assertNotNull(operations.dropNamespace("bogus", DELIMITER,
"skip", "cascade"));
+ }
+
+ private static GravitinoLanceNamespaceWrapper.CatalogOperator mockOperator()
{
+ // By default the backing drop succeeds, as it would for any catalog the
caller owns.
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator =
+ Mockito.mock(GravitinoLanceNamespaceWrapper.CatalogOperator.class);
+ when(operator.dropCatalog(anyString(), anyBoolean())).thenReturn(true);
+ return operator;
+ }
+
+ private static GravitinoLanceNameSpaceOperations newOperations(
+ GravitinoLanceNamespaceWrapper.CatalogOperator operator) {
+ // A real wrapper, so the scope check under test is the production one.
+ GravitinoLanceNamespaceWrapper wrapper = new
GravitinoLanceNamespaceWrapper();
+ wrapper.setCatalogOperator(operator);
+ return new GravitinoLanceNameSpaceOperations(wrapper);
+ }
+
+ private static Catalog mockCatalog(Catalog.Type type, String provider) {
+ Catalog catalog = Mockito.mock(Catalog.class);
+ when(catalog.type()).thenReturn(type);
+ when(catalog.provider()).thenReturn(provider);
+ return catalog;
+ }
}
