nevzheng opened a new pull request, #13332:
URL: https://github.com/apache/gravitino/pull/13332
Stacked on #13331: please review only the last commit.
### What changes were proposed in this pull request?
Apply `table-format-version.default` and `table-format-version.max` in the
Iceberg REST service, with the same names, rules and messages as the Gravitino
API in #13331:
- A create or stage-create without `format-version` gets the catalog
default. Iceberg does not apply `table-default.` properties to staged metadata,
so the service sets it on the stage-create request.
- A create, stage-create, `upgrade-format-version` or staged-create commit
above the maximum fails with HTTP 400. Above `4`, the build ceiling from
#13331, it fails with the ceiling message even when the maximum is unset.
Existing tables above the maximum still load and commit.
- Static catalogs (`gravitino.iceberg-rest.table-format-version.*`) are
validated at startup. Dynamic catalogs use the catalog properties and are
validated when they load.
- Catalogs with the `rest` backend forward requests unchanged, so the
properties are not read, validated or applied:
`IcebergConfig.governsTableFormatVersions()` keeps `CatalogWrapperForREST` from
parsing them, and a catalog with `table-format-version.max=abc` loads and
forwards.
### Why are the changes needed?
Without this, the Iceberg REST service ignores the catalog's format-version
settings, so a client could create or upgrade a table past the version the
operator allows.
Part of #13330
### Does this PR introduce _any_ user-facing change?
Yes. The Iceberg REST service honours the two catalog properties and the
build ceiling, and adds the static keys
`gravitino.iceberg-rest.table-format-version.default` and
`gravitino.iceberg-rest.table-format-version.max`. They are documented in
`docs/iceberg-rest-service.md` and the conf template;
`docs/lakehouse-iceberg-catalog.md` gains the upgrade limit. A catalog that
sets neither behaves as before, except that a version above `4` is refused with
the ceiling message.
### How was this patch tested?
Unit tests only. Coverage for the Iceberg REST service, in
`TestCatalogWrapperForREST` unless noted ("n/a" means the case doesn't arise on
that path):
| Case | Create | Stage-create | Upgrade | Staged commit | `rest` backend |
|---|---|---|---|---|---|
| No version, both unset | 2 | 2 | n/a | n/a | n/a |
| No version, default=3 | 3 | 3 | n/a | n/a | not applied |
| Requested up to max | kept | kept | kept | kept | n/a |
| Requested above a set max | 400, max message | 400 | 400 | 400 | not
capped |
| Requested 5, max unset | 400, ceiling message | 400 | 400, stays v4 | 400
| n/a |
| Existing table above max | n/a | n/a | loads and commits | n/a | n/a |
| Bad property value (0, 5, abc, blank) | catalog refused | catalog refused
| n/a | n/a | ignored (`TestIcebergCatalogWrapperManagerForREST`) |
| Default above max, or conflicting `table-default.format-version`, at load
| catalog refused | n/a | n/a | n/a | ignored |
| Static config bad value | startup fails
(`TestStaticIcebergConfigProvider`) | | | | skipped |
`testAboveBuildCeilingIsRefusedWithMaxUnset` is one parameterized test over
the four paths; it also checks that nothing is created and the upgraded table
stays at v4. `TestIcebergConfig` and `TestDynamicIcebergConfigProvider` cover
the config side.
`./gradlew :catalogs:catalog-common:test
:catalogs:catalog-lakehouse-iceberg:test :iceberg:iceberg-common:test
:iceberg:iceberg-rest-server:test -PskipITs`: 710 tests, 0 failures, 0 errors,
0 skipped (catalog-common 40, catalog-lakehouse-iceberg 124, iceberg-common 83,
iceberg-rest-server 463).
--
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]
