Skip to content

Hive: Write format-version to HMS table parameters - #18534

Open
anuragmantri wants to merge 2 commits into
apache:mainfrom
anuragmantri:iceberg-table-format-3da9f787
Open

anuragmantri wants to merge 2 commits into
apache:mainfrom
anuragmantri:iceberg-table-format-3da9f787

Conversation

@anuragmantri

@anuragmantri anuragmantri commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • Model: Claude Opus 5.5
  • Platform/Tool: Claude Code
  • Human Oversight: Fully reviewed by me.
  • Prompt Summary: Analyze why the Iceberg Hive catalog doesn't write the format version to HMS parameters, and add it with tests.

@github-actions github-actions Bot added the hive label Oct 16, 2026
@anuragmantri

Copy link
Copy Markdown
Collaborator Author

@pvary @gaborkaszab - I would like to know your thoughts on this.

@pvary

pvary commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Without it, these services cannot enforce format-level controls, such as blocking upgrades that break compatibility.

Could you please explain the use-case a bit?
Shouldn't the catalog read the original table metadata on upgrade? Based on that, shouldn't it be able to decide if an upgrade is possible, or not?

maxHiveTablePropertySize);
setStorageHandler(parameters, hiveEngineEnabled);
setMetadataHash(metadata, parameters);
parameters.put(TableProperties.FORMAT_VERSION, String.valueOf(metadata.formatVersion()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't do this in the setCommonParameters?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestHelpers#ALL_VERSIONS?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 anuragmantri left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()));

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants