FANNG1 opened a new pull request, #13684:
URL: https://github.com/apache/gravitino/pull/13684
### What changes were proposed in this pull request?
`CommonUtil.parseEnumToken` no longer resolves a mode token with
`Enum.valueOf` on the upper-cased string. It scans the enum constants instead
and matches a token against the constant name *or* that name with its
underscores removed, so `ExistOk` resolves to `EXIST_OK` just as `exist_ok`
does. The one change covers all five call sites: `CreateNamespace` mode,
`DropNamespace` mode and behavior, `CreateTable` mode and `RegisterTable` mode.
`normalizeToken` and `LanceMetadataAuthorizationMethodInterceptor` are
deliberately left untouched. The interceptor decides whether a create request
is really an overwrite — and therefore needs modify/ownership instead of a
create privilege — by comparing `normalizeToken(mode)` against the literal
`OVERWRITE`. Splitting camelCase inside `normalizeToken` would have turned
`OverWrite` into `OVER_WRITE` and regressed a spelling that works today;
stripping underscores on both sides would have made the operation accept
`over_write` as `OVERWRITE` while the interceptor did not, which is a privilege
escalation. Matching "name, or name without underscores" is exactly as
permissive as the interceptor's comparison for the single-word `OVERWRITE`, so
the two sides stay in agreement.
`docs/lance-rest-service.md` now states that both spellings are accepted.
### Why are the changes needed?
The lance-namespace spec declares these `mode` fields as "Case insensitive,
supports both PascalCase and snake_case", with values `Create` / `ExistOk` /
`Overwrite`, but `ExistOk` was rejected: `normalizeToken` only trims and
upper-cases, so the token became `EXISTOK` and never matched the `EXIST_OK`
constant. `ExistOk` is the only multi-word value in the whole mode/behavior
vocabulary, so it was the only one affected, and a client following the spec's
own spelling could not create a namespace or table idempotently:
```
lance_namespace.errors.InvalidInputError: Invalid input: Unknown create
namespace mode: ExistOk
```
Upstream `lancedb/lance` accepts both spellings
(`eq_ignore_ascii_case("existok") || eq_ignore_ascii_case("exist_ok")`).
Fix: #13680
### Does this PR introduce _any_ user-facing change?
Yes. `ExistOk` — and any other underscore-free spelling of a mode value,
such as `existOk` — is now accepted wherever `exist_ok` already was, on
`CreateNamespace` and `CreateTable`. No API or property-key changes; values
that were rejected before are still rejected with the same message.
### How was this patch tested?
- `TestGravitinoLanceModeParsing`: new
`testParseEnumTokenAcceptsPascalCaseAndSnakeCase`, plus
`testParseEnumTokenRejectsSpellingsThatAreNotTheConstant`, which pins down that
an underscore inserted into a single-word constant stays rejected — the
invariant the authorization path depends on.
`testCreateTableModeNormalizesCase` now also sends `" ExistOk "` and asserts
the stored `lance.creation-mode` is still the canonical `EXIST_OK`, since the
lakehouse-generic catalog reads it with a plain `CreationMode.valueOf`.
- `LanceRESTServiceIT`: `ExistOk` exercised over HTTP for catalog-level
create, schema-level create and create-table. 15/15 pass in embedded mode.
- 111 lance unit tests pass, including the 26 authorization-interceptor
tests unchanged.
- Confirmed the new coverage catches the bug: reverting only
`CommonUtil.java` makes `testCreateNamespace` and `testCreateTable` fail, and
they pass again with the fix restored.
--
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]
