From bf271c13658e8c71c36735daa603ba4c1c2e5168 Mon Sep 17 00:00:00 2001 From: Madan Kumar Date: Thu, 27 Aug 2026 09:24:50 +0530 Subject: [PATCH 1/3] fix(arrow/array): clamp run ends when concatenating a sliced RunEndEncoded array updateRuns normalizes each input array's run ends by subtracting its logical offset, but never clamps the final run end to the array's logical length. When a RunEndEncoded array is sliced in the middle of a run, the slice keeps that run's original physical end, so after normalization the last run end overshoots the slice length. The overshoot then shifts every following array's run ends, so array.Concatenate silently returns wrong values while the result still passes ValidateFull. Clamp each input's final run end to the running logical length. Signed-off-by: Madan Kumar --- arrow/array/concat.go | 39 ++++++++++++++++++++-------------- arrow/array/concat_test.go | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/arrow/array/concat.go b/arrow/array/concat.go index 3a9efe9e8..04351dcdb 100644 --- a/arrow/array/concat.go +++ b/arrow/array/concat.go @@ -861,37 +861,44 @@ func updateRuns[T int16 | int32 | int64](inputData []arrow.ArrayData, inputBuffe // can fold the end and beginning of each array we're concatenating // into a single run pos := 0 + logicalLen := T(0) for i, buf := range inputBuffers { if buf.Len() == 0 { continue } src := arrow.GetData[T](buf.Bytes()) + logicalLen += T(inputData[i].Len()) if pos == 0 { pos += copy(output, src) // normalize the first run ends by subtracting the offset for j := 0; j < pos; j++ { output[j] -= T(inputData[i].Offset()) } + } else { + lastEnd := output[pos-1] + // we can check the last runEnd in the src and add it to the + // last value that we're adjusting them all by to see if we + // are going to overflow + if uint64(lastEnd)+uint64(int(src[len(src)-1])-inputData[i].Offset()) > uint64(maxOf[T]()) { + return fmt.Errorf("%w: overflow in run-length-encoded run ends concat", arrow.ErrInvalid) + } - continue - } - - lastEnd := output[pos-1] - // we can check the last runEnd in the src and add it to the - // last value that we're adjusting them all by to see if we - // are going to overflow - if uint64(lastEnd)+uint64(int(src[len(src)-1])-inputData[i].Offset()) > uint64(maxOf[T]()) { - return fmt.Errorf("%w: overflow in run-length-encoded run ends concat", arrow.ErrInvalid) + // adjust all of the run ends by first normalizing them (e - data[i].offset) + // then adding the previous value we ended on. Since the offset + // is a logical length offset it should be accurate to just subtract + // it from each value. + for j, e := range src { + output[pos+j] = lastEnd + T(int(e)-inputData[i].Offset()) + } + pos += len(src) } - // adjust all of the run ends by first normalizing them (e - data[i].offset) - // then adding the previous value we ended on. Since the offset - // is a logical length offset it should be accurate to just subtract - // it from each value. - for j, e := range src { - output[pos+j] = lastEnd + T(int(e)-inputData[i].Offset()) + // a slice can end in the middle of a run, so this input's final physical run end + // can reach past its logical length; clamp it to keep the run ends within bounds and + // stop the overshoot from shifting every following array's run ends. + if output[pos-1] > logicalLen { + output[pos-1] = logicalLen } - pos += len(src) } return nil } diff --git a/arrow/array/concat_test.go b/arrow/array/concat_test.go index df906bd92..b7b8d0d88 100644 --- a/arrow/array/concat_test.go +++ b/arrow/array/concat_test.go @@ -819,6 +819,49 @@ func TestConcatRunEndEncoded(t *testing.T) { } } +func TestConcatRunEndEncodedMidRunSlice(t *testing.T) { + // A run-end encoded array sliced in the middle of a run keeps that run's physical end, which + // reaches past the slice's logical length. Concatenating it must clamp that final run end, + // otherwise the overshoot shifts every following array's run ends and silently corrupts values. + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + bldr := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int32, arrow.PrimitiveTypes.Int64) + defer bldr.Release() + valBldr := bldr.ValueBuilder().(*array.Int64Builder) + + // runs: 100x3, 200x2, 300x3 -> run ends [3, 5, 8], logical length 8 + bldr.Append(3) + valBldr.Append(100) + bldr.Append(2) + valBldr.Append(200) + bldr.Append(3) + valBldr.Append(300) + full := bldr.NewArray() + defer full.Release() + + // slice [1, 4): logical [100, 100, 200], length 3, ending in the middle of the "200" run + sliced := array.NewSlice(full, 1, 4) + defer sliced.Release() + + bldr.Append(2) + valBldr.Append(700) + tail := bldr.NewArray() + defer tail.Release() + + result, err := array.Concatenate([]arrow.Array{sliced, tail}, mem) + require.NoError(t, err) + defer result.Release() + + rle := result.(*array.RunEndEncoded) + values := rle.Values().(*array.Int64) + got := make([]int64, rle.Len()) + for i := range got { + got[i] = values.Value(rle.GetPhysicalIndex(i)) + } + assert.Equal(t, []int64{100, 100, 200, 700, 700}, got) +} + func TestConcatAlmostOverflowRunEndEncoding(t *testing.T) { tests := []struct { offsetType arrow.DataType From 300dbe3066e17f60cc0f4605cafc367df4cf94e9 Mon Sep 17 00:00:00 2001 From: Madan Kumar Date: Sat, 29 Aug 2026 03:37:19 +0530 Subject: [PATCH 2/3] fix(arrow/array): clamp the RunEndEncoded final run end before the overflow check The logical-length clamp fixed the sliced-array overshoot but the overflow check still used the input's unclamped physical final run end, so a valid slice whose physical run end is near the run-end type limit was rejected before the clamp applied. Clamp the normalized final run end to the input's logical length up front and use it for both the overflow check and the written final run end. Adds a near-int16-limit boundary regression test. Signed-off-by: Madan Kumar --- arrow/array/concat.go | 55 +++++++++++++++++++++----------------- arrow/array/concat_test.go | 30 +++++++++++++++++++++ 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/arrow/array/concat.go b/arrow/array/concat.go index 04351dcdb..f71d04780 100644 --- a/arrow/array/concat.go +++ b/arrow/array/concat.go @@ -861,44 +861,51 @@ func updateRuns[T int16 | int32 | int64](inputData []arrow.ArrayData, inputBuffe // can fold the end and beginning of each array we're concatenating // into a single run pos := 0 - logicalLen := T(0) for i, buf := range inputBuffers { if buf.Len() == 0 { continue } src := arrow.GetData[T](buf.Bytes()) - logicalLen += T(inputData[i].Len()) + offset := inputData[i].Offset() + + // A slice can end in the middle of a run, leaving this input's final physical + // run end past its logical length. Clamp the normalized final run end to the + // input's logical length before both the overflow check and the output write: + // otherwise a valid near-limit slice trips a false overflow, and the written + // run end overshoots (shifting every following array's run ends). + finalEnd := int(src[len(src)-1]) - offset + if finalEnd > inputData[i].Len() { + finalEnd = inputData[i].Len() + } + if pos == 0 { pos += copy(output, src) // normalize the first run ends by subtracting the offset for j := 0; j < pos; j++ { - output[j] -= T(inputData[i].Offset()) - } - } else { - lastEnd := output[pos-1] - // we can check the last runEnd in the src and add it to the - // last value that we're adjusting them all by to see if we - // are going to overflow - if uint64(lastEnd)+uint64(int(src[len(src)-1])-inputData[i].Offset()) > uint64(maxOf[T]()) { - return fmt.Errorf("%w: overflow in run-length-encoded run ends concat", arrow.ErrInvalid) + output[j] -= T(offset) } + output[pos-1] = T(finalEnd) + continue + } - // adjust all of the run ends by first normalizing them (e - data[i].offset) - // then adding the previous value we ended on. Since the offset - // is a logical length offset it should be accurate to just subtract - // it from each value. - for j, e := range src { - output[pos+j] = lastEnd + T(int(e)-inputData[i].Offset()) - } - pos += len(src) + lastEnd := output[pos-1] + // check whether adding this input's clamped final run end to the previous + // end will overflow the run-end type + if uint64(lastEnd)+uint64(finalEnd) > uint64(maxOf[T]()) { + return fmt.Errorf("%w: overflow in run-length-encoded run ends concat", arrow.ErrInvalid) } - // a slice can end in the middle of a run, so this input's final physical run end - // can reach past its logical length; clamp it to keep the run ends within bounds and - // stop the overshoot from shifting every following array's run ends. - if output[pos-1] > logicalLen { - output[pos-1] = logicalLen + // adjust all of the run ends by first normalizing them (e - data[i].offset) + // then adding the previous value we ended on. Since the offset + // is a logical length offset it should be accurate to just subtract + // it from each value. + for j, e := range src { + output[pos+j] = lastEnd + T(int(e)-offset) } + pos += len(src) + // the write above uses the unclamped physical end for the final run; set it + // to the clamped logical end. + output[pos-1] = lastEnd + T(finalEnd) } return nil } diff --git a/arrow/array/concat_test.go b/arrow/array/concat_test.go index b7b8d0d88..f8cd0c515 100644 --- a/arrow/array/concat_test.go +++ b/arrow/array/concat_test.go @@ -862,6 +862,36 @@ func TestConcatRunEndEncodedMidRunSlice(t *testing.T) { assert.Equal(t, []int64{100, 100, 200, 700, 700}, got) } +func TestConcatRunEndEncodedNearTypeLimitSlice(t *testing.T) { + // A sliced input whose physical final run end is near the run-end type limit must + // not trip a false overflow: the overflow check has to use the clamped logical end, + // not the physical one. int16 prefix of 32760 + a 1-element slice of a physical + // 32767-length run should yield 32761, not an overflow error. + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + prefixBldr := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int16, arrow.PrimitiveTypes.Int64) + defer prefixBldr.Release() + prefixBldr.Append(32760) + prefixBldr.ValueBuilder().(*array.Int64Builder).Append(1) + prefix := prefixBldr.NewArray() + defer prefix.Release() + + bigBldr := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int16, arrow.PrimitiveTypes.Int64) + defer bigBldr.Release() + bigBldr.Append(32767) + bigBldr.ValueBuilder().(*array.Int64Builder).Append(2) + big := bigBldr.NewArray() + defer big.Release() + oneElem := array.NewSlice(big, 0, 1) // first element of the 32767-length run + defer oneElem.Release() + + result, err := array.Concatenate([]arrow.Array{prefix, oneElem}, mem) + require.NoError(t, err) + defer result.Release() + assert.EqualValues(t, 32761, result.Len()) +} + func TestConcatAlmostOverflowRunEndEncoding(t *testing.T) { tests := []struct { offsetType arrow.DataType From fe3b0ab9fa62e638cea1741c08136fa520047e3f Mon Sep 17 00:00:00 2001 From: Madan Kumar Date: Tue, 1 Sep 2026 13:57:41 +0530 Subject: [PATCH 3/3] fix(arrow/array): keep REE concat final run-end arithmetic in the run-end type Converting the final run end through int overflowed on 32-bit targets: a valid run-end-encoded array with final run end math.MaxInt64 and logical length 1 became -1 during concatenation. Compute and clamp the final run end (and the per-run normalization) in the run-end type T, clamping against T(inputData[i].Len()) rather than going through int. Adds a MaxInt64 regression test. Signed-off-by: Madan Kumar --- arrow/array/concat.go | 12 ++++++------ arrow/array/concat_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/arrow/array/concat.go b/arrow/array/concat.go index f71d04780..f0546b5cb 100644 --- a/arrow/array/concat.go +++ b/arrow/array/concat.go @@ -873,9 +873,9 @@ func updateRuns[T int16 | int32 | int64](inputData []arrow.ArrayData, inputBuffe // input's logical length before both the overflow check and the output write: // otherwise a valid near-limit slice trips a false overflow, and the written // run end overshoots (shifting every following array's run ends). - finalEnd := int(src[len(src)-1]) - offset - if finalEnd > inputData[i].Len() { - finalEnd = inputData[i].Len() + finalEnd := src[len(src)-1] - T(offset) + if finalEnd > T(inputData[i].Len()) { + finalEnd = T(inputData[i].Len()) } if pos == 0 { @@ -884,7 +884,7 @@ func updateRuns[T int16 | int32 | int64](inputData []arrow.ArrayData, inputBuffe for j := 0; j < pos; j++ { output[j] -= T(offset) } - output[pos-1] = T(finalEnd) + output[pos-1] = finalEnd continue } @@ -900,12 +900,12 @@ func updateRuns[T int16 | int32 | int64](inputData []arrow.ArrayData, inputBuffe // is a logical length offset it should be accurate to just subtract // it from each value. for j, e := range src { - output[pos+j] = lastEnd + T(int(e)-offset) + output[pos+j] = lastEnd + e - T(offset) } pos += len(src) // the write above uses the unclamped physical end for the final run; set it // to the clamped logical end. - output[pos-1] = lastEnd + T(finalEnd) + output[pos-1] = lastEnd + finalEnd } return nil } diff --git a/arrow/array/concat_test.go b/arrow/array/concat_test.go index f8cd0c515..a70951344 100644 --- a/arrow/array/concat_test.go +++ b/arrow/array/concat_test.go @@ -892,6 +892,36 @@ func TestConcatRunEndEncodedNearTypeLimitSlice(t *testing.T) { assert.EqualValues(t, 32761, result.Len()) } +func TestConcatRunEndEncodedInt64FinalRunEndClamp(t *testing.T) { + // A run-end-encoded array whose final physical run end is math.MaxInt64, + // sliced to a small logical length, must clamp the final run end with + // run-end-typed arithmetic. Converting the value through int overflows on + // 32-bit targets, turning a valid run end into a negative output value. + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + prefixBldr := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int64, arrow.PrimitiveTypes.Int64) + defer prefixBldr.Release() + prefixBldr.Append(5) + prefixBldr.ValueBuilder().(*array.Int64Builder).Append(1) + prefix := prefixBldr.NewArray() + defer prefix.Release() + + bigBldr := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int64, arrow.PrimitiveTypes.Int64) + defer bigBldr.Release() + bigBldr.Append(math.MaxInt64) + bigBldr.ValueBuilder().(*array.Int64Builder).Append(2) + big := bigBldr.NewArray() + defer big.Release() + oneElem := array.NewSlice(big, 0, 1) // first element of the MaxInt64-length run + defer oneElem.Release() + + result, err := array.Concatenate([]arrow.Array{prefix, oneElem}, mem) + require.NoError(t, err) + defer result.Release() + assert.EqualValues(t, 6, result.Len()) +} + func TestConcatAlmostOverflowRunEndEncoding(t *testing.T) { tests := []struct { offsetType arrow.DataType