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 3a9323d2 fix(arrow): include REE value nullability in type equality
(#1158)
3a9323d2 is described below
commit 3a9323d25aa9b04cf3dc1987dcab1bc1d8a1dd2c
Author: Minh Vu <[email protected]>
AuthorDate: Wed Aug 26 20:29:46 2026 +0200
fix(arrow): include REE value nullability in type equality (#1158)
### Rationale for this change
TypeEqual compares run-end and value data types for run-end encoded
types, but it does not compare whether the value field is nullable.
Structurally different types can therefore compare equal and produce the
same type fingerprint.
### What changes are included in this PR?
Include ValueNullable in run-end encoded type equality and fingerprints.
### Are these changes tested?
Yes. The regression test verifies both comparison directions and
distinct fingerprints for nullable and non-nullable value fields. The
full arrow package suite passes.
### Are there any user-facing changes?
Run-end encoded types with different value nullability now compare
unequal and have different fingerprints.
---
arrow/array/builder.go | 2 +-
arrow/array/encoded.go | 16 +++++++-
arrow/array/encoded_test.go | 53 ++++++++++++++++++++++++++
arrow/cdata/cdata.go | 1 +
arrow/compare.go | 3 +-
arrow/compare_test.go | 16 ++++++++
arrow/datatype_encoded.go | 6 ++-
arrow/extensions/timestamp_with_offset.go | 8 ++--
arrow/extensions/timestamp_with_offset_test.go | 12 ++++++
arrow/internal/arrdata/arrdata.go | 16 +++++---
arrow/internal/arrjson/arrjson.go | 1 +
arrow/ipc/metadata.go | 6 ++-
12 files changed, 123 insertions(+), 17 deletions(-)
diff --git a/arrow/array/builder.go b/arrow/array/builder.go
index 18a799aa..11bffb01 100644
--- a/arrow/array/builder.go
+++ b/arrow/array/builder.go
@@ -451,7 +451,7 @@ func NewBuilder(mem memory.Allocator, dtype arrow.DataType)
Builder {
return NewDurationBuilder(mem, typ)
case arrow.RUN_END_ENCODED:
typ := dtype.(*arrow.RunEndEncodedType)
- return NewRunEndEncodedBuilder(mem, typ.RunEnds(),
typ.Encoded())
+ return newRunEndEncodedBuilder(mem, typ)
case arrow.BINARY_VIEW:
return NewBinaryViewBuilder(mem)
case arrow.STRING_VIEW:
diff --git a/arrow/array/encoded.go b/arrow/array/encoded.go
index bd9a6fb5..e3cf21fa 100644
--- a/arrow/array/encoded.go
+++ b/arrow/array/encoded.go
@@ -42,7 +42,15 @@ type RunEndEncoded struct {
}
func NewRunEndEncodedArray(runEnds, values arrow.Array, logicalLength, offset
int) *RunEndEncoded {
- data := NewData(arrow.RunEndEncodedOf(runEnds.DataType(),
values.DataType()), logicalLength,
+ return NewRunEndEncodedArrayWithType(
+ arrow.RunEndEncodedOf(runEnds.DataType(), values.DataType()),
+ runEnds, values, logicalLength, offset)
+}
+
+// NewRunEndEncodedArrayWithType constructs a run-end encoded array with the
+// provided type.
+func NewRunEndEncodedArrayWithType(dt *arrow.RunEndEncodedType, runEnds,
values arrow.Array, logicalLength, offset int) *RunEndEncoded {
+ data := NewData(dt, logicalLength,
[]*memory.Buffer{nil}, []arrow.ArrayData{runEnds.Data(),
values.Data()}, 0, offset)
defer data.Release()
return NewRunEndEncodedData(data)
@@ -399,7 +407,11 @@ type RunEndEncodedBuilder struct {
}
func NewRunEndEncodedBuilder(mem memory.Allocator, runEnds, encoded
arrow.DataType) *RunEndEncodedBuilder {
- dt := arrow.RunEndEncodedOf(runEnds, encoded)
+ return newRunEndEncodedBuilder(mem, arrow.RunEndEncodedOf(runEnds,
encoded))
+}
+
+func newRunEndEncodedBuilder(mem memory.Allocator, dt
*arrow.RunEndEncodedType) *RunEndEncodedBuilder {
+ runEnds, encoded := dt.RunEnds(), dt.Encoded()
if !dt.ValidRunEndsType(runEnds) {
panic("arrow/ree: invalid runEnds type for run length encoded
array")
}
diff --git a/arrow/array/encoded_test.go b/arrow/array/encoded_test.go
index 512adf85..3370d0f1 100644
--- a/arrow/array/encoded_test.go
+++ b/arrow/array/encoded_test.go
@@ -456,6 +456,59 @@ func TestRunEndEncodedBuilderDictionaryEmptyValue(t
*testing.T) {
assert.Equal(t, "", arr.GetOneForMarshal(0))
}
+func TestRunEndEncodedBuilderPreservesValueNullability(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ dt := arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int16,
arrow.BinaryTypes.String)
+ dt.ValueNullable = false
+ bldr := array.NewBuilder(mem, dt)
+ defer bldr.Release()
+
+ assert.False(t, bldr.Type().(*arrow.RunEndEncodedType).ValueNullable)
+ arr := bldr.NewArray()
+ defer arr.Release()
+ assert.False(t, arr.DataType().(*arrow.RunEndEncodedType).ValueNullable)
+}
+
+func TestRunEndEncodedArrayWithTypePreservesValueNullability(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ dt := arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int16,
arrow.PrimitiveTypes.Int32)
+ dt.ValueNullable = false
+
+ newArray := func(runEndsJSON, valuesJSON string) *array.RunEndEncoded {
+ runEnds, _, err := array.FromJSON(mem,
arrow.PrimitiveTypes.Int16, strings.NewReader(runEndsJSON))
+ require.NoError(t, err)
+ values, _, err := array.FromJSON(mem,
arrow.PrimitiveTypes.Int32, strings.NewReader(valuesJSON))
+ require.NoError(t, err)
+ defer runEnds.Release()
+ defer values.Release()
+
+ return array.NewRunEndEncodedArrayWithType(dt, runEnds, values,
2, 0)
+ }
+
+ first := newArray(`[1, 2]`, `[10, 20]`)
+ defer first.Release()
+ second := newArray(`[1, 2]`, `[30, 40]`)
+ defer second.Release()
+
+ schema := arrow.NewSchema([]arrow.Field{{Name: "values", Type: dt}},
nil)
+ record := array.NewRecordBatch(schema, []arrow.Array{first}, -1)
+ defer record.Release()
+
+ assert.False(t,
first.DataType().(*arrow.RunEndEncodedType).ValueNullable)
+
+ concatenated, err := array.Concatenate([]arrow.Array{first, second},
mem)
+ require.NoError(t, err)
+ defer concatenated.Release()
+
+ assert.True(t, arrow.TypeEqual(dt, concatenated.DataType()))
+ assert.False(t,
concatenated.DataType().(*arrow.RunEndEncodedType).ValueNullable)
+ assert.NoError(t, array.ValidateFull(concatenated))
+}
+
func TestRunEndEncodedStringRoundTrip(t *testing.T) {
// 1. create array
mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
diff --git a/arrow/cdata/cdata.go b/arrow/cdata/cdata.go
index 9fbdd008..161440e4 100644
--- a/arrow/cdata/cdata.go
+++ b/arrow/cdata/cdata.go
@@ -347,6 +347,7 @@ func importSchema(schema *CArrowSchema) (ret arrow.Field,
err error) {
return ret, fmt.Errorf("%w: run-end encoded
arrays must have 2 children", arrow.ErrInvalid)
}
dt = arrow.RunEndEncodedOf(childFields[0].Type,
childFields[1].Type)
+ dt.(*arrow.RunEndEncodedType).ValueNullable =
childFields[1].Nullable
case 'm': // map type is basically a list of structs.
if f != "+m" {
return ret, fmt.Errorf("%w: invalid map type
format %q", arrow.ErrInvalid, f)
diff --git a/arrow/compare.go b/arrow/compare.go
index abf5ac83..997d25ad 100644
--- a/arrow/compare.go
+++ b/arrow/compare.go
@@ -150,7 +150,8 @@ func TypeEqual(left, right DataType, opts
...TypeEqualOption) bool {
case *RunEndEncodedType:
r := right.(*RunEndEncodedType)
return TypeEqual(l.Encoded(), r.Encoded(), opts...) &&
- TypeEqual(l.runEnds, r.runEnds, opts...)
+ TypeEqual(l.runEnds, r.runEnds, opts...) &&
+ l.ValueNullable == r.ValueNullable
case *ListViewType:
return l.elem.Equal(right.(*ListViewType).elem)
default:
diff --git a/arrow/compare_test.go b/arrow/compare_test.go
index 84f005d3..c78eb457 100644
--- a/arrow/compare_test.go
+++ b/arrow/compare_test.go
@@ -411,3 +411,19 @@ func TestTypeEqual(t *testing.T) {
})
}
}
+
+func TestTypeEqualRunEndEncodedValueNullability(t *testing.T) {
+ nullable := RunEndEncodedOf(PrimitiveTypes.Int16, BinaryTypes.String)
+ nonNullable := RunEndEncodedOf(PrimitiveTypes.Int16, BinaryTypes.String)
+ nonNullable.ValueNullable = false
+
+ if TypeEqual(nullable, nonNullable) {
+ t.Fatal("run-end encoded types with different value nullability
compared equal")
+ }
+ if TypeEqual(nonNullable, nullable) {
+ t.Fatal("run-end encoded type equality was not symmetric")
+ }
+ if nullable.Fingerprint() == nonNullable.Fingerprint() {
+ t.Fatal("run-end encoded types with different value nullability
had identical fingerprints")
+ }
+}
diff --git a/arrow/datatype_encoded.go b/arrow/datatype_encoded.go
index 749f03a5..3eb7e83c 100644
--- a/arrow/datatype_encoded.go
+++ b/arrow/datatype_encoded.go
@@ -45,7 +45,11 @@ func (t *RunEndEncodedType) String() string {
}
func (t *RunEndEncodedType) Fingerprint() string {
- return typeFingerprint(t) + "{" + t.runEnds.Fingerprint() + ";" +
t.values.Fingerprint() + ";}"
+ nullability := "N"
+ if t.ValueNullable {
+ nullability = "n"
+ }
+ return typeFingerprint(t) + "{" + t.runEnds.Fingerprint() + ";" +
t.values.Fingerprint() + ";" + nullability + ";}"
}
func (t *RunEndEncodedType) RunEnds() DataType { return t.runEnds }
diff --git a/arrow/extensions/timestamp_with_offset.go
b/arrow/extensions/timestamp_with_offset.go
index 257707a0..77143a14 100644
--- a/arrow/extensions/timestamp_with_offset.go
+++ b/arrow/extensions/timestamp_with_offset.go
@@ -44,12 +44,9 @@ func isOffsetTypeOk(offsetType arrow.DataType) bool {
case *arrow.DictionaryType:
return arrow.TypeEqual(offsetType.ValueType,
arrow.PrimitiveTypes.Int16)
case *arrow.RunEndEncodedType:
- return offsetType.ValidRunEndsType(offsetType.RunEnds()) &&
+ return !offsetType.ValueNullable &&
+ offsetType.ValidRunEndsType(offsetType.RunEnds()) &&
arrow.TypeEqual(offsetType.Encoded(),
arrow.PrimitiveTypes.Int16)
- // FIXME: Technically this should be non-nullable, but a Arrow
IPC does not deserialize
- // ValueNullable properly, so enforcing this here would always
fail when reading from an IPC
- // stream
- // !offsetType.ValueNullable
default:
return false
}
@@ -153,6 +150,7 @@ func NewTimestampWithOffsetTypeDictionaryEncoded[I
DictIndexType](unit arrow.Tim
// valid run-ends type.
func NewTimestampWithOffsetTypeRunEndEncoded[E RunEndsType](unit
arrow.TimeUnit, runEnds E) *TimestampWithOffsetType {
offsetType := arrow.RunEndEncodedOf(arrow.DataType(runEnds),
arrow.PrimitiveTypes.Int16)
+ offsetType.ValueNullable = false
v, _ := NewTimestampWithOffsetTypeCustomOffset(unit, offsetType)
// SAFETY: This should never error as RunEndsType always a valid run
ends type
diff --git a/arrow/extensions/timestamp_with_offset_test.go
b/arrow/extensions/timestamp_with_offset_test.go
index 26221dc9..b45a706e 100644
--- a/arrow/extensions/timestamp_with_offset_test.go
+++ b/arrow/extensions/timestamp_with_offset_test.go
@@ -133,6 +133,7 @@ func TestTimestampWithOffsetTypeDeserializeInvalidStorage(t
*testing.T) {
badDict := &arrow.DictionaryType{IndexType: arrow.PrimitiveTypes.Int8,
ValueType: arrow.PrimitiveTypes.Int32}
badREE := arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int32,
arrow.PrimitiveTypes.Int32)
+ nullableREE := arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int16,
arrow.PrimitiveTypes.Int16)
valid, err := base.Deserialize(base.StorageType(), "")
require.NoError(t, err)
@@ -152,6 +153,7 @@ func TestTimestampWithOffsetTypeDeserializeInvalidStorage(t
*testing.T) {
"offset nullable": arrow.StructOf(tsField,
arrow.Field{Name: "offset_minutes", Type: arrow.PrimitiveTypes.Int16, Nullable:
true}),
"offset dict value not int16": arrow.StructOf(tsField,
arrow.Field{Name: "offset_minutes", Type: badDict}),
"offset ree encoded not int16": arrow.StructOf(tsField,
arrow.Field{Name: "offset_minutes", Type: badREE}),
+ "offset ree values nullable": arrow.StructOf(tsField,
arrow.Field{Name: "offset_minutes", Type: nullableREE}),
"fields swapped": arrow.StructOf(offField,
tsField),
}
@@ -163,6 +165,12 @@ func
TestTimestampWithOffsetTypeDeserializeInvalidStorage(t *testing.T) {
}
}
+func TestTimestampWithOffsetTypeRejectsNullableRunEndEncodedOffset(t
*testing.T) {
+ offsetType := arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int16,
arrow.PrimitiveTypes.Int16)
+ _, err :=
extensions.NewTimestampWithOffsetTypeCustomOffset(testTimeUnit, offsetType)
+ assert.Error(t, err)
+}
+
func assertDictBasics[I extensions.DictIndexType](t *testing.T, indexType I) {
typ :=
extensions.NewTimestampWithOffsetTypeDictionaryEncoded(testTimeUnit, indexType)
@@ -685,6 +693,10 @@ func TestTimestampWithOffsetTypeBatchIPCRoundTrip(t
*testing.T) {
assert.Truef(t, batch.Schema().Equal(written.Schema()),
"expected: %s\n\ngot: %s",
batch.Schema(), written.Schema())
+ if _, ok := offsetType.(*arrow.RunEndEncodedType); ok {
+ writtenType :=
written.Schema().Field(0).Type.(*extensions.TimestampWithOffsetType)
+ assert.False(t,
writtenType.OffsetType().(*arrow.RunEndEncodedType).ValueNullable)
+ }
assert.Truef(t, array.RecordEqual(batch, written),
"expected: %s\n\ngot: %s",
batch, written)
diff --git a/arrow/internal/arrdata/arrdata.go
b/arrow/internal/arrdata/arrdata.go
index 095571a8..b4f0e626 100644
--- a/arrow/internal/arrdata/arrdata.go
+++ b/arrow/internal/arrdata/arrdata.go
@@ -1107,20 +1107,20 @@ func makeUnionRecords() []arrow.RecordBatch {
func makeRunEndEncodedRecords() []arrow.RecordBatch {
mem := memory.NewGoAllocator()
+ ree32Type := arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int32,
arrow.PrimitiveTypes.Int32)
+ ree32Type.ValueNullable = false
schema := arrow.NewSchema([]arrow.Field{
{Name: "ree16", Type:
arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int16, arrow.BinaryTypes.String)},
- {Name: "ree32", Type:
arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int32, arrow.PrimitiveTypes.Int32)},
+ {Name: "ree32", Type: ree32Type},
{Name: "ree64", Type:
arrow.RunEndEncodedOf(arrow.PrimitiveTypes.Int64, arrow.BinaryTypes.Binary)},
}, nil)
-
- schema.Field(1).Type.(*arrow.RunEndEncodedType).ValueNullable = false
isValid := []bool{true, false, true, false, true}
chunks := [][]arrow.Array{
{
runEndEncodedOf(
arrayOf(mem, []int16{5, 10, 20, 1020, 1120},
nil),
arrayOf(mem, []string{"foo", "bar", "baz",
"foo", ""}, isValid), 1100, 20),
- runEndEncodedOf(
+ runEndEncodedOfType(ree32Type,
arrayOf(mem, []int32{100, 200, 800, 1000,
1100}, nil),
arrayOf(mem, []int32{-1, -2, -3, -4, -5}, nil),
1100, 0),
runEndEncodedOf(
@@ -1131,7 +1131,7 @@ func makeRunEndEncodedRecords() []arrow.RecordBatch {
runEndEncodedOf(
arrayOf(mem, []int16{110, 160, 170, 1070,
1120}, nil),
arrayOf(mem, []string{"super", "dee", "",
"duper", "doo"}, isValid), 1100, 20),
- runEndEncodedOf(
+ runEndEncodedOfType(ree32Type,
arrayOf(mem, []int32{100, 120, 710, 810, 1100},
nil),
arrayOf(mem, []int32{-1, -2, -3, -4, -5}, nil),
1100, 0),
runEndEncodedOf(
@@ -1664,6 +1664,12 @@ func runEndEncodedOf(runEnds, values arrow.Array,
logicalLen, offset int) arrow.
return array.NewRunEndEncodedArray(runEnds, values, logicalLen, offset)
}
+func runEndEncodedOfType(dt *arrow.RunEndEncodedType, runEnds, values
arrow.Array, logicalLen, offset int) arrow.Array {
+ defer runEnds.Release()
+ defer values.Release()
+ return array.NewRunEndEncodedArrayWithType(dt, runEnds, values,
logicalLen, offset)
+}
+
func buildArray(bldr array.Builder, data arrow.Array) {
defer data.Release()
diff --git a/arrow/internal/arrjson/arrjson.go
b/arrow/internal/arrjson/arrjson.go
index 864356d7..afd34f50 100644
--- a/arrow/internal/arrjson/arrjson.go
+++ b/arrow/internal/arrjson/arrjson.go
@@ -541,6 +541,7 @@ func typeFromJSON(typ json.RawMessage, children
[]FieldWrapper) (arrowType arrow
return
}
arrowType = arrow.RunEndEncodedOf(children[0].arrowType,
children[1].arrowType)
+ arrowType.(*arrow.RunEndEncodedType).ValueNullable =
children[1].Nullable
}
if arrowType == nil {
diff --git a/arrow/ipc/metadata.go b/arrow/ipc/metadata.go
index 54cde676..7a3a2321 100644
--- a/arrow/ipc/metadata.go
+++ b/arrow/ipc/metadata.go
@@ -482,7 +482,7 @@ func (fv *fieldVisitor) visit(field arrow.Field) {
offsets[0] = fieldToFB(fv.b, fv.pos.Child(0),
arrow.Field{Name: "run_ends", Type: dt.RunEnds()},
fv.memo)
offsets[1] = fieldToFB(fv.b, fv.pos.Child(1),
- arrow.Field{Name: "values", Type: dt.Encoded(),
Nullable: true}, fv.memo)
+ arrow.Field{Name: "values", Type: dt.Encoded(),
Nullable: dt.ValueNullable}, fv.memo)
flatbuf.RunEndEncodedStart(fv.b)
fv.b.PrependUOffsetT(offsets[1])
fv.b.PrependUOffsetT(offsets[0])
@@ -889,7 +889,9 @@ func concreteTypeFromFB(typ flatbuf.Type, data
flatbuffers.Table, children []arr
default:
return nil, fmt.Errorf("%w: arrow/ipc: run-end encoded
run_ends field must be one of int16, int32, or int64 type", arrow.ErrInvalid)
}
- return arrow.RunEndEncodedOf(children[0].Type,
children[1].Type), nil
+ ret := arrow.RunEndEncodedOf(children[0].Type, children[1].Type)
+ ret.ValueNullable = children[1].Nullable
+ return ret, nil
default:
panic(fmt.Errorf("arrow/ipc: type %v not implemented",
flatbuf.EnumNamesType[typ]))
