Conversation
…x catalog properties Add the table-format-version.default and table-format-version.max catalog properties so operators can choose the format version of new tables that do not request one and cap the version tables may be created at. Both are validated as supported Iceberg versions (1-4), default to 2 and 4, and a conflicting table-default.format-version fails catalog initialization. A format-version above 4, the highest this Gravitino build accepts, is now refused as unsupported by this Gravitino (supports 1-4) rather than as exceeding an unset table-format-version.max. The Gravitino API's table property validation reports the same message as its cause. A guard test fails when the ceiling and the bundled Iceberg's TableMetadata.SUPPORTED_TABLE_FORMAT_VERSION differ, so an Iceberg upgrade forces a review of the ceiling. Pin what a stored catalog with table-format-version.default above .max does: it loads as metadata, and its first schema or table operation fails with an IllegalArgumentException (HTTP 400) naming both properties. Docs: drop the upgrade clause from this layer (the Iceberg REST catalog adds it) and state the ceiling and the load-time behaviour.
…n the Iceberg REST catalog Apply table-format-version.default and table-format-version.max in the Iceberg REST service: a stage-create without format-version gets the catalog default, and create, stage-create and upgrade-format-version commits above the maximum fail with 400. Static catalogs are validated at startup, and catalogs with the rest backend are exempt because they forward requests unchanged. A federated catalog no longer reads table-format-version.max at all, so a value that would not parse (for example max=abc) neither stops it from loading nor keeps a request from reaching the remote catalog. Cover the build's ceiling on the Iceberg REST catalog: with the maximum unset, format-version 5 on create, stage-create, upgrade-format-version and a staged-create commit is refused with HTTP 400 and the ceiling message, and nothing is created or changed. Docs: add the upgrade clause to the catalog doc now that the Iceberg REST service enforces it, and state the ceiling in the REST service doc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #13331: please review only the last commit.
What changes were proposed in this pull request?
Apply
table-format-version.defaultandtable-format-version.maxin the Iceberg REST service, with the same names, rules and messages as the Gravitino API in #13331:format-versiongets the catalog default. Iceberg does not applytable-default.properties to staged metadata, so the service sets it on the stage-create request.upgrade-format-versionor staged-create commit above the maximum fails with HTTP 400. Above4, the build ceiling from [#13330] feat(iceberg): add table-format-version default and max catalog properties #13331, it fails with the ceiling message even when the maximum is unset. Existing tables above the maximum still load and commit.gravitino.iceberg-rest.table-format-version.*) are validated at startup. Dynamic catalogs use the catalog properties and are validated when they load.restbackend forward requests unchanged, so the properties are not read, validated or applied:IcebergConfig.governsTableFormatVersions()keepsCatalogWrapperForRESTfrom parsing them, and a catalog withtable-format-version.max=abcloads 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.defaultandgravitino.iceberg-rest.table-format-version.max. They are documented indocs/iceberg-rest-service.mdand the conf template;docs/lakehouse-iceberg-catalog.mdgains the upgrade limit. A catalog that sets neither behaves as before, except that a version above4is refused with the ceiling message.How was this patch tested?
Unit tests only. Coverage for the Iceberg REST service, in
TestCatalogWrapperForRESTunless noted ("n/a" means the case doesn't arise on that path):restbackendTestIcebergCatalogWrapperManagerForREST)table-default.format-version, at loadTestStaticIcebergConfigProvider)testAboveBuildCeilingIsRefusedWithMaxUnsetis one parameterized test over the four paths; it also checks that nothing is created and the upgraded table stays at v4.TestIcebergConfigandTestDynamicIcebergConfigProvidercover 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).