fix(manifest): protect DataFile metadata from external mutation

[fallintoplace]

What changed

Return defensive copies from DataFile collection and optional-value getters. Deep-copy byte bounds and binary partition values, and clone caller-owned builder inputs that contain mutable storage.

Why

The getters exposed cached maps, slices, byte buffers, and pointers directly. Callers could mutate a decoded or built data file through a value returned by an accessor, including partition and bounds data used during planning.

The regression test mutates builder inputs and every affected getter, then verifies the data file remains unchanged.

Testing

• go test .
• go vet .

[laskoviymishka]

The defensive copying looks thorough, and the isolation test is a good way to catch the aliasing cases in the getters.

I’d still hold the merge over DataFileStatsRef.

The PR is trying to make DataFile state immutable to callers, but DataFileStatsRef returns map[int][]byte with slices that still share backing arrays with the cached maps. An in-package caller can modify those bytes and corrupt the bounds. Since initColumnStatsData is guarded by sync.Once, that corruption then stays in place.

The current evaluators only read from the map, so this is not an active bug today, but the “zero-copy access to immutable DataFile state” comment is stronger than what the API guarantees. At minimum, the returned slices should be documented as read-only.

I’m also not convinced the escape hatch is worth the extra machinery yet. It adds a package, token type, interface, and runtime assertion to avoid five map copies per evaluation, but there is no benchmark showing those copies matter. Partition() is on the same per-file scan path and now always copies, without a similar fast path. That makes the optimization look both unproven and inconsistent.

I’d settle the following before merge:

• Document, or enforce, that the byte slices returned by DataFileStatsRef are read-only.
• Add a benchmark for the stats path, or explain why the escape hatch is needed. It would also be worth deciding whether Partition() needs the same treatment.
• Document that clonePartitionMap only guarantees deep isolation for the concrete types produced by the decoder, since the builder accepts arbitrary map[int]any values.
• Remove the second clone of binary partition values in NewDataFileBuilder.
• Tighten the zero-allocation test with InDelta and a correctness assertion inside the closure.
• Consider adding the Avro decode path to the isolation test.

Once those are sorted, I’ll take another look.

[laskoviymishka]

This is the one I'd want settled before merge.

The PR is about protecting DataFile state from callers, but the []byte values in the lowerBounds/upperBounds returned here are the same backing arrays as d.lowerBoundMap/d.upperBoundMap — a caller doing lowerBounds[id][0] = x writes straight through into cached state, and since initColumnStatsData is a sync.Once it never rebuilds.

Both current callers are read-only so there's no live bug, but "zero-copy access to immutable DataFile state" claims more than the code delivers — the maps are shared and the byte values are mutable. At minimum, document on this method that callers must treat the returned slices as read-only, so the next caller wiring into this path doesn't corrupt bounds by accident.

[laskoviymishka]

clonePartitionMap only deep-copies []byte — every other any is carried through by the shallow maps.Clone, so the isolation guarantee is really "[]byte only".

For Avro-decoded files that's fine, since convertAvroValueToIcebergType only produces value types. But NewDataFileBuilder takes a caller-supplied map[int]any, and nothing stops a caller putting a *big.Rat or a nested map in there — those still alias after the clone, so the stated protection doesn't hold on the builder path. I'd document that isolation only holds for the decoder's concrete types, or handle the unsupported case explicitly.

[laskoviymishka]

Partition() is on the same per-file scan path as the stats getters (partition pruning calls it once per file, plus once per equality-delete/data pair) and now clones unconditionally, while stats get the DataFileStatsRef fast path — so the optimization is asymmetric.

More to the point: the escape hatch adds a new internal package, a token type, an interface, and a runtime assertion to avoid five map copies per eval, and there's no benchmark showing that copy was ever hot. This PR needs a benchmark that demonstrates the saving (e.g. BenchmarkInclusiveMetricsEval with and without the ref path) before we take on this machinery — if it doesn't move the numbers, the escape hatch should come out and we just copy. Either way the result has to reconcile with Partition() copying unconditionally on the same path.