Core: Discard changes when suppressing historical snapshots in CatalogHandlers

[Planet-X]

Apache Iceberg version

1.11.0 (latest release)

Query engine

None

Please describe the bug 🐞

Behavior

In snapshot-loading-mode=refs, CatalogHandlers.loadTable fails with IllegalArgumentException: Cannot set metadata location with changes to table metadata: 1 changes when serving GET .../tables/{table}?snapshots=refs for a table that has a statistics file (or partition statistics file) attached to a historical (unreferenced) snapshot.

Cause

The REFS branch builds the response metadata like this (CatalogHandlers.java#L526-L531):

metadata =
    TableMetadata.buildFrom(loadedMetadata)
        .withMetadataLocation(loadedMetadata.metadataFileLocation())
        .suppressHistoricalSnapshots()
        .build();

suppressHistoricalSnapshots() does not record RemoveSnapshots changes, but it removes the suppressed snapshots' statistics via removeStatistics(...) / removePartitionStatistics(...), which do record MetadataUpdate.RemoveStatistics / RemovePartitionStatistics changes (TableMetadata.java, rewriteSnapshotsInternal). build() then rejects the combination of pending changes and a set metadata location.

The client-side equivalent in RESTSessionCatalog.loadTable already prevents this with by calling .discardChanges() when rebuilding metadata; CatalogHandlers performs the change-recording suppression without that measure.

To Reproduce

E.g. via RESTCatalogAdapter with snapshot-loading-mode=refs on the client:

1. Create a table, commit snapshot A, attach a StatisticsFile to A (e.g. UpdateStatistics).
2. Commit snapshot B so A becomes historical (only reachable via parent chain, not via refs).
3. loadTable with snapshots=refs → 500 / IllegalArgumentException: Cannot set metadata location with changes to table metadata: 1 changes.

Encountered in practice testing Trino's REST catalog against the in-memory test server with snapshot-loading-mode=refs: Trino writes statistics on INSERT by default, so the first loadTable after a stats-bearing snapshot becomes historical reliably fails.

Suggested Fix

Add .discardChanges() to the builder chain in CatalogHandlers.loadTable (matching the client-side wrapper), e.g.

TableMetadata.buildFrom(loadedMetadata)
    .withMetadataLocation(loadedMetadata.metadataFileLocation())
    .suppressHistoricalSnapshots()
    .discardChanges()
    .build();

This would be the minimal fix, keeping the general behavior as is. Alternatively, the suppress-parameter that already prevents MetadataUpdate.RemoveSnapshots changes from being created here could be incorporated into removeStatistics() and removePartitionStatistics() to prevent change creation there at the root.

AI assistance

Found this while working on a trino patch. This root-cause analysis was AI-assisted. I'm quite confident this is an actual bug and checked this in detail before opening the issue. I'll link to the related PR at trino shortly, whose test triggers this behavior.

Willingness to contribute

• I can contribute a fix for this bug independently
• I would be willing to contribute a fix for this bug with guidance from the Iceberg community
• I cannot contribute a fix for this bug at this time

[Planet-X]

Related trino PR that triggered the issue can be found here: trinodb/trino#30612
This is the test that triggers the issue, where it is mitigated by not writing statistics: https://github.com/Planet-X/trino/blob/1602f911f9a7db96c95474aaf6228483c4a0ecbb/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestIcebergRestCatalogSnapshotLoadingMode.java#L72-L74
It fails without the iceberg.extended-statistics.collect-on-write-property.