Planet-X opened a new issue, #17538:
URL: https://github.com/apache/iceberg/issues/17538
### 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](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java#L526-L531)):
```java
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](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1442-L1454)).
`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()`](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L553)
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.
```java
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](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1449-L1450)
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
- [x] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
