ribaraka commented on code in PR #1828:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1828#discussion_r2034109135
##########
marshal.go:
##########
@@ -172,6 +172,11 @@ func Marshal(info TypeInfo, value interface{}) ([]byte,
error) {
return marshalDate(info, value)
case TypeDuration:
return marshalDuration(info, value)
+ case TypeCustom:
+ switch info.(type) {
+ case VectorType:
+ return marshalVector(info.(VectorType), value)
+ }
Review Comment:
you can simplify this block of code by assigning the type to var so you
eliminate the need for the second type assertion:
```suggestion
case TypeCustom:
switch t := info.(type) {
case VectorType:
return marshalVector(t, value)
}
}
```
##########
marshal.go:
##########
@@ -1716,6 +1726,169 @@ func unmarshalList(info TypeInfo, data []byte, value
interface{}) error {
return unmarshalErrorf("can not unmarshal %s into %T. Accepted types:
*slice, *array.", info, value)
}
+func marshalVector(info VectorType, value interface{}) ([]byte, error) {
+ if value == nil {
+ return nil, nil
+ } else if _, ok := value.(unsetColumn); ok {
+ return nil, nil
+ }
+
+ rv := reflect.ValueOf(value)
+ t := rv.Type()
+ k := t.Kind()
+ if k == reflect.Slice && rv.IsNil() {
+ return nil, nil
+ }
+
+ switch k {
+ case reflect.Slice, reflect.Array:
+ buf := &bytes.Buffer{}
+ n := rv.Len()
+ if n != info.Dimensions {
+ return nil, marshalErrorf("expected vector with %d
dimensions, received %d", info.Dimensions, n)
+ }
+
+ for i := 0; i < n; i++ {
+ item, err := Marshal(info.SubType,
rv.Index(i).Interface())
+ if err != nil {
+ return nil, err
+ }
+ if isVectorVariableLengthType(info.SubType) {
+ writeUnsignedVInt(buf, uint64(len(item)))
+ }
+ buf.Write(item)
+ }
+ return buf.Bytes(), nil
+ }
+ return nil, marshalErrorf("can not marshal %T into %s", value, info)
+}
+
+func unmarshalVector(info VectorType, data []byte, value interface{}) error {
+ rv := reflect.ValueOf(value)
+ if rv.Kind() != reflect.Ptr {
+ return unmarshalErrorf("can not unmarshal into non-pointer %T",
value)
+ }
+ rv = rv.Elem()
+ t := rv.Type()
+ k := t.Kind()
+ switch k {
+ case reflect.Slice, reflect.Array:
+ if data == nil {
+ if k == reflect.Array {
+ return unmarshalErrorf("unmarshal vector: can
not store nil in array value")
+ }
+ if rv.IsNil() {
+ return nil
+ }
+ rv.Set(reflect.Zero(t))
+ return nil
+ }
+ if k == reflect.Array {
+ if rv.Len() != info.Dimensions {
+ return unmarshalErrorf("unmarshal vector: array
of size %d cannot store vector of %d dimensions", rv.Len(), info.Dimensions)
+ }
+ } else {
+ rv.Set(reflect.MakeSlice(t, info.Dimensions,
info.Dimensions))
+ }
+ elemSize := len(data) / info.Dimensions
+ for i := 0; i < info.Dimensions; i++ {
+ offset := 0
+ if isVectorVariableLengthType(info.SubType) {
+ m, p, err := readUnsignedVint(data, 0)
+ if err != nil {
+ return err
+ }
+ elemSize = int(m)
+ offset = p
+ }
+ if offset > 0 {
+ data = data[offset:]
+ }
+ var unmarshalData []byte
+ if elemSize >= 0 {
+ if len(data) < elemSize {
+ return unmarshalErrorf("unmarshal
vector: unexpected eof")
+ }
+ unmarshalData = data[:elemSize]
+ data = data[elemSize:]
+ }
+ err := Unmarshal(info.SubType, unmarshalData,
rv.Index(i).Addr().Interface())
+ if err != nil {
+ return unmarshalErrorf("failed to unmarshal %s
into %T: %s", info.SubType, unmarshalData, err.Error())
+ }
+ }
+ return nil
+ }
+ return unmarshalErrorf("can not unmarshal %s into %T", info, value)
Review Comment:
same as above
##########
marshal.go:
##########
@@ -1716,6 +1726,169 @@ func unmarshalList(info TypeInfo, data []byte, value
interface{}) error {
return unmarshalErrorf("can not unmarshal %s into %T. Accepted types:
*slice, *array.", info, value)
}
+func marshalVector(info VectorType, value interface{}) ([]byte, error) {
+ if value == nil {
+ return nil, nil
+ } else if _, ok := value.(unsetColumn); ok {
+ return nil, nil
+ }
+
+ rv := reflect.ValueOf(value)
+ t := rv.Type()
+ k := t.Kind()
+ if k == reflect.Slice && rv.IsNil() {
+ return nil, nil
+ }
+
+ switch k {
+ case reflect.Slice, reflect.Array:
+ buf := &bytes.Buffer{}
+ n := rv.Len()
+ if n != info.Dimensions {
+ return nil, marshalErrorf("expected vector with %d
dimensions, received %d", info.Dimensions, n)
+ }
+
+ for i := 0; i < n; i++ {
+ item, err := Marshal(info.SubType,
rv.Index(i).Interface())
+ if err != nil {
+ return nil, err
+ }
+ if isVectorVariableLengthType(info.SubType) {
+ writeUnsignedVInt(buf, uint64(len(item)))
Review Comment:
nitpick: These functions could use some documentation imo—particularly
explaining how it works, the intended purpose, and what happens if
`info.SubType` is not `isVectorVariableLengthType`.
##########
helpers.go:
##########
@@ -162,54 +164,154 @@ func getCassandraBaseType(name string) Type {
}
}
-func getCassandraType(name string, logger StdLogger) TypeInfo {
+// Parses long Java-style type definition to internal data structures.
+func getCassandraLongType(name string, protoVer byte, logger StdLogger)
TypeInfo {
+ if strings.HasPrefix(name, SET_TYPE) {
+ return CollectionType{
+ NativeType: NewNativeType(protoVer, TypeSet),
+ Elem:
getCassandraLongType(strings.TrimPrefix(name[:len(name)-1], SET_TYPE+"("),
protoVer, logger),
+ }
+ } else if strings.HasPrefix(name, LIST_TYPE) {
+ return CollectionType{
+ NativeType: NewNativeType(protoVer, TypeList),
+ Elem:
getCassandraLongType(strings.TrimPrefix(name[:len(name)-1], LIST_TYPE+"("),
protoVer, logger),
+ }
+ } else if strings.HasPrefix(name, MAP_TYPE) {
+ names :=
splitJavaCompositeTypes(strings.TrimPrefix(name[:len(name)-1], MAP_TYPE+"("))
+ if len(names) != 2 {
+ logger.Printf("gocql: error parsing map type, it has %d
subelements, expecting 2\n", len(names))
+ return NewNativeType(protoVer, TypeCustom)
+ }
+ return CollectionType{
+ NativeType: NewNativeType(protoVer, TypeMap),
+ Key: getCassandraLongType(names[0], protoVer,
logger),
+ Elem: getCassandraLongType(names[1], protoVer,
logger),
+ }
+ } else if strings.HasPrefix(name, TUPLE_TYPE) {
+ names :=
splitJavaCompositeTypes(strings.TrimPrefix(name[:len(name)-1], TUPLE_TYPE+"("))
+ types := make([]TypeInfo, len(names))
+
+ for i, name := range names {
+ types[i] = getCassandraLongType(name, protoVer, logger)
+ }
+
+ return TupleTypeInfo{
+ NativeType: NewNativeType(protoVer, TypeTuple),
+ Elems: types,
+ }
+ } else if strings.HasPrefix(name, UDT_TYPE) {
+ names :=
splitJavaCompositeTypes(strings.TrimPrefix(name[:len(name)-1], UDT_TYPE+"("))
+ fields := make([]UDTField, len(names)-2)
+
+ for i := 2; i < len(names); i++ {
+ spec := strings.Split(names[i], ":")
+ fieldName, _ := hex.DecodeString(spec[0])
+ fields[i-2] = UDTField{
+ Name: string(fieldName),
+ Type: getCassandraLongType(spec[1], protoVer,
logger),
+ }
+ }
+
+ udtName, _ := hex.DecodeString(names[1])
+ return UDTTypeInfo{
+ NativeType: NewNativeType(protoVer, TypeUDT),
+ KeySpace: names[0],
+ Name: string(udtName),
+ Elements: fields,
+ }
+ } else if strings.HasPrefix(name, VECTOR_TYPE) {
+ names :=
splitJavaCompositeTypes(strings.TrimPrefix(name[:len(name)-1], VECTOR_TYPE+"("))
Review Comment:
In the `splitCQLCompositeTypes` and `splitJavaCompositeTypes`, the prefix
trimming logic is repeated in multiple cases. It could be moved inside those
funcs, which would make the code cleaner and reduce repetition.
##########
helpers.go:
##########
@@ -162,54 +164,154 @@ func getCassandraBaseType(name string) Type {
}
}
-func getCassandraType(name string, logger StdLogger) TypeInfo {
+// Parses long Java-style type definition to internal data structures.
+func getCassandraLongType(name string, protoVer byte, logger StdLogger)
TypeInfo {
Review Comment:
Should we add a TODO to cover `getCassandraLongType` with tests?
##########
marshal.go:
##########
@@ -276,6 +281,11 @@ func Unmarshal(info TypeInfo, data []byte, value
interface{}) error {
return unmarshalDate(info, data, value)
case TypeDuration:
return unmarshalDuration(info, data, value)
+ case TypeCustom:
+ switch info.(type) {
+ case VectorType:
+ return unmarshalVector(info.(VectorType), data, value)
+ }
Review Comment:
same as above
##########
marshal.go:
##########
@@ -1716,6 +1726,169 @@ func unmarshalList(info TypeInfo, data []byte, value
interface{}) error {
return unmarshalErrorf("can not unmarshal %s into %T. Accepted types:
*slice, *array.", info, value)
}
+func marshalVector(info VectorType, value interface{}) ([]byte, error) {
+ if value == nil {
+ return nil, nil
+ } else if _, ok := value.(unsetColumn); ok {
+ return nil, nil
+ }
+
+ rv := reflect.ValueOf(value)
+ t := rv.Type()
+ k := t.Kind()
+ if k == reflect.Slice && rv.IsNil() {
+ return nil, nil
+ }
+
+ switch k {
+ case reflect.Slice, reflect.Array:
+ buf := &bytes.Buffer{}
+ n := rv.Len()
+ if n != info.Dimensions {
+ return nil, marshalErrorf("expected vector with %d
dimensions, received %d", info.Dimensions, n)
+ }
+
+ for i := 0; i < n; i++ {
+ item, err := Marshal(info.SubType,
rv.Index(i).Interface())
+ if err != nil {
+ return nil, err
+ }
+ if isVectorVariableLengthType(info.SubType) {
+ writeUnsignedVInt(buf, uint64(len(item)))
+ }
+ buf.Write(item)
+ }
+ return buf.Bytes(), nil
+ }
+ return nil, marshalErrorf("can not marshal %T into %s", value, info)
Review Comment:
I think it would be helpful to mention which types are actually marshalable
(e.g., array, slice)
##########
marshal.go:
##########
@@ -172,6 +172,11 @@ func Marshal(info TypeInfo, value interface{}) ([]byte,
error) {
return marshalDate(info, value)
case TypeDuration:
return marshalDuration(info, value)
+ case TypeCustom:
+ switch info.(type) {
+ case VectorType:
+ return marshalVector(info.(VectorType), value)
+ }
Review Comment:
Also the switch statement appears to only handle the `VectorType` case
without any `default` case.
##########
marshal.go:
##########
@@ -1716,6 +1726,169 @@ func unmarshalList(info TypeInfo, data []byte, value
interface{}) error {
return unmarshalErrorf("can not unmarshal %s into %T. Accepted types:
*slice, *array.", info, value)
}
+func marshalVector(info VectorType, value interface{}) ([]byte, error) {
+ if value == nil {
+ return nil, nil
+ } else if _, ok := value.(unsetColumn); ok {
+ return nil, nil
+ }
+
+ rv := reflect.ValueOf(value)
+ t := rv.Type()
+ k := t.Kind()
+ if k == reflect.Slice && rv.IsNil() {
+ return nil, nil
+ }
+
+ switch k {
+ case reflect.Slice, reflect.Array:
+ buf := &bytes.Buffer{}
+ n := rv.Len()
+ if n != info.Dimensions {
+ return nil, marshalErrorf("expected vector with %d
dimensions, received %d", info.Dimensions, n)
+ }
+
+ for i := 0; i < n; i++ {
+ item, err := Marshal(info.SubType,
rv.Index(i).Interface())
+ if err != nil {
+ return nil, err
+ }
+ if isVectorVariableLengthType(info.SubType) {
+ writeUnsignedVInt(buf, uint64(len(item)))
+ }
+ buf.Write(item)
+ }
+ return buf.Bytes(), nil
+ }
+ return nil, marshalErrorf("can not marshal %T into %s", value, info)
+}
+
+func unmarshalVector(info VectorType, data []byte, value interface{}) error {
+ rv := reflect.ValueOf(value)
+ if rv.Kind() != reflect.Ptr {
+ return unmarshalErrorf("can not unmarshal into non-pointer %T",
value)
+ }
+ rv = rv.Elem()
+ t := rv.Type()
+ k := t.Kind()
+ switch k {
+ case reflect.Slice, reflect.Array:
+ if data == nil {
+ if k == reflect.Array {
+ return unmarshalErrorf("unmarshal vector: can
not store nil in array value")
+ }
+ if rv.IsNil() {
+ return nil
+ }
+ rv.Set(reflect.Zero(t))
+ return nil
+ }
+ if k == reflect.Array {
+ if rv.Len() != info.Dimensions {
+ return unmarshalErrorf("unmarshal vector: array
of size %d cannot store vector of %d dimensions", rv.Len(), info.Dimensions)
+ }
+ } else {
+ rv.Set(reflect.MakeSlice(t, info.Dimensions,
info.Dimensions))
+ }
+ elemSize := len(data) / info.Dimensions
+ for i := 0; i < info.Dimensions; i++ {
+ offset := 0
+ if isVectorVariableLengthType(info.SubType) {
+ m, p, err := readUnsignedVint(data, 0)
+ if err != nil {
+ return err
+ }
+ elemSize = int(m)
+ offset = p
+ }
+ if offset > 0 {
+ data = data[offset:]
+ }
+ var unmarshalData []byte
+ if elemSize >= 0 {
+ if len(data) < elemSize {
+ return unmarshalErrorf("unmarshal
vector: unexpected eof")
+ }
+ unmarshalData = data[:elemSize]
+ data = data[elemSize:]
+ }
+ err := Unmarshal(info.SubType, unmarshalData,
rv.Index(i).Addr().Interface())
+ if err != nil {
+ return unmarshalErrorf("failed to unmarshal %s
into %T: %s", info.SubType, unmarshalData, err.Error())
+ }
+ }
+ return nil
+ }
+ return unmarshalErrorf("can not unmarshal %s into %T", info, value)
+}
+
+func isVectorVariableLengthType(elemType TypeInfo) bool {
+ switch elemType.Type() {
+ case TypeVarchar, TypeAscii, TypeBlob, TypeText:
+ return true
+ case TypeCounter:
+ return true
+ case TypeDuration, TypeDate, TypeTime:
+ return true
+ case TypeDecimal, TypeSmallInt, TypeTinyInt, TypeVarint:
+ return true
+ case TypeInet:
+ return true
+ case TypeList, TypeSet, TypeMap, TypeUDT, TypeTuple:
+ return true
Review Comment:
why don't combine it in one return?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
