Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
@yevgenypats
yevgenypats committed Mar 6, 2023
1 parent 131365d commit cded2e3
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 78 deletions.
2 changes: 1 addition & 1 deletion internal/cqarrow/arrow_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestCQTypesToRecord(t *testing.T) {
testCqTypes := testdata.GenTestData(testTable)
arrowSchema := CQSchemaToArrow(testTable)
mem := memory.NewGoAllocator()
record := CqTypesToRecord(mem, []schema.CQTypes{testCqTypes}, arrowSchema)
record := CQTypesToRecord(mem, []schema.CQTypes{testCqTypes}, arrowSchema)
str, err := json.MarshalIndent(record, "", " ")
if err != nil {
t.Error(err)
Expand Down
16 changes: 8 additions & 8 deletions internal/cqarrow/inet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (b *InetBuilder) UnmarshalJSON(data []byte) error {
return b.Unmarshal(dec)
}

// UUIDArray is a simple array which is a FixedSizeBinary(16)
// InetArray is a simple array which is a FixedSizeBinary(16)
type InetArray struct {
array.ExtensionArrayBase
}
Expand Down Expand Up @@ -137,13 +137,13 @@ func (a *InetArray) GetOneForMarshal(i int) interface{} {
return nil
}

// InetType is a simple extension type that represents a FixedSizeBinary(16)
// to be used for representing UUIDs
// InetType is a simple extension type that represents a StringType
// to be used for representing IP Addresses and CIDRs
type InetType struct {
arrow.ExtensionBase
}

// NewInetType is a convenience function to create an instance of UuidType
// NewInetType is a convenience function to create an instance of InetType
// with the correct storage type
func NewInetType() *InetType {
return &InetType{
Expand All @@ -157,11 +157,11 @@ func (InetType) InetType() reflect.Type { return reflect.TypeOf(InetArray{}) }

func (InetType) ExtensionName() string { return "inet" }

// Serialize returns "uuid-serialized" for testing proper metadata passing
// Serialize returns "inet-serialized" for testing proper metadata passing
func (InetType) Serialize() string { return "inet-serialized" }

// Deserialize expects storageType to be FixedSizeBinaryType{ByteWidth: 16} and the data to be
// "uuid-serialized" in order to correctly create a UuidType for testing deserialize.
// Deserialize expects storageType to be StringType and the data to be
// "inet-serialized" in order to correctly create a InetType for testing deserialize.
func (InetType) Deserialize(storageType arrow.DataType, data string) (arrow.ExtensionType, error) {
if data != "inet-serialized" {
return nil, fmt.Errorf("type identifier did not match: '%s'", data)
Expand All @@ -172,7 +172,7 @@ func (InetType) Deserialize(storageType arrow.DataType, data string) (arrow.Exte
return NewInetType(), nil
}

// UuidTypes are equal if both are named "uuid"
// InetType are equal if both are named "inet"
func (u InetType) ExtensionEquals(other arrow.ExtensionType) bool {
return u.ExtensionName() == other.ExtensionName()
}
Expand Down
8 changes: 4 additions & 4 deletions internal/cqarrow/inet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ func TestInetBuilder(t *testing.T) {

a := b.NewArray()

// check state of builder after NewUUIDBuilder
require.Zero(t, b.Len(), "unexpected ArrayBuilder.Len(), NewInetBuilder did not reset state")
require.Zero(t, b.Cap(), "unexpected ArrayBuilder.Cap(), NewInetBuilder did not reset state")
require.Zero(t, b.NullN(), "unexpected ArrayBuilder.NullN(), NewInetBuilder did not reset state")
// check state of builder after NewInetBuilder
require.Zero(t, b.Len(), "unexpected ArrayBuilder.Len(), did not reset state")
require.Zero(t, b.Cap(), "unexpected ArrayBuilder.Cap(), did not reset state")
require.Zero(t, b.NullN(), "unexpected ArrayBuilder.NullN(), did not reset state")

require.Equal(t, `["192.168.0.0/24" (null) "192.168.0.0/25" (null) "192.168.0.0/26" "192.168.0.0/27"]`, a.String())
st, err := a.MarshalJSON()
Expand Down
60 changes: 3 additions & 57 deletions internal/cqarrow/json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,49 +57,6 @@ func (b *JSONBuilder) AppendValues(v []any, valid []bool) {
b.ExtensionBuilder.Builder.(*array.BinaryBuilder).AppendValues(data, valid)
}

// func (b *JSONBuilder) UnmarshalOne(dec *json.Decoder) error {
// t, err := dec.Token()
// if err != nil {
// return err
// }

// var data any
// switch v := t.(type) {
// case string:
// err := json.Unmarshal([]byte(v), &data)
// if err != nil {
// return err
// }
// case []byte:
// err := json.Unmarshal(v, &data)
// if err != nil {
// return err
// }
// case nil:
// b.AppendNull()
// return nil
// default:
// return &json.UnmarshalTypeError{
// Value: fmt.Sprint(t),
// Type: reflect.TypeOf([]byte{}),
// Offset: dec.InputOffset(),
// Struct: "JSONBuilder",
// }
// }

// b.Append(data)
// return nil
// }

// func (b *JSONBuilder) Unmarshal(dec *json.Decoder) error {
// for dec.More() {
// if err := b.UnmarshalOne(dec); err != nil {
// return err
// }
// }
// return nil
// }

func (b *JSONBuilder) UnmarshalJSON(data []byte) error {
var a []any
if err := json.Unmarshal(data, &a); err != nil {
Expand All @@ -110,17 +67,6 @@ func (b *JSONBuilder) UnmarshalJSON(data []byte) error {
valid[i] = a[i] != nil
}
b.AppendValues(a, valid)
// dec := json.NewDecoder(bytes.NewReader(data))
// t, err := dec.Token()
// if err != nil {
// return err
// }

// if delim, ok := t.(json.Delim); !ok || delim != '[' {
// return fmt.Errorf("json builder must unpack from json array, found %s", delim)
// }

// return b.Unmarshal(dec)
return nil
}

Expand Down Expand Up @@ -177,7 +123,7 @@ func (a *JSONArray) GetOneForMarshal(i int) interface{} {
return nil
}

// JSONType is a simple extension type that represents a FixedSizeBinary(16)
// JSONType is a simple extension type that represents a BinaryType
// to be used for representing JSONs
type JSONType struct {
arrow.ExtensionBase
Expand Down Expand Up @@ -205,8 +151,8 @@ func (e JSONType) MarshalJSON() ([]byte, error) {
// Serialize returns "json-serialized" for testing proper metadata passing
func (JSONType) Serialize() string { return "json-serialized" }

// Deserialize expects storageType to be FixedSizeBinaryType{ByteWidth: 16} and the data to be
// "json-serialized" in order to correctly create a UuidType for testing deserialize.
// Deserialize expects storageType to be BinaryBuilder and the data to be
// "json-serialized" in order to correctly create a JSONType for testing deserialize.
func (JSONType) Deserialize(storageType arrow.DataType, data string) (arrow.ExtensionType, error) {
if data != "json-serialized" {
return nil, fmt.Errorf("type identifier did not match: '%s'", data)
Expand Down
6 changes: 3 additions & 3 deletions internal/cqarrow/mac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (b *MacBuilder) UnmarshalJSON(data []byte) error {
return b.Unmarshal(dec)
}

// UUIDArray is a simple array which is a FixedSizeBinary(16)
// MacArray is a simple array which is a wrapper around a BinaryArray
type MacArray struct {
array.ExtensionArrayBase
}
Expand Down Expand Up @@ -143,8 +143,8 @@ func (a *MacArray) GetOneForMarshal(i int) interface{} {
return nil
}

// InetType is a simple extension type that represents a FixedSizeBinary(16)
// to be used for representing UUIDs
// MacType is a simple extension type that represents a BinaryType
// to be used for representing mac addresses.
type MacType struct {
arrow.ExtensionBase
}
Expand Down
8 changes: 4 additions & 4 deletions internal/cqarrow/mac_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ func TestMacBuilder(t *testing.T) {

a := b.NewArray()

// check state of builder after NewUUIDBuilder
require.Zero(t, b.Len(), "unexpected ArrayBuilder.Len(), NewUUIDBuilder did not reset state")
require.Zero(t, b.Cap(), "unexpected ArrayBuilder.Cap(), NewUUIDBuilder did not reset state")
require.Zero(t, b.NullN(), "unexpected ArrayBuilder.NullN(), NewUUIDBuilder did not reset state")
// check state of builder after NewArray
require.Zero(t, b.Len(), "unexpected ArrayBuilder.Len(), did not reset state")
require.Zero(t, b.Cap(), "unexpected ArrayBuilder.Cap(), did not reset state")
require.Zero(t, b.NullN(), "unexpected ArrayBuilder.NullN(), did not reset state")

require.Equal(t, `["00:00:00:00:00:01" (null) "00:00:00:00:00:02" (null) "00:00:00:00:00:03" "00:00:00:00:00:04"]`, a.String())
st, err := a.MarshalJSON()
Expand Down
2 changes: 1 addition & 1 deletion json/write.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (*Client) WriteTableBatch(w io.Writer, table *schema.Table, resources [][]a
cqTypes[i][j] = resources[i][j].(schema.CQType)
}
}
record := cqarrow.CqTypesToRecord(memory.DefaultAllocator, cqTypes, arrowSchema)
record := cqarrow.CQTypesToRecord(memory.DefaultAllocator, cqTypes, arrowSchema)
defer record.Release()

arr := array.RecordToStructArray(record)
Expand Down

0 comments on commit cded2e3

Please sign in to comment.