Jeffail opened a new pull request, #1030: URL: https://github.com/apache/iceberg-go/pull/1030
## Problem `ManifestListWriter.AddManifests` requires every input manifest file's version to match the writer's version exactly, which contradicts the Iceberg spec: a v2 manifest list must be able to reference v1 manifest files so that v1 tables can be upgraded without rewriting historical manifests. Java's [`ManifestListWriter`](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/ManifestListWriter.java) handles this; `iceberg-go` does not. The practical symptom for downstream users is that any commit to a table whose metadata has been upgraded from v1 to v2 — whether via `Transaction.UpgradeFormatVersion` or out-of-band by another engine — fails with `invalid argument: ManifestListWriter only supports version 2 manifest files`. The v1 manifest files written before the upgrade are never rewritten and continue to surface via `snapshotProducer.existingManifests()` on every subsequent commit, so the failure is permanent. ## Fix `manifest.go:1410-1414`: replace the `file.Version() != m.version` exact-match gate with `file.Version() > m.version`. Newer-than-writer inputs are still rejected — the v2 entry schema has no place for v3 fields such as `first_row_id`, and accepting them would silently drop data. Lower-than-writer inputs are accepted because the in-memory `manifestFile` produced from a v1 input (via `manifestFileV1.toFile()` or `NewManifestFile(1, ...)`) already carries the spec's inheritance values — `Content = data` and `SeqNumber = MinSeqNumber = 0` — so it can be encoded directly against the v2/v3 entry schema. The existing `first_row_id` assignment for data manifests with `FirstRowIDValue == nil` covers v1 inputs in v3 lists without further changes. ## Testing - `TestV2ManifestListAcceptsV1Manifests` — round-trips a v1 manifest through a v2 manifest list and asserts that the decoded entry has `content = data` and `sequence_number = min_sequence_number = 0` per spec inheritance. - `TestV3ManifestListAcceptsV1AndV2Manifests` — writes a v1 data manifest and a v2 delete manifest into a v3 list; asserts inheritance for the v1 entry, asserts that `first_row_id` is assigned to the v1 data manifest and is left unset on the v2 delete manifest (data-only assignment per the existing v3 writer rules). - `TestV2ManifestListRejectsV3Manifests` — confirms the downgrade direction is still blocked. - `TestWriteManifestListClosesWriterOnError` — existing test updated to drive its `AddManifests` failure path through a v3-in-v2 input (still rejected) instead of v1-in-v2. Confirmed failing on `main` before the fix and passing after. `go test ./...` passes. Fixes #1029 -- 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]
