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 5b999225 fix(arrow/array): widen dictionary index bounds checks (#1129)
5b999225 is described below
commit 5b999225fa9405ee412c22f674320410383386e7
Author: Minh Vu <[email protected]>
AuthorDate: Tue Aug 11 17:24:01 2026 +0200
fix(arrow/array): widen dictionary index bounds checks (#1129)
### Rationale for this change
Signed dictionary index validation converts the dictionary length to the
index type before comparing it with the largest index. A dictionary with
128 entries therefore turns the upper bound into -128 for int8 and
rejects valid indices.
### What changes are included in this PR?
Compare signed index values after widening them to uint64. Add coverage
for a valid int8 index at the dictionary size boundary.
### Are these changes tested?
- `go test ./arrow/array`
### Are there any user-facing changes?
Valid signed dictionary indices at the index type boundary are now
accepted. Negative and out-of-bounds indices continue to return errors.
---
arrow/array/dictionary.go | 8 ++---
arrow/array/dictionary_internal_test.go | 55 +++++++++++++++++++++++++++++++++
arrow/array/dictionary_test.go | 25 +++++++++++++++
3 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/arrow/array/dictionary.go b/arrow/array/dictionary.go
index 38a43f77..435460e9 100644
--- a/arrow/array/dictionary.go
+++ b/arrow/array/dictionary.go
@@ -110,7 +110,7 @@ func checkIndexBounds(indices *Data, upperlimit uint64)
error {
case arrow.INT8:
data :=
arrow.Int8Traits.CastFromBytes(indices.buffers[1].Bytes())
min, max := utils.GetMinMaxInt8(data[start:end])
- if min < 0 || max >= int8(upperlimit) {
+ if min < 0 || uint64(max) >= upperlimit {
return fmt.Errorf("contains out of bounds index: min:
%d, max: %d", min, max)
}
case arrow.UINT8:
@@ -122,7 +122,7 @@ func checkIndexBounds(indices *Data, upperlimit uint64)
error {
case arrow.INT16:
data :=
arrow.Int16Traits.CastFromBytes(indices.buffers[1].Bytes())
min, max := utils.GetMinMaxInt16(data[start:end])
- if min < 0 || max >= int16(upperlimit) {
+ if min < 0 || uint64(max) >= upperlimit {
return fmt.Errorf("contains out of bounds index: min:
%d, max: %d", min, max)
}
case arrow.UINT16:
@@ -134,7 +134,7 @@ func checkIndexBounds(indices *Data, upperlimit uint64)
error {
case arrow.INT32:
data :=
arrow.Int32Traits.CastFromBytes(indices.buffers[1].Bytes())
min, max := utils.GetMinMaxInt32(data[start:end])
- if min < 0 || max >= int32(upperlimit) {
+ if min < 0 || uint64(max) >= upperlimit {
return fmt.Errorf("contains out of bounds index: min:
%d, max: %d", min, max)
}
case arrow.UINT32:
@@ -146,7 +146,7 @@ func checkIndexBounds(indices *Data, upperlimit uint64)
error {
case arrow.INT64:
data :=
arrow.Int64Traits.CastFromBytes(indices.buffers[1].Bytes())
min, max := utils.GetMinMaxInt64(data[start:end])
- if min < 0 || max >= int64(upperlimit) {
+ if min < 0 || uint64(max) >= upperlimit {
return fmt.Errorf("contains out of bounds index: min:
%d, max: %d", min, max)
}
case arrow.UINT64:
diff --git a/arrow/array/dictionary_internal_test.go
b/arrow/array/dictionary_internal_test.go
new file mode 100644
index 00000000..5c51a18e
--- /dev/null
+++ b/arrow/array/dictionary_internal_test.go
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package array
+
+import (
+ "math"
+ "testing"
+
+ "github.com/apache/arrow-go/v18/arrow"
+ "github.com/apache/arrow-go/v18/arrow/memory"
+ "github.com/stretchr/testify/require"
+)
+
+func TestCheckIndexBoundsAllowsSignedIndexAtTypeLimit(t *testing.T) {
+ tests := []struct {
+ name string
+ indexType arrow.DataType
+ indexBytes []byte
+ upperLimit uint64
+ }{
+ {"int8", arrow.PrimitiveTypes.Int8,
+ arrow.Int8Traits.CastToBytes([]int8{math.MaxInt8}),
uint64(math.MaxInt8) + 1},
+ {"int16", arrow.PrimitiveTypes.Int16,
+ arrow.Int16Traits.CastToBytes([]int16{math.MaxInt16}),
uint64(math.MaxInt16) + 1},
+ {"int32", arrow.PrimitiveTypes.Int32,
+ arrow.Int32Traits.CastToBytes([]int32{math.MaxInt32}),
uint64(math.MaxInt32) + 1},
+ {"int64", arrow.PrimitiveTypes.Int64,
+ arrow.Int64Traits.CastToBytes([]int64{math.MaxInt64}),
uint64(math.MaxInt64) + 1},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ values := memory.NewBufferBytes(tt.indexBytes)
+ indices := NewData(tt.indexType, 1,
[]*memory.Buffer{nil, values}, nil, 0, 0)
+ values.Release()
+ defer indices.Release()
+
+ require.NoError(t, checkIndexBounds(indices,
tt.upperLimit))
+ })
+ }
+}
diff --git a/arrow/array/dictionary_test.go b/arrow/array/dictionary_test.go
index 24aab674..90f3c0bd 100644
--- a/arrow/array/dictionary_test.go
+++ b/arrow/array/dictionary_test.go
@@ -1207,6 +1207,31 @@ func TestDictionaryFromArrays(t *testing.T) {
}
}
+func TestValidatedDictionaryAllowsSignedIndexAtTypeLimit(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ indicesBuilder := array.NewInt8Builder(mem)
+ indicesBuilder.Append(127)
+ indices := indicesBuilder.NewArray()
+ indicesBuilder.Release()
+ defer indices.Release()
+
+ dictBuilder := array.NewStringBuilder(mem)
+ for i := 0; i < 128; i++ {
+ dictBuilder.AppendString(fmt.Sprintf("value-%d", i))
+ }
+ dict := dictBuilder.NewArray()
+ dictBuilder.Release()
+ defer dict.Release()
+
+ dictType := &arrow.DictionaryType{IndexType: arrow.PrimitiveTypes.Int8,
ValueType: arrow.BinaryTypes.String}
+ result, err := array.NewValidatedDictionaryArray(dictType, indices,
dict)
+ require.NoError(t, err)
+ require.NotNil(t, result)
+ defer result.Release()
+}
+
func TestListOfDictionary(t *testing.T) {
mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
defer mem.AssertSize(t, 0)
