Repository: parquet-go/parquet-go
Pull request #51 (2023-08-06 09:15:12)

Fix panic reading/writing `any` types
Fixes a few edge cases where conversions to and/or from `any` types would panic due to boxed reflect.Value types. I've added a unit test to run through the examples I've come across and plugged all the panics from those, but I'm fairly confident there will be other (even more edgier) cases lurking in there.


Patch:
diff --git a/node.go b/node.go
index e62eabd..d9e19ce 100644
--- a/node.go
+++ b/node.go
@@ -292,6 +292,14 @@ type groupField struct {
 func (f *groupField) Name() string { return f.name }
 
 func (f *groupField) Value(base reflect.Value) reflect.Value {
+	if base.Kind() == reflect.Interface {
+		if base.IsNil() {
+			return reflect.ValueOf(nil)
+		}
+		if base = base.Elem(); base.Kind() == reflect.Pointer && base.IsNil() {
+			return reflect.ValueOf(nil)
+		}
+	}
 	return base.MapIndex(reflect.ValueOf(&f.name).Elem())
 }
 
diff --git a/row.go b/row.go
index df2e8e9..c101d9d 100644
--- a/row.go
+++ b/row.go
@@ -444,6 +444,10 @@ func deconstructFuncOfOptional(columnIndex int16, node Node) (int16, deconstruct
 func deconstructFuncOfRepeated(columnIndex int16, node Node) (int16, deconstructFunc) {
 	columnIndex, deconstruct := deconstructFuncOf(columnIndex, Required(node))
 	return columnIndex, func(columns [][]Value, levels levels, value reflect.Value) {
+		if value.Kind() == reflect.Interface {
+			value = value.Elem()
+		}
+
 		if !value.IsValid() || value.Len() == 0 {
 			deconstruct(columns, levels, reflect.Value{})
 			return
diff --git a/value.go b/value.go
index ba11e0a..a48f434 100644
--- a/value.go
+++ b/value.go
@@ -253,6 +253,15 @@ func ByteArrayValue(value []byte) Value { return makeValueBytes(ByteArray, value
 func FixedLenByteArrayValue(value []byte) Value { return makeValueBytes(FixedLenByteArray, value) }
 
 func makeValue(k Kind, lt *format.LogicalType, v reflect.Value) Value {
+	if v.Kind() == reflect.Interface {
+		if v.IsNil() {
+			return Value{}
+		}
+		if v = v.Elem(); v.Kind() == reflect.Pointer && v.IsNil() {
+			return Value{}
+		}
+	}
+
 	switch v.Type() {
 	case reflect.TypeOf(time.Time{}):
 		unit := Nanosecond.TimeUnit()


Test patch:
diff --git a/parquet_test.go b/parquet_test.go
index 63acb26..1180647 100644
--- a/parquet_test.go
+++ b/parquet_test.go
@@ -1026,3 +1026,223 @@ var quickCheckConfig = quick.Config{
 func quickCheck(f interface{}) error {
 	return quickCheckConfig.Check(f)
 }
+
+func TestParquetAnyValueConversions(t *testing.T) {
+	// This test runs conversions to/from any values with edge case schemas.
+
+	type obj = map[string]any
+	type arr = []any
+
+	for _, test := range []struct {
+		name           string
+		input          any
+		explicitOutput any // Only set if we expect a difference from the input
+		schema         parquet.Group
+	}{
+		{
+			name: "simple strings",
+			input: obj{
+				"A": "foo",
+				"B": "bar",
+				"C": "baz",
+			},
+			schema: parquet.Group{
+				"A": parquet.String(),
+				"B": parquet.String(),
+				"C": parquet.String(),
+			},
+		},
+		{
+			name: "simple strings with nil",
+			input: obj{
+				"A": "foo",
+				"B": (*string)(nil),
+				"C": nil,
+			},
+			explicitOutput: obj{
+				"A": "foo",
+				"B": "",
+				"C": "",
+			},
+			schema: parquet.Group{
+				"A": parquet.String(),
+				"B": parquet.String(),
+				"C": parquet.String(),
+			},
+		},
+		{
+			name: "simple groups with nil",
+			input: obj{
+				"A": obj{
+					"AA": "foo",
+				},
+				"B": nil,
+				"C": (*obj)(nil),
+				"D": obj{
+					"DA": "bar",
+				},
+			},
+			explicitOutput: obj{
+				"A": obj{
+					"AA": "foo",
+				},
+				"B": obj{
+					"BA": "",
+				},
+				"C": obj{
+					"CA": "",
+				},
+				"D": obj{
+					"DA": "bar",
+				},
+			},
+			schema: parquet.Group{
+				"A": parquet.Group{
+					"AA": parquet.String(),
+				},
+				"B": parquet.Group{
+					"BA": parquet.String(),
+				},
+				"C": parquet.Group{
+					"CA": parquet.String(),
+				},
+				"D": parquet.Group{
+					"DA": parquet.String(),
+				},
+			},
+		},
+		{
+			name: "simple values",
+			input: obj{
+				"A": "foo",
+				"B": int64(5),
+				"C": 0.5,
+			},
+			schema: parquet.Group{
+				"A": parquet.String(),
+				"B": parquet.Int(64),
+				"C": parquet.Leaf(parquet.DoubleType),
+			},
+		},
+		{
+			name: "repeated values",
+			input: obj{
+				"A": arr{"foo", "bar", "baz"},
+				"B": arr{int64(5), int64(6)},
+				"C": arr{0.5},
+			},
+			schema: parquet.Group{
+				"A": parquet.Repeated(parquet.String()),
+				"B": parquet.Repeated(parquet.Int(64)),
+				"C": parquet.Repeated(parquet.Leaf(parquet.DoubleType)),
+			},
+		},
+		{
+			name: "nested groups",
+			input: obj{
+				"A": obj{
+					"B": obj{
+						"C": "here we are",
+					},
+				},
+			},
+			schema: parquet.Group{
+				"A": parquet.Group{
+					"B": parquet.Group{
+						"C": parquet.String(),
+					},
+				},
+			},
+		},
+		{
+			name: "nested repeated groups",
+			input: obj{
+				"A": arr{
+					obj{
+						"B": arr{
+							obj{"C": arr{"first", "second"}},
+							obj{"C": arr{"third", "fourth"}},
+							obj{"C": arr{"fifth"}},
+						},
+					},
+					obj{
+						"B": arr{
+							obj{"C": arr{"sixth"}},
+						},
+					},
+				},
+			},
+			schema: parquet.Group{
+				"A": parquet.Repeated(parquet.Group{
+					"B": parquet.Repeated(parquet.Group{
+						"C": parquet.Repeated(parquet.String()),
+					}),
+				}),
+			},
+		},
+		{
+			name: "optional values",
+			input: obj{
+				"A": "foo",
+				"B": nil,
+				"C": "baz",
+			},
+			schema: parquet.Group{
+				"A": parquet.Optional(parquet.String()),
+				"B": parquet.Optional(parquet.String()),
+				"C": parquet.Optional(parquet.String()),
+			},
+		},
+		{
+			name: "nested optional groups",
+			input: obj{
+				"A": obj{
+					"B": obj{
+						"C": "here we are",
+					},
+					"D": nil,
+				},
+			},
+			schema: parquet.Group{
+				"A": parquet.Group{
+					"B": parquet.Optional(parquet.Group{
+						"C": parquet.String(),
+					}),
+					"D": parquet.Optional(parquet.Group{
+						"E": parquet.String(),
+					}),
+				},
+			},
+		},
+	} {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			pWtr := parquet.NewGenericWriter[any](&buf, parquet.NewSchema("", test.schema))
+			if _, err := pWtr.Write([]any{test.input}); err != nil {
+				t.Fatal(err)
+			}
+			if err := pWtr.Close(); err != nil {
+				t.Fatal(err)
+			}
+
+			pRdr := parquet.NewGenericReader[any](bytes.NewReader(buf.Bytes()))
+			outRows := make([]any, 1)
+			if _, err := pRdr.Read(outRows); err != nil {
+				t.Fatal(err)
+			}
+			if err := pRdr.Close(); err != nil {
+				t.Fatal(err)
+			}
+
+			expected := test.input
+			if test.explicitOutput != nil {
+				expected = test.explicitOutput
+			}
+
+			if value1, value2 := expected, outRows[0]; !reflect.DeepEqual(value1, value2) {
+				t.Errorf("value mismatch: want=%+v got=%+v", value1, value2)
+			}
+		})
+	}
+}