fix(arrow/array): clamp run ends when concatenating a sliced RunEndEncoded array

[winklemad]

Rationale for this change

array.Concatenate silently corrupts values when one of the inputs is a RunEndEncoded array that was sliced in the middle of a run.

updateRuns normalizes each input's run ends by subtracting its logical offset, but never clamps the final run end to the array's logical length. A slice keeps the original physical end of the run it cuts through, so after normalization the last run end overshoots the slice length. That overshoot then shifts every following array's run ends, and the result still passes ValidateFull, so nothing flags the corruption.

Reproduction (values are wrong, no error):

b := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int32, arrow.PrimitiveTypes.Int64)
vb := b.ValueBuilder().(*array.Int64Builder)
// run ends [3,5,8]: 100x3, 200x2, 300x3
b.Append(3); vb.Append(100); b.Append(2); vb.Append(200); b.Append(3); vb.Append(300)
full := b.NewArray()

sliced := array.NewSlice(full, 1, 4) // logical [100,100,200], ends mid-run of the 200s
b.Append(2); vb.Append(700)
tail := b.NewArray()

result, _ := array.Concatenate([]arrow.Array{sliced, tail}, mem)
// want [100,100,200,700,700]
// got  [100,100,200,200,700]   <- index 3 corrupted

sliced on its own decodes correctly ([100,100,200]); only the concatenated result is wrong, so the defect is entirely in the run-end merge.

What changes are included in this PR?

Clamp each input array's final run end to the running logical length in updateRuns (arrow/array/concat.go). This is the single place run ends are merged for RunEndEncoded concatenation (the generic function covers int16/int32/int64 run-end types). No change to any array that ends on a run boundary — only a slice that cuts through a run is affected, and it now stays within its logical length.

Are these changes tested?

Yes — added TestConcatRunEndEncodedMidRunSlice in arrow/array/concat_test.go, which reproduces the corruption (it fails without the fix) and uses a checked allocator to confirm no leaks. The existing TestConcatRunEndEncoded / TestConcatAlmostOverflowRunEndEncoding and the full arrow/array package tests still pass; gofmt and go vet are clean.

Are there any user-facing changes?

Yes — array.Concatenate now returns correct values when an input is a mid-run slice of a RunEndEncoded array, instead of silently wrong ones. No API change.

[zeroshade]

The logical run-end clamp fixes the ordinary sliced-array overshoot, but the overflow check still runs against the unclamped physical run end. As a result, valid sliced inputs near the run-end type limit are rejected before the clamp can take effect.

I reproduced this with valid int16 RunEndEncoded arrays: a 32,760-element prefix followed by a one-element slice of a physical 32,767-element run returns an overflow error instead of a valid 32,761-element result. Details are inline.

The package, race, randomized slicing, vet, and CI checks otherwise pass.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Arrow Go maintainer. After you've
addressed the points above and pushed an update, an Apache Arrow Go
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Apache Arrow Go handles maintainer review:
CONTRIBUTING.md.

[zeroshade]

The overflow check still uses the slice’s physical final run end before the new logical-length clamp. This rejects valid near-limit inputs. A valid int16 REE prefix of length 32,760 followed by a one-element slice of a physical 32,767-element run should produce length 32,761, but currently returns invalid: overflow in run-length-encoded run ends concat. Please clamp the normalized final source end to the sliced input’s logical length before both the overflow check and output write, and add this boundary regression test.

[winklemad]

Thanks @zeroshade — you're exactly right, the overflow check was still measuring the unclamped physical run end. Fixed in 300dbe3: I compute the clamped final run end once up front (min(normalized physical end, input.Len())) and use it for both the overflow check and the written final run end (the raw write could otherwise overshoot/wrap before the post-clamp could catch it).

Your repro now passes — an int16 prefix of 32,760 followed by a one-element slice of a physical 32,767-length run returns length 32,761 instead of the false overflow. Added TestConcatRunEndEncodedNearTypeLimitSlice for that boundary; the existing RunEndEncoded tests and the full arrow/array suite stay green.

[zeroshade]

The previous overflow-check finding is fixed at the current head: clamping now precedes the checked addition, with near-boundary regression coverage.

A separate 32-bit overflow remains when the final int64 run end is converted through int. This can turn valid run ends into negative output values; details and the fix direction are inline.

Array tests, race tests, vet, randomized differential tests, and Linux/386 cross-compilation otherwise pass. No CI checks are currently reported for this head.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Arrow Go maintainer. After you've
addressed the points above and pushed an update, an Apache Arrow Go
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Apache Arrow Go handles maintainer review:
CONTRIBUTING.md.

[zeroshade]

Converting the final int64 run end to int overflows on 32-bit targets. A valid run-end encoded array with final run end math.MaxInt64 and logical length 1 becomes -1 during concatenation, producing invalid output. Please keep this calculation in type T, clamp against T(inputData[i].Len()), and add a 32-bit int64 regression test.

[winklemad]

Fixed in fe3b0ab. The final run end and the per-run normalization in the write loop now stay in the run-end type T and clamp against T(inputData[i].Len()), so nothing goes through int — the MaxInt64 / logical-length-1 case no longer wraps to a negative value on 32-bit targets. Added TestConcatRunEndEncodedInt64FinalRunEndClamp for that scenario (it passes on a 64-bit host where int is 64-bit, and guards the 32-bit path). REE/concat suite + go vet are green.