ribaraka commented on code in PR #1828:
URL: 
https://github.com/apache/cassandra-gocql-driver/pull/1828#discussion_r2035216417


##########
marshal.go:
##########
@@ -1716,6 +1724,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)

Review Comment:
   The `start` parameter of the `readUnsignedVint` func seem to be redundunt. 
Could move inside the func. Or am I missing something? 



##########
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:
   Thanks for the explanation — that clarifies things. I was curios how data 
considered "Vector Variable Length" in this case. So if you like it, I suggest 
adding comment as following:
   ```
   // isVectorVariableLengthType determines if a type requires explicit length 
serialization within a vector. 
   Variable-length types need their length encoded before the actual data to 
allow proper deserialization.
   Fixed-length types, on the other hand, don't require this kind of length 
prefix.
   ```
   
   Also, if there are no serialization requirements for fixed-length types, 
should `TypeSmallInt` and `TypeTinyInt` be removed from 
`isVectorVariableLengthType`?



##########
marshal.go:
##########
@@ -1716,6 +1724,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
+       case TypeCustom:
+               switch elemType.(type) {
+               case VectorType:
+                       vecType := elemType.(VectorType)
+                       return isVectorVariableLengthType(vecType.SubType)
+               }
+               return true

Review Comment:
   since we agreed on similar switch statement, let's then keep it consistent 
:pray: 
   
   ctx: 
https://github.com/apache/cassandra-gocql-driver/pull/1828#discussion_r2035057901



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to