feat(table): adopt arrow-go 18.8, fast-path shredded variant extract, restore parquet dict fallback

[nssalian]

Rationale for the change

Bumps arrow-go to v18.8, uses its new compute.VariantGet for variant extract with a zero-copy fast path for shredded fields, and re-enables the Parquet dictionary cost-fallback that v18.8 disabled for compressed columns.

Changes

Arrow-go v18.8

1. Bump github.com/apache/arrow-go/v18 to v18.8.0.
2. Accept both the canonical arrow.parquet.variant and legacy parquet.variant extension names (arrow_utils.go, internal/parquet_files.go).
3. Force WithDictionaryCostFallbackFor on every leaf so zstd columns drop a dictionary that saves nothing (parquet-mr shouldFallBack parity; v18.8 disabled this for compressed columns). Leaf paths are enumerated by walking the arrow schema directly, with a pqarrow.ToParquet fallback for list/map schemas (internal/parquet_files.go).

Variant extract

4. Residual extract navigates the path via compute.VariantGet (feat(extensions): add VariantGet for path extraction from variant arrays arrow-go#1206), with the per-row CastVariantLiteral walk as fallback. The member-name path is obtained through an internal type-assertion, so it stays off the public BoundExtract interface (variant_residual.go, variant_extract.go).
5. tryShreddedTypedColumn: when a field is shredded to exactly the target type (no field-level residual), return its typed_value column directly - Iceberg's cast is then an identity (mirrors Java VariantExpressionUtil.castTo). Null rows and absent-object ancestors fold into the validity mask (zero-copy when none, one bitmap-AND per level otherwise); promotions, field-level residual, and unshredded data fall back (variant_residual.go).
6. Fast path honors context cancellation (returns the context error instead of a silent partial result) and bails on non-zero child offsets (a sliced array reads through the per-row path) (variant_residual.go).

Upgrade note

Variant columns written after this bump embed the canonical arrow.parquet.variant Arrow extension name. Java/pqarrow and iceberg-rust read these fine (the Parquet VARIANT logical type is unchanged); only an older iceberg-go reader still matching just parquet.variant would not recognize the field during a mixed-version rolling upgrade.
The read side here accepts both names, so no action is needed for iceberg-go >= this version.

Performance

BenchmarkVariantExtract measures extract-column materialization on a 131,072-row batch (the default read batch size), Apple M4 Max. "Production" is the current code (the fast path when the field is shredded to the queried type, otherwise the compute.VariantGet fallback); "per-row" is the previous walk.

int64 field, 131,072 rows	production	per-row (previous)
clean, fully shredded (fast path)	66 ns, 1 alloc	68.1 ms, 2,358,045 allocs
1% null rows (fast path)	4.7 us, 10 allocs	65.9 ms, 2,334,461 allocs
nested $.a.b (fast path)	97 ns, 2 allocs	98.5 ms, 2,751,261 allocs
1% off-type rows, field-level residual (fallback)	32.2 ms	64.8 ms
unshredded (routed to per-row)	32.9 ms	32.8 ms

String targets show the same pattern.

• When the field is shredded to the queried type - the common case after feat(table): type uniformity for variant shredding inference #1846 type uniformity, including nullable columns and nested paths - extraction returns the shredded typed_value column directly: O(1) in the row count and effectively allocation-free, versus a per-row decode+cast that scales with the batch.
• When the field is not cleanly shredded, extraction falls back. Partly-shredded data (field-level residual) uses the columnar compute.VariantGet (~2x faster than the per-row walk); unshredded columns route directly to the per-row walk rather than paying VariantGet.
• These figures cover the extract-column materialization step only, not a full scan, which additionally includes predicate evaluation, IO, and decompression.

Testing

• TestExtractFastPathParity: fast-path output byte-identical to the per-row reference across exact/nested/absent/promotion/residual/null/string shapes; asserts it is wired in and zero-copy (mutation-checked) under a checked allocator.
• TestFastPathRootResidualObjectFallsBack, TestFastPathDecimalScaleNearMissFallsBack, TestFastPathTimestampTzNearMissFallsBack, TestFastPathSlicedOffsetFallsBack: hand-built shapes that must bail (root residual, decimal-scale / timestamp-tz TypeEqual near-miss, non-zero offset).
• TestFastPathWrapperFieldNullMatchesPerRow: the spec-undefined null-wrapper/live-child shape reads the child in both paths (parity).
• TestExtractColumnValuesContextCancelled: a cancelled context returns its error, not a silent result.
• TestMiddleTierColumnarCast: int32-shredded extracted as int64 exercises compute.VariantGet + the promoting cast.
• TestShreddedVariantExtractResidualNoLeak: extract through the real residual filter under a checked allocator (leak check).
• TestGetWritePropertiesEnablesDictCostFallback, TestDictCostFallbackWalkMatchesToParquet: cost fallback enabled per leaf, and the arrow-leaf walk's paths match pqarrow.ToParquet.
• TestVariantExtractScanEndToEnd / TestVariantExtractResidualAndHeterogeneous: fast path and fallback via the public Scan().WithRowFilter(...).
• BenchmarkVariantExtract: the numbers above.

AI Disclosure

• Model: Claude Opus 4.8
• Platform/Tool: Claude Code
• Human Oversight: fully reviewed
• Prompt Summary: Adopt arrow-go 18.8 and add a zero-copy fast path for fully-shredded variant extract

[laskoviymishka]

Really nice work on this. The three-tier extract routing reads cleanly (zero-copy typed leaf, then compute.VariantGet + iceberg's cast, then the per-row fallback), and I like that the parity test checks the fast and columnar output against the per-row reference on every case with the reader on a checked allocator to catch a zero-copy leak. CI is green across both Go versions, the Spark 3.5 and 4.0 integration runs, and the s390x cross-compile, so this is in good shape.

A few things I'd sort before it goes in, none of them huge. The one I'd weigh most is the public API: VariantPath() is added to the exported BoundExtract interface, which is a breaking change for anything downstream implementing it, and it leaks arrow-go's variant.VariantPath into our public surface for what's really an internal optimization. I'd keep it inside table/ behind a small unexported interface, or expose []string and rebuild the path there. The other is the error handling in extractColumnValues: err != nil from VariantGet falls back to the per-row walk for every error, including context.Canceled/DeadlineExceeded and OOM, and the per-row path doesn't check the context either, so a cancelled scan quietly finishes and looks like success. I'd inspect the error (or check ctx.Err()) and fall back only on a real navigation miss.

Two more worth a look even with CI green: the fast path never merges validity for the per-key {value, typed_value} wrapper struct, so an externally-built shredded array with an absent key could return a live value where the result should be null; and the child bitmap access assumes offset 0, which a sliced array would break. Both are one-line guards plus a fixture that actually drives them. The dictionary-cost-fallback schema conversion, the first-init self-AND, the deferred-release hygiene in the leak tests, the middle-tier test gap, and the arrow.parquet.variant CHANGELOG note are all smaller and left inline.

Given this rides on the arrow-go 18.8 bump and leans on compute.VariantGet plus the extension-name handling, I think we'd also want @zeroshade to take an explicit pass and sign off before it lands.

[laskoviymishka]

This is in good shape. This is already good for me.

The two things I weighed most last round are both handled: the public-API break is gone (VariantPath() sits on the unexported boundExtract[T] and reaches table through the duck-typed variantPathOf, so variant.VariantPath stays off BoundExtract), and context is honored end to end now, with the fallback inspecting the error (errors.Is(err, context.Canceled)) and the per-row walk polling ctx.Err(), locked in by TestExtractColumnValuesContextCancelled.

Everything else I flagged last round is in too:

• sliced / non-zero-offset arrays bail the fast path (TestFastPathSlicedOffsetFallsBack)
• the wrapper-null case has a fixture and both paths agree (TestFastPathWrapperFieldNullMatchesPerRow)
• the leak test reads on the checked allocator, so the zero-copy leaf is tracked
• a middle-tier columnar-cast test exists (TestMiddleTierColumnarCast)

Nothing left blocks. Two small ones I'd still like to see land: a short CHANGELOG/upgrade note that variant columns written after the 18.8 bump need 18.8+ readers (the read side already takes both names, this is just so a mixed-version cluster isn't surprised by the default: panic), and one list-schema case exercising dictCostFallbackViaParquet, which no test currently reaches. The rest, a ctx.Err() poll in the middle-tier loop, a nil-guard on tvb, and the strict-vs-permissive call on the wrapper-null shape, are true nits and left inline.

Still worth getting @zeroshade's eyes since it leans on the 18.8 bump and compute.VariantGet, but I don't see anything here that should hold it up.

[laskoviymishka]

Good from me to merge!

Everything I left inline last round landed:

• the middle-tier loop polls ctx.Err() at the 4096 boundary
• rootResidualHidesRows guards a nil tvb conservatively
• the null-wrapper-is-absent-key call is settled, with TestFastPathWrapperFieldNullIsAbsentKey locking it in
• the list-schema coverage is there as TestDictCostFallbackListSchemaUsesToParquet

The one round-2 ask still not in the diff is the CHANGELOG/upgrade note: files written after the 18.8 bump embed arrow.parquet.variant and need 18.8+ readers (the read side already takes both names, so this is just so a mixed-version cluster isn't surprised by the default: panic). Not a blocker, but it's the one thing I'd still like to see land before this closes.

A few small things I noticed this pass, all inline and none blocking: mergeValidity bare-returns on a nil validity buffer with NullN() > 0 where rootResidualHidesRows bails on that same shape; schemaHasListOrMap and the dict-fallback walk don't unwrap RunEndEncodedType (latent, perf-only, since we never emit RLE); the context-cancellation test only reaches the pre-entry guard, not the 4096 poll it's meant to protect; and the root-residual fixture builds a child-length-mismatched struct that only passes because the fast path bails first.

Still worth @zeroshade's explicit sign-off since this leans on the 18.8 bump and compute.VariantGet, but nothing here holds it up. Sort the CHANGELOG note and I'm happy.

[zeroshade]

Reviewed the full diff, built the branch, and ran ./table/... ./ — all green. Verified there are no remaining hardcoded parquet.variant literals outside tests, and that the memory handling in tryShreddedTypedColumn is sound: array.NewData retains buffers, so the mask.Release() after constructing the output is correct, and bail() returns a true nil interface so there is no typed-nil trap at the fast != nil check.

The fast-path testing is the strongest part of this PR. TestExtractFastPathParity asserts three things at once per shape — that the fast path fires (or deliberately does not), byte-parity against the per-row reference, and buffer sharing via sharesDataBuffers — all under a leak-checked allocator. That combination is what makes the structural type assertion in variantPathOf safe: if boundExtract.VariantPath() is ever renamed, sharesDataBuffers fails loudly rather than the extract silently degrading to the per-row walk.

One design point worth discussing (inline on dictCostFallbackProps) plus two smaller items. Nothing blocking.

Compatibility — worth capturing outside the PR description. With WithStoreSchema() enabled, the embedded ARROW:schema now carries arrow.parquet.variant, so an older iceberg-go reader will not recognize variant columns during a mixed-version rolling upgrade. The description covers this accurately, but it should land in release notes too, since that is where an operator planning an upgrade will look.

On scope. Three concerns in one PR, but they are not equally separable: the dict-fallback restore has to ship with the bump, since v18.8 is what disabled it for compressed columns — splitting it would land a file-size regression. The variant fast path is genuinely independent and could have been a follow-up. Flagging it mainly because the dict-fallback change affects every Parquet write, not just variant, so the variant work cannot be reverted cleanly on its own if it needs to be.

[zeroshade]

Approving.

CI is green across the board — both Spark integration suites, s390x cross-compile, and all four Go/OS combinations — and @laskoviymishka has made three thorough passes with everything substantive resolved.

What I checked independently: built the branch and ran ./table/... ./ clean; confirmed the memory handling in tryShreddedTypedColumn is correct (array.NewData retains buffers, so releasing mask after constructing the output is right, and bail() returns a true nil interface so there is no typed-nil trap at the fast != nil check); and confirmed no hardcoded parquet.variant literals remain outside tests.

TestExtractFastPathParity is the part that earns the most confidence here — asserting fast-path firing, byte-parity against the per-row reference, and buffer sharing together, under a leak-checked allocator, is what makes both the zero-copy claim and the variantPathOf type assertion safe against silent regression.

The three threads I left open are all non-blocking and fine as follow-ups:

• two more cases (RunEndEncodedType, arrow.Null) in TestDictCostFallbackWalkMatchesToParquet — latent and perf-only, since Iceberg-generated schemas never carry RLE and the consequence is file size rather than correctness
• dictCostFallbackViaParquet passing DefaultWriterProps() where the writer uses WithStoreSchema() — no behavioral difference today, purely defensive against future drift
• the length-assumption nit in mergeValidity — safe as written

One thing I did not verify: the benchmark numbers. That is a performance claim rather than a correctness one, and the fast path's correctness is well covered, so I am not holding on it — but for the record nobody has independently reproduced the 66ns / 1-alloc figure.

The only outstanding item is the upgrade note at release time: variant columns written after this bump embed arrow.parquet.variant and need a reader from this version or later.

@nssalian @laskoviymishka thanks for the thorough back-and-forth on this one.

[nssalian]

Thanks for the review, @zeroshade. I can make those changes here so this goes in cleanly. Let me follow up with them in a follow up commit.

[laskoviymishka]

LGTM! Let's Go!