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 d24b83eb fix(arrow/extensions): reject Null Variant typed_value (#1243)
d24b83eb is described below
commit d24b83ebd28d2bb2568d0a2ea2e1f10d0240694b
Author: Digvijay <[email protected]>
AuthorDate: Wed Sep 2 13:27:16 2026 -0500
fix(arrow/extensions): reject Null Variant typed_value (#1243)
### Rationale for this change
A Null `typed_value` column is not a valid shredded Variant type. Nulls
are encoded in the `value` column as Variant null (`00`). A dedicated
Null `typed_value` would also make “field present and null”
indistinguishable from “field missing” for shredded objects.
See apache/arrow#50622 and apache/arrow#50810.
Fixes #1205
### What changes are included in this PR?
- `NewVariantType` rejects a Null `typed_value` field
- Nested shredded fields with a Null `typed_value` are also rejected
### Are these changes tested?
- `go test ./arrow/extensions`
### Are there any user-facing changes?
Yes — constructing a Variant extension type with a Null `typed_value`
now returns `arrow.ErrInvalid`.
---------
Signed-off-by: Digvijay <[email protected]>
---
arrow/extensions/variant.go | 43 +++++++++++++++++++++++++------
arrow/extensions/variant_test.go | 55 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 8 deletions(-)
diff --git a/arrow/extensions/variant.go b/arrow/extensions/variant.go
index a0cc74ed..5222d5fb 100644
--- a/arrow/extensions/variant.go
+++ b/arrow/extensions/variant.go
@@ -122,10 +122,10 @@ func createShreddedField(dt arrow.DataType)
arrow.DataType {
// ), Nullable: true})
//
// This is intended to be a convenient way to create a shredded variant type
from a definition
-// of the fields to shred. If the provided data type is nil, it will create a
default
-// variant type.
+// of the fields to shred. If the provided data type is nil or Null (including
an
+// extension whose storage is Null), it will create a default variant type.
func NewShreddedVariantType(dt arrow.DataType) *VariantType {
- if dt == nil {
+ if dt == nil || isNullType(dt) {
return NewDefaultVariantType()
}
@@ -220,9 +220,12 @@ func NewVariantType(storage arrow.DataType) (*VariantType,
error) {
return nil, fmt.Errorf("%w: typed_value field must be nullable,
got %s", arrow.ErrInvalid, typedValueField.Type)
}
- dt := typedValueField.Type
- if dt.ID() == arrow.EXTENSION {
- dt = dt.(arrow.ExtensionType).StorageType()
+ dt := storageType(typedValueField.Type)
+ if dt == nil {
+ return nil, fmt.Errorf("%w: typed_value field has invalid
storage type", arrow.ErrInvalid)
+ }
+ if dt.ID() == arrow.NULL {
+ return nil, fmt.Errorf("%w: typed_value field must not be null
type", arrow.ErrInvalid)
}
if nt, ok := dt.(arrow.NestedType); ok {
@@ -291,11 +294,35 @@ func isBinary(dt arrow.DataType) bool {
dt.ID() == arrow.BINARY_VIEW
}
+func storageType(dt arrow.DataType) arrow.DataType {
+ seen := make(map[arrow.DataType]struct{})
+ for dt != nil {
+ ext, ok := dt.(arrow.ExtensionType)
+ if !ok {
+ return dt
+ }
+ if _, dup := seen[dt]; dup {
+ return nil
+ }
+ seen[dt] = struct{}{}
+ dt = ext.StorageType()
+ }
+ return nil
+}
+
+func isNullType(dt arrow.DataType) bool {
+ st := storageType(dt)
+ return st == nil || st.ID() == arrow.NULL
+}
+
func validStruct(s *arrow.StructType) bool {
switch s.NumFields() {
case 1:
f := s.Field(0)
- return (f.Name == "value" && isBinary(f.Type)) || f.Name ==
"typed_value"
+ if f.Name == "value" {
+ return isBinary(f.Type)
+ }
+ return f.Name == "typed_value" && !isNullType(f.Type)
case 2:
valField, ok := s.FieldByName("value")
if !ok || !valField.Nullable || !isBinary(valField.Type) {
@@ -311,7 +338,7 @@ func validStruct(s *arrow.StructType) bool {
return validNestedType(nt)
}
- return true
+ return !isNullType(typedField.Type)
default:
return false
}
diff --git a/arrow/extensions/variant_test.go b/arrow/extensions/variant_test.go
index a39fd513..905fc5d9 100644
--- a/arrow/extensions/variant_test.go
+++ b/arrow/extensions/variant_test.go
@@ -83,6 +83,27 @@ func TestVariantExtensionType(t *testing.T) {
arrow.Field{Name: "metadata", Type:
arrow.BinaryTypes.String, Nullable: false},
arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: false}),
"metadata field must be non-nullable binary type, got
utf8"},
+ {arrow.StructOf(
+ arrow.Field{Name: "metadata", Type:
arrow.BinaryTypes.Binary, Nullable: false},
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type: arrow.Null,
Nullable: true}),
+ "typed_value field must not be null type"},
+ {arrow.StructOf(
+ arrow.Field{Name: "metadata", Type:
arrow.BinaryTypes.Binary, Nullable: false},
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(arrow.Null, "null", "test"), Nullable: true}),
+ "typed_value field must not be null type"},
+ {arrow.StructOf(
+ arrow.Field{Name: "metadata", Type:
arrow.BinaryTypes.Binary, Nullable: false},
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(
+ extensions.NewOpaqueType(arrow.Null, "null",
"test"), "null", "test"), Nullable: true}),
+ "typed_value field must not be null type"},
+ {arrow.StructOf(
+ arrow.Field{Name: "metadata", Type:
arrow.BinaryTypes.Binary, Nullable: false},
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(nil, "null", "test"), Nullable: true}),
+ "typed_value field has invalid storage type"},
}
for _, tt := range tests {
@@ -113,6 +134,31 @@ func TestVariantExtensionBadNestedTypes(t *testing.T) {
), Nullable: false})},
{"empty struct elem", arrow.StructOf(
arrow.Field{Name: "foobar", Type: arrow.StructOf(),
Nullable: false})},
+ {"null typed_value in shredded field", arrow.StructOf(
+ arrow.Field{Name: "foobar", Type: arrow.StructOf(
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type:
arrow.Null, Nullable: true},
+ ), Nullable: false})},
+ {"null typed_value extension in one-field shredded field",
arrow.StructOf(
+ arrow.Field{Name: "foobar", Type: arrow.StructOf(
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(arrow.Null, "null", "test"), Nullable: true},
+ ), Nullable: false})},
+ {"null typed_value extension in two-field shredded field",
arrow.StructOf(
+ arrow.Field{Name: "foobar", Type: arrow.StructOf(
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(arrow.Null, "null", "test"), Nullable: true},
+ ), Nullable: false})},
+ {"double-wrapped null typed_value in one-field shredded field",
arrow.StructOf(
+ arrow.Field{Name: "foobar", Type: arrow.StructOf(
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(
+ extensions.NewOpaqueType(arrow.Null,
"null", "test"), "null", "test"), Nullable: true},
+ ), Nullable: false})},
+ {"double-wrapped null typed_value in two-field shredded field",
arrow.StructOf(
+ arrow.Field{Name: "foobar", Type: arrow.StructOf(
+ arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
+ arrow.Field{Name: "typed_value", Type:
extensions.NewOpaqueType(
+ extensions.NewOpaqueType(arrow.Null,
"null", "test"), "null", "test"), Nullable: true},
+ ), Nullable: false})},
{"non-nullable two elem struct", arrow.StructOf(
arrow.Field{Name: "foobar", Type: arrow.StructOf(
arrow.Field{Name: "value", Type:
arrow.BinaryTypes.Binary, Nullable: true},
@@ -1560,6 +1606,15 @@ func TestVariantBuilderUnmarshalJSON(t *testing.T) {
func TestNewSimpleShreddedVariantType(t *testing.T) {
assert.True(t, arrow.TypeEqual(extensions.NewDefaultVariantType(),
extensions.NewShreddedVariantType(nil)))
+ assert.True(t, arrow.TypeEqual(extensions.NewDefaultVariantType(),
+ extensions.NewShreddedVariantType(arrow.Null)))
+ assert.True(t, arrow.TypeEqual(extensions.NewDefaultVariantType(),
+
extensions.NewShreddedVariantType(extensions.NewOpaqueType(arrow.Null, "null",
"test"))))
+ assert.True(t, arrow.TypeEqual(extensions.NewDefaultVariantType(),
+ extensions.NewShreddedVariantType(extensions.NewOpaqueType(
+ extensions.NewOpaqueType(arrow.Null, "null", "test"),
"null", "test"))))
+ assert.True(t, arrow.TypeEqual(extensions.NewDefaultVariantType(),
+ extensions.NewShreddedVariantType(extensions.NewOpaqueType(nil,
"null", "test"))))
vt := extensions.NewShreddedVariantType(arrow.PrimitiveTypes.Float32)
s := arrow.StructOf(
