feat(table): allow added data files to target any registered partition spec

[ryanworl]

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

[zeroshade]

The core change is correct: manifests declare their own partition-spec-id and the read path in this repo already plans per-manifest with the manifest's spec (and snapshotProducer summary collection already resolves each file's spec via sp.spec(int(df.SpecID()))), so the default-spec restriction in validateDataFilesToAdd was a writer-side constraint with no format basis. This also matches Java's MergingSnapshotProducer, which routes each file to a per-spec writer via spec(file.specId()). I verified the downstream paths hold up: manifestMergeManager already groups manifests by spec id before merging (table/snapshot_producers.go:369-379), so merge-append with mixed-spec manifests stays coherent, and the strict lookup in writeAddedManifest correctly replaces the sp.spec() empty-spec fallback that would otherwise have written a manifest whose declared spec disagrees with its entries. The per-file checkNoUnknownTransform on each resolved spec in transaction.go closes the gap where an old registered spec has a transform this library can't evaluate. The new test is thorough — round-trip of spec ids and partition tuples, per-manifest pruning via each manifest's own spec, summary partition paths, v3 first-row-id accounting, and unregistered-spec rejection, across v1/v2/v3. I ran go build ./... and the full ./table/... suite on the PR head; all green.

Two minor notes, neither blocking:

• transaction.go: the up-front checkNoUnknownTransform(currentSpec) is now redundant when files actually target the current spec (it's re-checked per resolved spec), and it will reject an add whose files all target other specs just because the current spec has an unknown transform. That's arguably fine as a conservative table-health check, but worth knowing it's intentional.
• snapshot_producers.go writeAddedManifest: err != nil || spec == nil — GetSpecByID never returns (nil, nil), so the spec == nil arm (and the %w of a possibly-nil error) is dead defensiveness. Harmless.

The sorted spec iteration for deterministic manifest order is a nice touch.