perf(table): reuse partition projections across planning phases

[fallintoplace]

What

• Reuse the per-spec partition projection cache across local planning phases.
• Manifest-summary pruning and manifest-entry pruning now share the same projections.
• Incremental append planning uses the same cache too.

Why

PlanFiles was building the same projection once while filtering manifests and again while filtering manifest entries. The schema, filter, and partition spec are unchanged between these phases, so the second phase can reuse the result.

The cache stays scoped to one planning operation.

Benchmark

Apple M1 Pro:

• 256 partition specs: 1.93 ms/op -> 1.55 ms/op
• Allocations: 2.56 MB/op -> 2.19 MB/op
• Projection builds: 512/op -> 256/op

Command:

go test ./table -run "^$" -bench BenchmarkPartitionProjectionPlanning -benchtime=500ms -count=5

Tests

• go test ./table -count=1
• go test -race ./table -count=1
• Full non-cloud package test sweep
• golangci-lint run --timeout=10m

[zeroshade]

The shared projection cache is correctly scoped and keyed for both local and incremental planning. Approving, with one test-strengthening follow-up.

A projection reused under a different spec, schema, or filter would silently produce wrong partition filtering, so I checked what the factory actually closes over: metadata and partition spec, the already-resolved schema pointer, rowFilter, and caseSensitive. Both PlanFiles and incremental planning resolve the schema once and then create a single cache, and none of those inputs change between the phases where the projection is now reused. Keying on spec ID alone is therefore sufficient within one planning operation — the cache is scoped to a single call, and ordinary helper callers still get a fresh one, so nothing leaks across scans with different filters.

Also verified:

• Immutability. The cached BooleanExpression AST is traversed, not mutated — RewriteNotExpr and BindExpr visitors construct new trees, and evaluator state is per-call. So one phase can't corrupt what another observes.
• Concurrency. keyDefaultMapErr.Get uses an RWMutex with double-checked insertion, which is safe for the concurrent manifest-entry workers.
• The schema.go change avoids copying the atomic lazy-cache fields during JSON encoding while still emitting explicit fields and identifier-field output, and the new concurrent test covers the race it fixes.

Minor — assert results, not just cache hits (table/scanner_internal_test.go:1001-1002)

TestScanReusesPartitionFiltersAcrossPlanningPhases asserts the factory ran once and neither phase errored, but it never compares the outcome against a run with freshly-computed projections. A change that made the cached projection semantically wrong — or mutated it between phases — would still satisfy that assertion while silently changing which manifests and entries survive pruning. Please add a shared-cache vs fresh-cache comparison over retained manifest paths and collected data entries using the same fixture. That converts this from "the cache was used" to "the cache didn't change the answer", which is the property that actually matters.

Evidence: BenchmarkPartitionProjectionPlanning is committed; the body reports 256 specs at 1.93 → 1.55 ms/op, 2.56 → 2.19 MB/op, and projection builds halving 512 → 256/op, which matches the intent exactly.

CI green (15/15), both commits signed off.

---

This review was drafted by an AI-assisted tool and confirmed by an Apache Iceberg Go maintainer, who has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.

More on how to contribute to Apache Iceberg Go: CONTRIBUTING.md

[zeroshade]

Heads-up: my approval above stands, but this went conflicted before I could merge it.

The cause is the duplicated schema.go MarshalJSON lazy-cache fix that this PR shares with #1960, #1962, #1963 and #1964. Those four landed first, so the identical hunk here now conflicts. A rebase onto current main should drop that hunk entirely and leave just the partition-projection reuse.

Once rebased and green I'll merge it — no need to re-request review. The minor test-strengthening note from my review is still worth picking up, either here or as a follow-up.

For future stacks: carrying the same incidental fix across several PRs guarantees that all but the first conflict. Landing it once on its own, ahead of the stack, avoids the churn.