Conversation
Pulls in apache/iceberg-go#1030, which fixes the upstream ManifestListWriter rejecting v1 manifest files when writing a v2 manifest list. Without this, every commit on a table that was upgraded from v1 to v2 (whether via Transaction.UpgradeFormatVersion in the committer or out-of-band by another engine) failed with "ManifestListWriter only supports version 2 manifest files" because the historical v1 manifests surfaced through existingManifests() on every commit and the v2 writer rejected them. Adds a regression test that pins the upstream behaviour: a v1 ManifestFile passed to WriteManifestList(2, ...) must round-trip and the decoded entry must inherit content=data and sequence_number=min_sequence_number=0 per the Iceberg spec.
|
Commits Review LGTM |
Co-authored-by: Joseph Woodward <joseph.woodward@redpanda.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // out-of-band) explodes when existingManifests() surfaces the historical | ||
| // v1 manifests. If this test starts failing after a future bump, the bug | ||
| // has regressed upstream. | ||
| func TestCON459_V2ManifestListAcceptsV1Manifest(t *testing.T) { |
There was a problem hiding this comment.
Test function names should use camelCase without underscores per the project's test patterns ("Test function names use camelCase, not underscores. Write TestMyProcessorBadArgs, not TestMyProcessor_BadArgs."). Consider renaming to TestCON459V2ManifestListAcceptsV1Manifest.
|
Commits
Review
|
2 participants
Summary
github.com/apache/iceberg-goto the merge commit of apache/iceberg-go#1030, which relaxesManifestListWriter.AddManifestsso a v2 manifest list may reference v1 manifest files (and a v3 list may reference v1/v2 manifests), per the Iceberg spec's mixed-version inheritance rules.Transaction.UpgradeFormatVersionininternal/impl/iceberg/committer.goor out-of-band by another engine such as Spark or Hive — failed permanently withinvalid argument: ManifestListWriter only supports version 2 manifest files, because historical v1 manifests resurfaced throughexistingManifests()on every commit.TestCON459_V2ManifestListAcceptsV1Manifest) that constructs a v1ManifestFilevia the publiciceberg-goAPI, writes it throughWriteManifestList(2, ...), and asserts that the decoded entry inheritscontent=dataandsequence_number=min_sequence_number=0. If the upstream fix ever regresses, this test fails.Resolves CON-459.
Test plan
task fmttask lint— 0 issuestask test— full unit + template suite passes, including the new regression test