fix(parquet): stop BYTE_STREAM_SPLIT FLBA spaced decode aliasing output

[zeroshade]

Rationale for this change

Fixes #1255.

A BYTE_STREAM_SPLIT FIXED_LEN_BYTE_ARRAY column that contains nulls and spans more than one data page decodes values shifted by one position — silently, with no error returned.

spacedExpand moves values into their spaced positions with copy and deliberately does not clean up the null slots:

// because we technically don't care what is in the null slots we don't actually have to clean
// up after ourselves ... Any data that happens to be left in the null slots is fine

That reasoning holds for the scalar column types, whose buffers hold values. But ByteArray / FixedLenByteArray buffers hold slice headers, so copy leaves duplicate headers behind: after expansion a null slot and a valid slot can reference the same backing array.

That is still harmless for a decoder that replaces the header, which is what PlainFixedLenByteArrayDecoder does:

out[idx] = pflba.data[:pflba.typeLen]   // replaces

ByteStreamSplitFixedLenByteArrayDecoder instead writes through the caller's existing slice:

out[idx] = out[idx][:dec.typeLen]       // reuses caller storage
...
out[element][stream] = data[encLoc]     // writes through it

So once flbaRecordReader reused its value buffer for the next page, two output slots shared one backing array and clobbered each other. This explains the full shape of the bug: BYTE_STREAM_SPLIT only, nulls required (to create the duplicates), and two or more pages required (the first page creates the aliases, the second decodes into them).

What changes are included in this PR?

• Add spacedExpandSwap, which swaps instead of copying so the buffer remains a permutation of its original elements — no slot aliases another, and every slot keeps its reusable capacity.
• Use it from ByteStreamSplitFixedLenByteArrayDecoder.DecodeSpaced.
• Leave spacedExpand itself untouched, so every other column type and the non-spaced path are unaffected.
• Add a decoder-level regression test (two pages through one reused buffer, widths 2/3/4/7/8/16) and a randomized differential test asserting spacedExpandSwap places values in exactly the same slots as spacedExpand while never leaving duplicates.
• Add a pqarrow round-trip test over a multi-page, nullable BSS FLBA column — the integration-level case that was returning wrong data.

The decoder-level regression test fails at every width without the fix.

Why swap rather than the simpler alternatives?

I measured two other approaches and rejected both:

Making spacedExpand itself swap is correct but replaces memmove with element-wise swaps for every column type, which is far too expensive on sparse-null runs:

SpacedExpandInt64/n65536/nullEvery0     4.457µ -> 81.265µ   +1723%
SpacedExpandFLBA/n65536/nullEvery0      3.969µ -> 173.959µ  +4283%

clear(out[:toRead]) in DecodeSpaced is a one-liner, but discards all reusable headers. On 4096 slots / 3511 values / width 16 that is 3511 allocations per DecodeSpaced on main today (it would drop to 1 once #1172 lands, but the fix should not depend on that).

Swapping keeps the reuse. Cost on the affected path only (BenchmarkBSSFLBADecodeSpaced, 8192 slots):

w4/nullEvery100    41.64µ -> 60.55µ   +45%
w4/nullEvery7      49.45µ -> 71.95µ   +46%
w4/nullEvery2      95.83µ -> 89.59µ    -7%
w16/nullEvery100   213.9µ -> 385.2µ   +80%
w16/nullEvery7     244.7µ -> 251.3µ      ~
w16/nullEvery2     154.3µ -> 160.4µ      ~
geomean                             +25%

Steady-state allocations stay at 1–2 per call. A ~25% geomean cost on a path that is currently returning incorrect data seemed clearly worth it, and nothing outside BSS FLBA spaced decoding is touched.

Are these changes tested?

Yes — new tests described above. parquet/... passes in full with PARQUET_TEST_DATA supplied, and -race is clean on parquet/internal/encoding.

Are there any user-facing changes?

Yes: BYTE_STREAM_SPLIT FIXED_LEN_BYTE_ARRAY columns with nulls spanning multiple data pages now decode correctly. Previously affected reads returned silently incorrect values.

This PR contains a "Critical Fix". Reading an affected file produced wrong values with no error, which could have been persisted or acted on downstream without any indication of a problem.

[zeroshade]

Checked how this composes with #1172, since both touch fixed_len_byte_array_decoder.go.

They merge cleanly with no conflict (prepareOutput and spacedExpandSwap are independent), and the merged result passes parquet/internal/encoding, parquet/file, and parquet/pqarrow in full, including both PRs' own test sets.

Allocation behaviour for DecodeSpaced on 4096 slots / 3511 values / width 16:

	allocs/op	correct?
main today	0	no
main + this PR	16	yes
#1172 + this PR	0	yes

So the two are complementary rather than competing: #1172's contiguous cold-output block absorbs the entries that need storage, and spacedExpandSwap preserves every header so the steady state needs no allocation at all. Either merge order works.

[zeroshade]

Rebased onto current main, which now carries both #1172 and #1220.

#1220 added an early return to spacedExpand once the decoded prefix is already aligned. I mirrored it in spacedExpandSwap — at that point every remaining swap is a self-swap, so the same reasoning applies and the two functions stay consistent:

idxDecode -= run.Length
// Once the decoded prefix is already aligned every remaining swap is a
// self-swap, so there is nothing left to do. Mirrors spacedExpand.
if idxDecode == run.Pos {
    return numValues
}

Re-verified on the new base: the randomized differential test now compares against the early-returning spacedExpand and still agrees at every valid position while never leaving duplicates; the decoder-level regression test still fails at all six widths without the fix, and the pqarrow round-trip still fails without it. parquet/... passes in full and golangci-lint reports 0 issues.

The earlier measurement stands: with #1172 now on main, DecodeSpaced is 0 allocs/op in steady state with this fix applied.

[fallintoplace]

I would request changes here because I think there is still one case this doesn't handle.

I tried PLAIN nullable FLBA first, then BYTE_STREAM_SPLIT FLBA on the next page, reusing the same valueBuf.

After the PLAIN page, spacedExpand can leave two entries pointing to the same slice. Then BSS uses that same buffer and writes into the slices before spacedExpandSwap happens.

So something like [P0, P1, P1, _, P2] can be left from the first page. Then BSS decodes into the first 3 entries, and the second and third values can both write into P1.

So I think one value can already be overwritten before spacedExpandSwap gets a chance to fix the layout.

Maybe worth adding a PLAIN -> BYTE_STREAM_SPLIT multi-page test for this case.