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 101e3475a3 [#12468] fix(lance): validate the identifier in
ListNamespaces at schema level (#12469)
101e3475a3 is described below
commit 101e3475a3706cb2a903c695bdfd8664bc5d1250
Author: FANNG <[email protected]>
AuthorDate: Tue Aug 18 19:38:56 2026 +0800
[#12468] fix(lance): validate the identifier in ListNamespaces at schema
level (#12469)
### What changes were proposed in this pull request?
`GravitinoLanceNameSpaceOperations.listNamespaces` now validates the
identifier
in its two-level branch: it resolves the catalog with
`loadAndValidateLakehouseCatalog` and checks the schema with
`schemaExists`
before returning the empty list. The schema check shared with
`namespaceExists`
is extracted into a `validateSchemaExists` helper so both operations
raise the
same `NamespaceNotFoundException`.
Returning an empty list at level 2 stays correct — a schema has no child
namespaces, only tables — the missing part was the existence check.
### Why are the changes needed?
`ListNamespaces` returned HTTP 200 with `{"namespaces":[]}` for any
two-level
identifier, without checking that it referred to anything real. That
made a
child of a nonexistent parent look like it existed, while the parent
itself
correctly failed:
GET /lance/v1/namespace/BOGUS_CATALOG%24BOGUS_SCHEMA/list?delimiter=%24
{"namespaces":[]} <-- success
GET /lance/v1/namespace/BOGUS_CATALOG/list?delimiter=%24
{"error":"Catalog not found: BOGUS_CATALOG", ...} <-- correctly fails
`listNamespaces` was also the only namespace operation without this
check —
`describeNamespace`, `namespaceExists`, `dropNamespace` and
`createNamespace`
all validate at level 2. Clients that probe a configured namespace path
with
`ListNamespaces` (for example Apache Doris's Lance REST catalog, which
probes
`lance.namespace.parent` at catalog-creation time) silently accepted a
mistyped
schema and only failed much later on the first read.
`ListTables` was checked for the mirror-image gap and does not have one:
it
requires exactly two levels, and `ManagedTableOperations.listTables`
throws
`NoSuchSchemaException` for a nonexistent schema, which the exception
mapper
turns into a 404.
Fix: #12468
### Does this PR introduce _any_ user-facing change?
Yes. `ListNamespaces` on a two-level identifier whose catalog or schema
does not
exist now returns 404 `NAMESPACE_NOT_FOUND` instead of 200 with an empty
list.
Listing a schema that does exist still returns an empty list as before.
### How was this patch tested?
- New unit test `TestGravitinoLanceNameSpaceOperations` (3 cases:
existing
schema returns an empty list; nonexistent schema returns 404;
nonexistent
catalog returns 404 at both one and two levels). Reverting the main-code
change makes 2 of the 3 fail.
- Extended `LanceRESTServiceIT.testListNamespaces` to cover the same
three
cases end to end against a running Lance REST service; passes.
- `./gradlew :lance:lance-common:test :lance:lance-rest-server:test
-PskipITs`
passes.
https://claude.ai/code/session_012J3xbmJ3pHcMuJfG7Sonr6
---
.../GravitinoLanceNameSpaceOperations.java | 19 +++--
.../TestGravitinoLanceNameSpaceOperations.java | 94 ++++++++++++++++++++++
.../lance/integration/test/LanceRESTServiceIT.java | 24 ++++++
3 files changed, 132 insertions(+), 5 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 a5fae9ecd4..025cac747e 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
@@ -106,6 +106,12 @@ public class GravitinoLanceNameSpaceOperations implements
LanceNamespaceOperatio
break;
case 2:
+ // A schema has no child namespaces, only tables, so the result is
always empty. The
+ // identifier still has to be validated, otherwise a nonexistent path
would be reported
+ // as an existing but empty namespace.
+ Catalog schemaCatalog =
+
namespaceWrapper.loadAndValidateLakehouseCatalog(nsId.levelAtListPos(0));
+ validateSchemaExists(schemaCatalog, nsId.levelAtListPos(1));
namespaces = Lists.newArrayList();
break;
@@ -215,11 +221,14 @@ public class GravitinoLanceNameSpaceOperations implements
LanceNamespaceOperatio
Catalog catalog =
namespaceWrapper.loadAndValidateLakehouseCatalog(nsId.levelAtListPos(0));
if (nsId.levels() == 2) {
- String schemaName = nsId.levelAtListPos(1);
- if (!namespaceWrapper.schemaExists(catalog, schemaName)) {
- throw new NamespaceNotFoundException(
- "Schema not found: " + schemaName,
CommonUtil.formatCurrentStackTrace(), schemaName);
- }
+ validateSchemaExists(catalog, nsId.levelAtListPos(1));
+ }
+ }
+
+ private void validateSchemaExists(Catalog catalog, String schemaName) {
+ if (!namespaceWrapper.schemaExists(catalog, schemaName)) {
+ throw new NamespaceNotFoundException(
+ "Schema not found: " + schemaName,
CommonUtil.formatCurrentStackTrace(), schemaName);
}
}
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
new file mode 100644
index 0000000000..51629e4478
--- /dev/null
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceNameSpaceOperations.java
@@ -0,0 +1,94 @@
+/*
+ * 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.lance.common.ops.gravitino;
+
+import static org.mockito.Mockito.when;
+
+import java.util.regex.Pattern;
+import org.apache.gravitino.Catalog;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.lance.namespace.errors.NamespaceNotFoundException;
+import org.lance.namespace.model.ListNamespacesResponse;
+import org.mockito.Mockito;
+
+class TestGravitinoLanceNameSpaceOperations {
+
+ private static final String DELIMITER = Pattern.quote(".");
+
+ @Test
+ void testListNamespacesOnSchemaReturnsEmptyList() {
+ GravitinoLanceNamespaceWrapper namespaceWrapper =
+ Mockito.mock(GravitinoLanceNamespaceWrapper.class);
+ Catalog catalog = Mockito.mock(Catalog.class);
+
when(namespaceWrapper.loadAndValidateLakehouseCatalog("catalog")).thenReturn(catalog);
+ when(namespaceWrapper.schemaExists(catalog, "schema")).thenReturn(true);
+ GravitinoLanceNameSpaceOperations operations =
+ new GravitinoLanceNameSpaceOperations(namespaceWrapper);
+
+ ListNamespacesResponse response =
+ operations.listNamespaces("catalog.schema", DELIMITER, null, null);
+
+ Assertions.assertTrue(response.getNamespaces().isEmpty());
+ Assertions.assertNull(response.getPageToken());
+ }
+
+ @Test
+ void testListNamespacesOnNonExistentSchemaThrows() {
+ GravitinoLanceNamespaceWrapper namespaceWrapper =
+ Mockito.mock(GravitinoLanceNamespaceWrapper.class);
+ Catalog catalog = Mockito.mock(Catalog.class);
+
when(namespaceWrapper.loadAndValidateLakehouseCatalog("catalog")).thenReturn(catalog);
+ when(namespaceWrapper.schemaExists(catalog,
"bogus_schema")).thenReturn(false);
+ GravitinoLanceNameSpaceOperations operations =
+ new GravitinoLanceNameSpaceOperations(namespaceWrapper);
+
+ NamespaceNotFoundException exception =
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.listNamespaces("catalog.bogus_schema", DELIMITER,
null, null));
+ Assertions.assertEquals("Schema not found: bogus_schema",
exception.getMessage());
+ Assertions.assertEquals("bogus_schema", exception.getInstance());
+ }
+
+ @Test
+ void testListNamespacesOnNonExistentCatalogThrows() {
+ GravitinoLanceNamespaceWrapper namespaceWrapper =
+ Mockito.mock(GravitinoLanceNamespaceWrapper.class);
+ when(namespaceWrapper.loadAndValidateLakehouseCatalog("bogus_catalog"))
+ .thenThrow(
+ new NamespaceNotFoundException(
+ "Catalog not found: bogus_catalog", "", "bogus_catalog"));
+ GravitinoLanceNameSpaceOperations operations =
+ new GravitinoLanceNameSpaceOperations(namespaceWrapper);
+
+ // A nonexistent catalog must be reported at every depth, not only for its
own level.
+ NamespaceNotFoundException exception =
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.listNamespaces("bogus_catalog.bogus_schema",
DELIMITER, null, null));
+ Assertions.assertEquals("Catalog not found: bogus_catalog",
exception.getMessage());
+
+ exception =
+ Assertions.assertThrows(
+ NamespaceNotFoundException.class,
+ () -> operations.listNamespaces("bogus_catalog", DELIMITER, null,
null));
+ Assertions.assertEquals("Catalog not found: bogus_catalog",
exception.getMessage());
+ }
+}
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
index 1450d9813d..dcaa97c7f2 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
@@ -186,6 +186,30 @@ public class LanceRESTServiceIT extends BaseIT {
listNamespacesResp = ns.listNamespaces(listNamespacesReq);
Assertions.assertEquals(Sets.newHashSet(schema1.name()),
listNamespacesResp.getNamespaces());
+
+ // a schema has no child namespaces, only tables, so the result is empty
+ listNamespacesReq.addIdItem(schema1.name());
+ listNamespacesResp = ns.listNamespaces(listNamespacesReq);
+
+ Assertions.assertTrue(listNamespacesResp.getNamespaces().isEmpty());
+
+ // listing under a non-existent schema should fail instead of returning an
empty list
+ ListNamespacesRequest nonExistentSchemaReq = new ListNamespacesRequest();
+ nonExistentSchemaReq.addIdItem(catalog1.name());
+ nonExistentSchemaReq.addIdItem("non_existent_schema");
+ RuntimeException exception =
+ Assertions.assertThrows(
+ RuntimeException.class, () ->
ns.listNamespaces(nonExistentSchemaReq));
+ assertLanceErrorCode(exception, ErrorCode.NAMESPACE_NOT_FOUND);
+
+ // a non-existent catalog should be reported at every depth
+ ListNamespacesRequest nonExistentCatalogReq = new ListNamespacesRequest();
+ nonExistentCatalogReq.addIdItem("non_existent_catalog");
+ nonExistentCatalogReq.addIdItem("non_existent_schema");
+ exception =
+ Assertions.assertThrows(
+ RuntimeException.class, () ->
ns.listNamespaces(nonExistentCatalogReq));
+ assertLanceErrorCode(exception, ErrorCode.NAMESPACE_NOT_FOUND);
}
@Test
