nevzheng opened a new issue, #13330:
URL: https://github.com/apache/gravitino/issues/13330
### What would you like to be improved?
Gravitino has no catalog setting for the Iceberg format version of new
tables, and its two Iceberg APIs pick it differently:
- **Gravitino API:** always 2
(`IcebergTablePropertiesMetadata.ICEBERG_DEFAULT_FORMAT_VERSION`); a catalog's
`table-default.format-version` is ignored.
- **Iceberg REST catalog (IRC):** the Iceberg library's default, or
`table-default.format-version` if set.
- **Allowed range:** fixed in code at 1–4.
So one catalog can create tables at different versions depending on the API,
and an operator can neither choose the version nor cap it.
### How should we improve?
Add two optional Iceberg catalog properties, applied by both APIs:
| Property | Meaning | When unset |
|---|---|---|
| `table-format-version.default` | Version of a new table that doesn't
request one | `2` (today) |
| `table-format-version.max` | Highest version a table may be created at or
upgraded to | `4` |
- **Default:** a new table that names no version gets the catalog default.
- **Max:** creating or upgrading above it fails with HTTP 400; existing
tables above it still load and commit.
- **Unchanged:** with both unset, behaviour is as today; federated IRC
catalogs (`rest` backend) forward requests untouched.
**Why it matters**
- **Control:** operators choose the version once for both APIs, e.g. v3 by
default for `variant` and deletion vectors, or a cap of 2 for older engines.
- **Predictability:** users get new tables at a version their engines read,
and a clear error instead of a table they can't.
- **Visibility:** the catalog properties show users, engine developers and
tools which version to expect.
I have a proposed implementation (appendices A and B: changes and
configuration) and can open PRs if the direction and names look right.
Related: #4844, #10986, #11954
---
#### Appendix A: proposed changes
Three PRs, reviewed in order. PR 3 is optional: without it, invalid values
are refused when the catalog loads instead of when it is created or altered.
**PR 1: Gravitino API** (modules catalog-common, catalog-lakehouse-iceberg)
- IcebergConstants.java: add the two property names, their fallbacks (2 and
4) and the supported versions, so the IRC shares one source.
- IcebergPropertiesUtils.java: add helpers to parse a version, resolve the
default and max, check a requested version against the max, and validate a
catalog's properties together.
- IcebergCatalogPropertiesMetadata.java: add property entries for both
properties.
- IcebergCatalogOperations.java, IcebergTable.java: stamp the catalog
default instead of the hard-coded 2, and refuse a requested version above the
max. format-version is immutable after create, so there is no upgrade to cap.
- IcebergTablePropertiesMetadata.java: point its existing constants at the
shared ones and update the format-version description.
- lakehouse-iceberg-catalog.md: document both properties.
- Tests: TestIcebergPropertiesUtils, TestIcebergCatalogPropertiesMetadata,
TestIcebergTable.
**PR 2: IRC** (modules iceberg-common, iceberg-rest-server)
- IcebergConfig.java: read and validate both properties (static IRC keys
gravitino.iceberg-rest.table-format-version.*), and map the default to
Iceberg's table-default.format-version.
- CatalogWrapperForREST.java: set the default on a stage-create that names
no version, because Iceberg builds staged metadata without table-default.*
properties; refuse a create, stage-create or upgrade-format-version commit
above the max.
- IcebergCatalogWrapperManager.java, StaticIcebergConfigProvider.java: skip
validation and the default for federated (rest backend) catalogs.
- iceberg-rest-service.md, gravitino-iceberg-rest-server.conf.template:
document the keys and add commented examples (Appendix B).
- Tests: TestIcebergConfig, TestCatalogWrapperForREST,
TestIcebergCatalogWrapperManagerForREST, TestStaticIcebergConfigProvider,
TestDynamicIcebergConfigProvider.
**PR 3: validate at catalog create and alter** (module core)
- PropertiesMetadata.java: add a no-op default validateProperties(Map) for
cross-property checks.
- PropertiesMetadataHelpers.java: call it on create, and on the post-alter
properties through a new overload of validatePropertyForAlter.
- CatalogManager.java: pass the stored catalog properties on its alter paths.
- IcebergCatalogPropertiesMetadata.java: override the hook with the PR 1
validation, so a bad alter is refused and the stored catalog is unchanged.
- Tests: TestPropertiesMetadataHelpers, TestCatalogManager, TestCatalog,
TestIcebergCatalogPropertiesMetadata.
**Validation:** each value is 1–4; `default` ≤ `max`; an explicit
`table-default.format-version` (plain or `gravitino.bypass.`) must equal
`default`.
```mermaid
flowchart TD
req["create or stage-create"] --> fed{"federated IRC catalog?"}
fed -->|yes| pass["forwarded unchanged"]
fed -->|no| asked{"format-version requested?"}
asked -->|"above max"| deny["HTTP 400"]
asked -->|"up to max"| keep["requested version"]
asked -->|no| dflt["table-format-version.default"]
```
**Tests:** unit tests for validation; catalog create and alter, including a
refused alter; the default and cap on both APIs, including stage-create and
upgrade; existing tables above the max; static and dynamic IRC configuration;
federated catalogs untouched.
#### Appendix B: configuration
No existing setting changes; the new keys are optional.
| Where | Key | Scope |
|---|---|---|
| Gravitino catalog properties (Gravitino API, dynamic IRC catalogs) |
`table-format-version.default`, `table-format-version.max` | per catalog;
alterable |
| `conf/gravitino-iceberg-rest-server.conf` (standalone IRC) or
`conf/gravitino.conf` (auxiliary IRC) |
`gravitino.iceberg-rest.table-format-version.default`,
`gravitino.iceberg-rest.table-format-version.max` | static IRC catalog; checked
at startup |
Proposed: add both keys, commented out, to
`conf/gravitino-iceberg-rest-server.conf.template` as examples:
```properties
# Format version of a new table that doesn't request one (default 2), and
the highest allowed (default 4).
# gravitino.iceberg-rest.table-format-version.default = 3
# gravitino.iceberg-rest.table-format-version.max = 3
```
A catalog-level `table-default.format-version`, which the IRC already passes
to Iceberg, keeps working but must equal `table-format-version.default` when
both are set.
--
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]
