Update Graph Binary serializer to 4.0.0 spec for Go #3034
Conversation
gremlin-go/driver/graphBinary.go
Outdated
| case reflect.Map: | ||
| mapData.Set(&k, v) | ||
| default: | ||
| mapData.Set(fmt.Sprint(k), v) |
There was a problem hiding this comment.
Would a slice key be set as a string key here?
There was a problem hiding this comment.
Yes, slices themselves cannot be set as keys (non-comparable type) in go, this turns it into a string to be used as the key.
gremlin-go/driver/graphBinary.go
Outdated
| @@ -1028,7 +877,8 @@ func readMapUnqualified(data *[]byte, i *int) (interface{}, error) { | |||
| } | |||
|
|
|||
| func readSet(data *[]byte, i *int) (interface{}, error) { | |||
| list, err := readList(data, i) | |||
| // TODO placeholder flag | |||
There was a problem hiding this comment.
Why was this left as TODO?
There was a problem hiding this comment.
I meant to update set as well. Will do that.
gremlin-go/driver/graphBinary.go
Outdated
| @@ -1351,9 +1073,17 @@ func readFullyQualifiedNullable(data *[]byte, i *int, nullable bool) (interface{ | |||
| } | |||
| return nil, nil | |||
| } else if nullable { | |||
| if readByteSafe(data, i) == valueFlagNull { | |||
| flag := readByteSafe(data, i) | |||
| fmt.Println(flag) | |||
gremlin-go/driver/graphBinary.go
Outdated
| convKey := k.Convert(v.Type().Key()) | ||
| // serialize k | ||
| _, err := typeSerializer.write(k.Interface(), buffer) | ||
| if val, ok := value.(*orderedmap.OrderedMap[interface{}, interface{}]); ok { |
There was a problem hiding this comment.
I can't seem to find where the {value_flag} equal to 0x02 for an ordered map is set, can you point me to it?
| return nil, err | ||
| } | ||
| bulk := readIntSafe(data, i) | ||
| for k := int32(0); k < bulk; k++ { |
There was a problem hiding this comment.
I can't seem to find where bulk list value flag and value are written. Can you point me to it?
There was a problem hiding this comment.
So both ordered dict and bulk list are result formats that's sent by the server, nothing (that I'm aware of, I could be wrong) the GLV sends is bulked or ordered.
There was a problem hiding this comment.
I can't find any usage of readFullyQualifiedNullable where nullable isn't hardcoded to true. Can that function param just be removed?
There was a problem hiding this comment.
I'll double check, but I think that'll be a separate set of changes from this PR
| return getDefaultValue(dataTyp), nil | ||
| } | ||
| if dataTyp == listType { |
There was a problem hiding this comment.
From what I can tell the readList and readMap functions are treated differently here because they accept the flag as a param. Since the flag value is always read before calling the deserializer can the deserializer's reader function be changed to accept a flag param and the implementations can either use it or ignore it depending on the type?
There was a problem hiding this comment.
If I recall correctly, to do that we'll have to update implementations for all and I wanted the change to be relatively isolated to these 2 types, but I'll take a closer look.
First set of changes for type serializers in Go. Updated element serializers, datetime, list and map serializers, and removed old types no longer in 4.0 specs.
For ordered map, Go doesn't have native support for ordered map, so an open source library was used. Alternative option is to not support ordered map in Go and just return regular map if it's a concern to users, and note in documentation.