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.
…talog create and alter Add a PropertiesMetadata.validateProperties hook for rules that relate several properties, and call it from PropertiesMetadataHelpers at catalog create and, with the change applied to the stored properties, at catalog alter. The Iceberg catalog uses it so an invalid table-format-version combination is refused when the catalog is created or altered instead of when it loads.
Code Coverage Report
Files
|
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 and #13332: please review only the last commit.
What changes were proposed in this pull request?
Add a
PropertiesMetadata.validateProperties(Map)hook for rules that relate several properties, which a single property entry's decoder cannot check. The default does nothing.PropertiesMetadataHelperscalls it at catalog create with the requested properties, and at catalog alter with the stored properties plus the change.CatalogManagerpasses the stored properties to the alter check.The Iceberg catalog implements the hook for the table-format-version properties from #13331. A
table-format-version.defaultabovetable-format-version.max, or a conflictingtable-default.format-version, now fails the create or alter request, and the catalog keeps its properties. The load-time check from #13331 stays for catalogs stored earlier: they load, but every schema and table operation fails with HTTP 400 until an alter fixes them.Why are the changes needed?
Without this, an invalid combination is accepted, stored, and only fails on the catalog's first operation, which leaves a broken catalog behind. Checking at create and alter makes the misconfiguration visible to the operator who made it.
Fix: #13330
Does this PR introduce any user-facing change?
docs/lakehouse-iceberg-catalog.mdis updated.PropertiesMetadata.validateProperties, for connector authors. Existing implementations are unaffected.How was this patch tested?
Unit tests only. This PR covers the last row of the format-version coverage (the others are in #13331 and #13332):
table-default.format-version, at catalog create and alterNew unit tests in
TestPropertiesMetadataHelpersandTestCatalogManagercover the hook at create and at alter with the change applied.TestIcebergCatalogPropertiesMetadatacovers the Iceberg rules../gradlew :core:test :catalogs:catalog-common:test :catalogs:catalog-lakehouse-iceberg:test :iceberg:iceberg-common:test :iceberg:iceberg-rest-server:test -PskipITs: 2960 tests, 0 failures, 0 errors, 2 skipped (core 2249 with 2 skipped, catalog-common 40, catalog-lakehouse-iceberg 125, iceberg-common 83, iceberg-rest-server 463).