FANNG1 opened a new issue, #13680:
URL: https://github.com/apache/gravitino/issues/13680
### Version
1.3.0, and `main` — `CommonUtil.normalizeToken` is unchanged on
`origin/main`.
### Describe what's wrong
The Lance REST server rejects the `ExistOk` mode value that the
lance-namespace spec documents, on both `CreateNamespace` and `CreateTable`.
The spec declares these `mode` fields as "Case insensitive, supports both
PascalCase and snake_case", with valid values `Create` / `ExistOk` /
`Overwrite`. `CommonUtil.normalizeToken` only trims and upper-cases:
```java
public static String normalizeToken(String value) {
return value == null ? "" : value.trim().toUpperCase(Locale.ROOT);
}
```
so `ExistOk` becomes `EXISTOK`, which does not match the `EXIST_OK` enum
constant that `parseEnumToken` resolves against:
```java
static <E extends Enum<E>> E parseEnumToken(
Class<E> enumClass, String value, String errorMessagePrefix, String
instance) {
try {
return Enum.valueOf(enumClass, normalizeToken(value));
} catch (IllegalArgumentException e) {
throw new InvalidInputException(
errorMessagePrefix + value, formatCurrentStackTrace(), instance);
}
}
```
snake_case works (`exist_ok` → `EXIST_OK`), so the "supports both" promise
holds for exactly one of the two spellings.
`ExistOk` is the only multi-word constant in the spec's mode/behavior
vocabulary, which is why nothing else is affected:
| Request field | Spec values | Status |
| --- | --- | --- |
| `CreateNamespaceRequest.mode` | Create / **ExistOk** / Overwrite |
`ExistOk` rejected |
| `CreateTableRequest.mode` | Create / **ExistOk** / Overwrite | `ExistOk`
rejected |
| `RegisterTableRequest.mode` | Create / Overwrite | ok |
| `DropNamespaceRequest.mode` | Fail / Skip | ok |
| `DropNamespaceRequest.behavior` | Restrict / Cascade | ok |
The five `parseEnumToken` call sites are
`GravitinoLanceNameSpaceOperations.java:449,457,465` and
`GravitinoLanceTableOperations.java:444,452`.
For reference, the `dir` namespace implementation in lancedb/lance parses
the same field with `eq_ignore_ascii_case("existok") ||
eq_ignore_ascii_case("exist_ok")`
(`rust/lance-namespace-impls/src/dir/manifest.rs`), i.e. it accepts both
spellings.
### Error message and/or stacktrace
```
lance_namespace.errors.InvalidInputError: Invalid input: Unknown create
namespace mode: ExistOk
```
Server side (`includeErrorStackTrace = true`):
```
java.lang.RuntimeException: Captured stacktrace
at
org.apache.gravitino.lance.common.ops.gravitino.CommonUtil.formatCurrentStackTrace(CommonUtil.java:31)
at
org.apache.gravitino.lance.common.ops.gravitino.CommonUtil.parseEnumToken(CommonUtil.java:54)
at
org.apache.gravitino.lance.common.ops.gravitino.GravitinoLanceNameSpaceOperations.parseCreateMode(GravitinoLanceNameSpaceOperations.java:449)
at
org.apache.gravitino.lance.service.rest.LanceNamespaceOperations.createNamespace(LanceNamespaceOperations.java:130)
```
The table path produces the same thing with `Unknown create table mode:
ExistOk`.
### How to reproduce
Gravitino 1.3.0 with the `lance-rest` auxiliary service on port 9101 and a
`lakehouse-generic` catalog named `lance_catalog`:
```properties
gravitino.auxService.names = iceberg-rest,lance-rest
gravitino.lance-rest.classpath = lance-rest-server/libs
gravitino.lance-rest.httpPort = 9101
gravitino.lance-rest.namespace-backend = gravitino
gravitino.lance-rest.gravitino-uri = http://127.0.0.1:8090
gravitino.lance-rest.gravitino-metalake = test
```
```python
# pip install lance-namespace==0.8.6 pyarrow
import io, uuid, pyarrow as pa, lance_namespace as ln
ns = ln.connect("rest", {"uri": "http://127.0.0.1:9101/lance"})
nsid = ["lance_catalog", f"modeprobe_{uuid.uuid4().hex[:6]}"]
def probe(label, fn):
try:
fn()
print(f" {label:36} -> OK")
except Exception as e:
print(f" {label:36} -> {type(e).__name__}:
{str(e).splitlines()[0]}")
ns.create_namespace(ln.CreateNamespaceRequest(id=nsid, mode="EXIST_OK"))
print("CreateNamespace, namespace already exists:")
for mode in ["ExistOk", "exist_ok", "EXIST_OK", "Overwrite"]:
probe(f"mode={mode!r}", lambda m=mode:
ns.create_namespace(ln.CreateNamespaceRequest(id=nsid, mode=m)))
sink = io.BytesIO()
with pa.ipc.new_stream(sink, pa.schema([pa.field("id", pa.int64())])):
pass
schema_ipc = sink.getvalue()
tid = nsid + ["t1"]
ns.create_table(ln.CreateTableRequest(id=tid), schema_ipc)
print("CreateTable, table already exists:")
for mode in ["ExistOk", "exist_ok", "EXIST_OK"]:
probe(f"mode={mode!r}", lambda m=mode:
ns.create_table(ln.CreateTableRequest(id=tid, mode=m), schema_ipc))
ns.drop_namespace(ln.DropNamespaceRequest(id=nsid, mode="Skip",
behavior="Cascade"))
```
Observed:
```
CreateNamespace, namespace already exists:
mode='ExistOk' -> InvalidInputError: Invalid input:
Unknown create namespace mode: ExistOk
mode='exist_ok' -> OK
mode='EXIST_OK' -> OK
mode='Overwrite' -> OK
CreateTable, table already exists:
mode='ExistOk' -> InvalidInputError: Invalid input:
Unknown create table mode: ExistOk
mode='exist_ok' -> OK
mode='EXIST_OK' -> OK
```
Expected: `ExistOk` is accepted wherever `exist_ok` is.
### Additional context
Found while building a Daft × Gravitino Lance REST × MinIO end-to-end suite.
A client that follows the spec's own example spelling cannot create a namespace
or a table idempotently and has to discover the snake_case spelling by trial.
`TestGravitinoLanceModeParsing` already covers the snake_case half:
```java
Assertions.assertEquals("EXIST_OK", CommonUtil.normalizeToken("exist_ok"));
Assertions.assertEquals(
TestMode.EXIST_OK,
CommonUtil.parseEnumToken(TestMode.class, "exist_ok", "Unknown mode: ",
"table"));
```
There is no corresponding PascalCase assertion, which is presumably why this
went unnoticed.
`normalizeToken` has one other caller,
`LanceMetadataAuthorizationMethodInterceptor.java:223`, which compares against
`OVERWRITE`. That token is single-word, so it is unaffected.
--
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]
