Repository navigation
Hive: Write format-version to HMS table parameters - #18534
anuragmantri wants to merge 2 commits into
Conversation
|
@pvary @gaborkaszab - I would like to know your thoughts on this. |
Could you please explain the use-case a bit? |
| maxHiveTablePropertySize); | ||
| setStorageHandler(parameters, hiveEngineEnabled); | ||
| setMetadataHash(metadata, parameters); | ||
| parameters.put(TableProperties.FORMAT_VERSION, String.valueOf(metadata.formatVersion())); |
There was a problem hiding this comment.
Why don't do this in the setCommonParameters?
There was a problem hiding this comment.
setCommonParameters is also used for views, and a view has its own format version in ViewMetadata. Writing it there would put a view version under the same key as a table version. I kept it in updateHmsTableForIcebergTable so only tables get it. If you would rather have it in the shared method, I can pass the format version in and use a different key for views. What do you prefer?
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(ints = {1, 2, 3}) |
There was a problem hiding this comment.
Done. The test now uses TestHelpers#ALL_VERSIONS, so it also covers v4. That needed the table location as a file: URI, because v4 requires an absolute location.
anuragmantri
left a comment
There was a problem hiding this comment.
Thanks for review @pvary.
Could you please explain the use-case a bit?
A catalog service that implements the HMS Thrift API but has no credentials for the storage layer. That is common when the service is a shared control plane and the data plane is deliberately kept out of its trust boundary. The catalog would like to stop tables from being upgraded from V2 to V3 until every engine in the organization can read V3, because an older client fails on a V3 table.
With the REST catalog the server receives the upgrade-format-version update, so it can check it. Over Thrift the service only sees alter_table with a new metadata_location pointer. It cannot open that file, so it cannot tell the version before or after the call. With a format-version parameter it can compare the old and new values and reject the upgrade.
The metadata file stays the source of truth. The parameter is derived from it on every commit and is never read back into Iceberg properties, in the same way as the uuid parameter from #3914, which was added so that applications without storage access could identify a table.
| maxHiveTablePropertySize); | ||
| setStorageHandler(parameters, hiveEngineEnabled); | ||
| setMetadataHash(metadata, parameters); | ||
| parameters.put(TableProperties.FORMAT_VERSION, String.valueOf(metadata.formatVersion())); |
There was a problem hiding this comment.
setCommonParameters is also used for views, and a view has its own format version in ViewMetadata. Writing it there would put a view version under the same key as a table version. I kept it in updateHmsTableForIcebergTable so only tables get it. If you would rather have it in the shared method, I can pass the format version in and use a different key for views. What do you prefer?
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(ints = {1, 2, 3}) |
There was a problem hiding this comment.
Done. The test now uses TestHelpers#ALL_VERSIONS, so it also covers v4. That needed the table location as a file: URI, because v4 requires an absolute location.
Catalog services that read HMS over Thrift and without storage level access cannot see an Iceberg table's format version, because format-version is a reserved key kept only in metadata.json. Without it, these services cannot enforce format-level controls, such as blocking upgrades that break compatibility.
After this PR, format version is written as an HMS table parameter on every commit, so create and upgrade both update it. Like
uuid(#3914), it is a derived, one-way value that is never read back into Iceberg properties. Views are unchanged. Existing tables get the parameter on their next commit.Test plan: TestHiveCatalog checks the HMS parameter after create at all format versions, and after upgrading v2 to v3.
AI Disclosure