This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new 0ffa1b63 fix(arrow/array): validate bufferless run-end encoded arrays
(#1421)
0ffa1b63 is described below
commit 0ffa1b63337b10272f8a4009003e7280223922e6
Author: Matt Topol <[email protected]>
AuthorDate: Mon Oct 26 16:08:03 2026 +0000
fix(arrow/array): validate bufferless run-end encoded arrays (#1421)
### Rationale for this change
Split out of #1409.
`ValidateFull` rejects run-end-encoded arrays from the builder. A REE
array from `NewRunEndEncodedBuilder` (or a `Concatenate` of such arrays)
fails with `expected at least 1 buffers for run_end_encoded<...>, got
0`, on every platform. The REE layout declares one always-null
placeholder buffer (`datatype_encoded.go:40`); `NewRunEndEncodedArray`
and the IPC reader supply a `nil` placeholder, but the builder and C
Data import produce zero buffers. The Arrow spec says REE arrays have no
physical buffers, so both representations are legitimate.
### What changes are included in this PR?
`arrow/array/validate.go` accepts a REE array (including extension types
with REE storage) that has no buffers. Arrays with a `nil` placeholder
go through the existing checks as before, and non-nil or extra parent
buffers are still rejected. Constructors, layouts, IPC and C Data are
unchanged.
### Are these changes tested?
Yes. New `TestRunEndEncodedValidateConstructedArrays` checks that
builder-produced, empty, sliced, constructor-built, `Concatenate`d and
REE-extension arrays pass `ValidateFull`. The builder, `Concatenate`,
empty, sliced and extension cases fail before the fix; constructor cases
passed already. Existing rejection cases (bad run ends, non-nil/extra
parent buffers) still fail validation. `go vet` and `gofmt -l` are clean
on `arrow/array`.
### Are there any user-facing changes?
Yes: `ValidateFull` accepts run-end-encoded arrays produced by the
builder, `Concatenate` and C Data import. No API changes.
---
arrow/array/encoded_test.go | 93 +++++++++++++++++++++++++++++++++++++++++----
arrow/array/validate.go | 14 +++++++
2 files changed, 100 insertions(+), 7 deletions(-)
diff --git a/arrow/array/encoded_test.go b/arrow/array/encoded_test.go
index c8a5c4e2..8d02ed9e 100644
--- a/arrow/array/encoded_test.go
+++ b/arrow/array/encoded_test.go
@@ -22,6 +22,7 @@ import (
"github.com/apache/arrow-go/v18/arrow"
"github.com/apache/arrow-go/v18/arrow/array"
+ "github.com/apache/arrow-go/v18/arrow/extensions"
"github.com/apache/arrow-go/v18/arrow/memory"
"github.com/apache/arrow-go/v18/internal/json"
@@ -85,6 +86,55 @@ func TestRLEFromRunEndsAndValues(t *testing.T) {
})
}
+func TestRunEndEncodedValidateConstructedArrays(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ bldr := array.NewRunEndEncodedBuilder(mem, arrow.PrimitiveTypes.Int64,
arrow.PrimitiveTypes.Int64)
+ defer bldr.Release()
+
+ bldr.Append(2)
+ bldr.ValueBuilder().(*array.Int64Builder).Append(42)
+ bldr.AppendNull()
+ bldr.Append(3)
+ bldr.ValueBuilder().(*array.Int64Builder).Append(7)
+
+ built := bldr.NewArray().(*array.RunEndEncoded)
+ defer built.Release()
+ constructed := array.NewRunEndEncodedArray(built.RunEndsArr(),
built.Values(), built.Len(), 0)
+ defer constructed.Release()
+ concatenated, err := array.Concatenate([]arrow.Array{built,
constructed}, mem)
+ require.NoError(t, err)
+ defer concatenated.Release()
+ concatenatedWithPlaceholder, err :=
array.Concatenate([]arrow.Array{constructed, built}, mem)
+ require.NoError(t, err)
+ defer concatenatedWithPlaceholder.Release()
+ empty := bldr.NewArray()
+ defer empty.Release()
+ sliced := array.NewSlice(built, 1, 5)
+ defer sliced.Release()
+ ext :=
array.NewExtensionArrayWithStorage(extensions.NewOpaqueType(built.DataType(),
"ree", "test"), built)
+ defer ext.Release()
+
+ for _, tt := range []struct {
+ name string
+ arr arrow.Array
+ }{
+ {"builder", built},
+ {"constructor", constructed},
+ {"concatenate", concatenated},
+ {"concatenate with placeholder", concatenatedWithPlaceholder},
+ {"empty builder", empty},
+ {"sliced builder", sliced},
+ {"extension", ext},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ assert.NoError(t, array.Validate(tt.arr))
+ assert.NoError(t, array.ValidateFull(tt.arr))
+ })
+ }
+}
+
func TestRunEndEncodedValidate(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
defer mem.AssertSize(t, 0)
@@ -103,6 +153,33 @@ func TestRunEndEncodedValidate(t *testing.T) {
assert.NoError(t, arr.ValidateFull())
})
+ t.Run("invalid parent buffers", func(t *testing.T) {
+ runEnds, _, err := array.FromJSON(mem,
arrow.PrimitiveTypes.Int64, strings.NewReader(`[1, 3]`))
+ require.NoError(t, err)
+ defer runEnds.Release()
+ values, _, err := array.FromJSON(mem,
arrow.PrimitiveTypes.Int64, strings.NewReader(`[42, 7]`))
+ require.NoError(t, err)
+ defer values.Release()
+
+ for _, tt := range []struct {
+ name string
+ buffers []*memory.Buffer
+ wantErr string
+ }{
+ {"non-nil buffer",
[]*memory.Buffer{memory.NewBufferBytes([]byte{0xff})}, "must be nil"},
+ {"extra buffer", []*memory.Buffer{nil, nil}, "expected
at most 1 buffers"},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ arr := makeRunEndEncodedArrayRaw(t,
arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int64, arrow.PrimitiveTypes.Int64),
3, 0, 0,
+ tt.buffers,
[]arrow.ArrayData{runEnds.Data(), values.Data()})
+ defer arr.Release()
+
+ assert.ErrorContains(t, array.Validate(arr),
tt.wantErr)
+ assert.ErrorContains(t,
array.ValidateFull(arr), tt.wantErr)
+ })
+ }
+ })
+
t.Run("last run end shorter than logical length fails Validate", func(t
*testing.T) {
runEnds, _, _ := array.FromJSON(mem,
arrow.PrimitiveTypes.Int32, strings.NewReader(`[1, 2]`))
values, _, _ := array.FromJSON(mem, arrow.BinaryTypes.String,
strings.NewReader(`["a", "b"]`))
@@ -140,14 +217,16 @@ func TestRunEndEncodedValidate(t *testing.T) {
defer runEnds.Release()
defer values.Release()
- arr := makeRunEndEncodedArrayRaw(t,
arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int32, arrow.BinaryTypes.String), 2,
0, 0,
- []*memory.Buffer{nil},
[]arrow.ArrayData{runEnds.Data(), values.Data()})
- defer arr.Release()
+ for _, buffers := range [][]*memory.Buffer{nil, {nil}} {
+ arr := makeRunEndEncodedArrayRaw(t,
arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int32, arrow.BinaryTypes.String), 2,
0, 0,
+ buffers, []arrow.ArrayData{runEnds.Data(),
values.Data()})
+ defer arr.Release()
- assert.NoError(t, array.Validate(arr))
- err := array.ValidateFull(arr)
- require.Error(t, err)
- assert.Contains(t, err.Error(), "strictly greater")
+ assert.NoError(t, array.Validate(arr))
+ err := array.ValidateFull(arr)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "strictly greater")
+ }
})
t.Run("unknown null counts without bitmaps still validate", func(t
*testing.T) {
diff --git a/arrow/array/validate.go b/arrow/array/validate.go
index c1605d44..f627b6ee 100644
--- a/arrow/array/validate.go
+++ b/arrow/array/validate.go
@@ -37,6 +37,8 @@ type Validator interface {
// Validate performs a basic O(1) consistency check on arr, returning an error
// if the array's internal buffers or nested child data are inconsistent.
+// Run-end encoded arrays may omit the always-nil buffer placeholder used by
+// some constructors.
//
// Use this to detect corrupted data from untrusted sources such as Arrow
Flight
// or Flight SQL servers before accessing values, which may otherwise panic.
@@ -177,6 +179,18 @@ func validateArrayData(data *Data) error {
return fmt.Errorf("arrow/array: array offset and length
overflow")
}
+ // REE arrays have no physical buffers. Constructors and IPC may
include a
+ // nil placeholder, but builders and C Data imports omit it.
+ if len(data.buffers) == 0 {
+ dt := data.dtype
+ if ext, ok := dt.(arrow.ExtensionType); ok {
+ dt = ext.StorageType()
+ }
+ if dt.ID() == arrow.RUN_END_ENCODED {
+ return nil
+ }
+ }
+
layout := data.dtype.Layout()
// Union arrays reserve the first buffer slot for a validity bitmap,
even
// though that slot must be nil and is not part of their type layout.
