ryanworl opened a new pull request, #1818: URL: https://github.com/apache/iceberg-go/pull/1818
## What `Transaction.AddDataFiles` (and the replace paths sharing its validation) rejected any `DataFile` whose `SpecID()` was not the current default spec — even though the snapshot producer already groups added files by spec id and writes one manifest per spec. This PR makes that path reachable and safe: - `validateDataFilesToAdd` now resolves each file's spec from the table metadata and validates the partition tuple against *that* spec. Unregistered spec ids are still rejected (`unregistered partition spec id %d`). - `writeAddedManifest` resolved each group's spec via the `sp.spec()` helper, which silently substitutes an **empty** spec on lookup failure — previously unreachable, but it would have written a manifest whose declared spec disagrees with its entries' partition tuples. It now errors instead, and spec groups are written in sorted order for deterministic output. ## Why Manifests declare their own `partition-spec-id` and readers plan per-manifest with that spec; the default spec is a writer convention for new data, not a format constraint. Enforcing it as a constraint strands data after a partition evolution: a compactor holding correctly-encoded old-spec files cannot consolidate them even among themselves, and a rewrite migrating files between specs cannot express its adds. The delete side of the Go producer already groups entries per spec — this brings the add side in line. ## Java analogue `MergingSnapshotProducer.add(DataFile)` has always resolved the file's spec via `spec(file.specId())` and routed it to a per-spec `ManifestWriter`, producing one manifest per spec per commit, with unknown spec ids failing on lookup. This PR matches that behavior; nothing in Java requires added files to use the default spec. ## Testing New `TestAddDataFilesMultipleSpecs` in `TableWritingTestSuite` (runs v1/v2/v3): a mixed-spec commit produces one manifest per spec with spec ids and partition tuples round-tripping; real parquet files scan back fully; `PlanFiles` prunes each manifest via its own spec; the snapshot summary renders partition paths with each file's own spec; v3 first-row-id accounting is unaffected by the grouping; unregistered spec ids are rejected. The existing spec-id validation test keeps its scenario (spec id 999) with the new error message. `AddFiles` (path-based) is intentionally unchanged — it still infers partition values against the current spec only. Made with [Cursor](https://cursor.com) -- 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]
