Skip to content

Commit f483c57

Browse files
authored
fix: Inet extension MarshalJSON (#894)
Right now there is an inconsistency in our extensions: Some of them are using `BinaryType` as the underlying storage and some `StringType`. There is another issue in the parquet reader that seems not to like extensions with `StringType` but we can solve this bug if we want our Extensions to be of `StringType` rather then `BinaryType`. Right now I changed `Inet` to `BinaryType` as well for consistency but we have a chance to change all of them to `StringType` before we migrate sources without breaking backward compatibility because we didn't send anything over the wire yet - so right it's just an implementation change rather then a breaking change.
1 parent 70a00d0 commit f483c57

File tree

1 file changed

+27
-14
lines changed

1 file changed

+27
-14
lines changed

types/inet.go

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,26 @@ func (b *InetBuilder) Append(v *net.IPNet) {
2525
b.AppendNull()
2626
return
2727
}
28-
b.ExtensionBuilder.Builder.(*array.StringBuilder).Append(v.String())
28+
b.ExtensionBuilder.Builder.(*array.BinaryBuilder).Append([]byte(v.String()))
2929
}
3030

3131
func (b *InetBuilder) UnsafeAppend(v *net.IPNet) {
32-
b.ExtensionBuilder.Builder.(*array.StringBuilder).UnsafeAppend([]byte(v.String()))
32+
b.ExtensionBuilder.Builder.(*array.BinaryBuilder).UnsafeAppend([]byte(v.String()))
3333
}
3434

3535
func (b *InetBuilder) AppendValues(v []*net.IPNet, valid []bool) {
3636
if len(v) != len(valid) && len(valid) != 0 {
3737
panic("len(v) != len(valid) && len(valid) != 0")
3838
}
3939

40-
data := make([]string, len(v))
40+
data := make([][]byte, len(v))
4141
for i, v := range v {
4242
if len(valid) > 0 && !valid[i] {
4343
continue
4444
}
45-
data[i] = v.String()
45+
data[i] = []byte(v.String())
4646
}
47-
b.ExtensionBuilder.Builder.(*array.StringBuilder).AppendValues(data, valid)
47+
b.ExtensionBuilder.Builder.(*array.BinaryBuilder).AppendValues(data, valid)
4848
}
4949

5050
func (b *InetBuilder) AppendValueFromString(s string) error {
@@ -127,7 +127,7 @@ type InetArray struct {
127127
}
128128

129129
func (a *InetArray) String() string {
130-
arr := a.Storage().(*array.String)
130+
arr := a.Storage().(*array.Binary)
131131
o := new(strings.Builder)
132132
o.WriteString("[")
133133
for i := 0; i < arr.Len(); i++ {
@@ -149,7 +149,7 @@ func (a *InetArray) Value(i int) *net.IPNet {
149149
if a.IsNull(i) {
150150
return nil
151151
}
152-
_, ipnet, err := net.ParseCIDR(a.Storage().(*array.String).Value(i))
152+
_, ipnet, err := net.ParseCIDR(string(a.Storage().(*array.Binary).Value(i)))
153153
if err != nil {
154154
panic(fmt.Errorf("invalid ip+net: %w", err))
155155
}
@@ -167,13 +167,26 @@ func (a *InetArray) ValueStr(i int) string {
167167
}
168168

169169
func (a *InetArray) GetOneForMarshal(i int) any {
170-
if val := a.Value(i); val != nil {
171-
return val.String()
170+
if a.IsNull(i) {
171+
return nil
172172
}
173-
return nil
173+
return string(a.Storage().(*array.Binary).Value(i))
174+
}
175+
176+
func (a *InetArray) MarshalJSON() ([]byte, error) {
177+
arr := a.Storage().(*array.Binary)
178+
values := make([]any, a.Len())
179+
for i := 0; i < a.Len(); i++ {
180+
if a.IsValid(i) {
181+
values[i] = string(arr.Value(i))
182+
} else {
183+
values[i] = nil
184+
}
185+
}
186+
return json.Marshal(values)
174187
}
175188

176-
// InetType is a simple extension type that represents a StringType
189+
// InetType is a simple extension type that represents a BinaryType
177190
// to be used for representing IP Addresses and CIDRs
178191
type InetType struct {
179192
arrow.ExtensionBase
@@ -182,7 +195,7 @@ type InetType struct {
182195
// NewInetType is a convenience function to create an instance of InetType
183196
// with the correct storage type
184197
func NewInetType() *InetType {
185-
return &InetType{ExtensionBase: arrow.ExtensionBase{Storage: &arrow.StringType{}}}
198+
return &InetType{ExtensionBase: arrow.ExtensionBase{Storage: &arrow.BinaryType{}}}
186199
}
187200

188201
// ArrayType returns TypeOf(InetArray{}) for constructing Inet arrays
@@ -199,13 +212,13 @@ func (*InetType) Serialize() string {
199212
return "inet-serialized"
200213
}
201214

202-
// Deserialize expects storageType to be StringType and the data to be
215+
// Deserialize expects storageType to be BinaryType and the data to be
203216
// "inet-serialized" in order to correctly create a InetType for testing deserialize.
204217
func (*InetType) Deserialize(storageType arrow.DataType, data string) (arrow.ExtensionType, error) {
205218
if data != "inet-serialized" {
206219
return nil, fmt.Errorf("type identifier did not match: '%s'", data)
207220
}
208-
if !arrow.TypeEqual(storageType, &arrow.StringType{}) {
221+
if !arrow.TypeEqual(storageType, &arrow.BinaryType{}) {
209222
return nil, fmt.Errorf("invalid storage type for InetType: %s", storageType.Name())
210223
}
211224
return NewInetType(), nil

0 commit comments

Comments
 (0)