-
Notifications
You must be signed in to change notification settings - Fork 909
GODRIVER-3472: Add support for unmarshaling BSON Vector binary values into slices #2097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kumarlokesh Thanks for the contribution. The solution you've proposed does not quite conform to the "Definition of Done" requirements for GODRIVER-3472. See comments in vector_unmarshal_test.go
for more details.
if val.Type().Elem() == tByte { | ||
// Treat []byte as binary data, but skip for []int8 since it's a different type | ||
// even though byte is an alias for uint8 which has the same underlying type as int8 | ||
if val.Type().Elem() == tByte && val.Type() != reflect.TypeOf([]int8{}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify and add a test illustrating the purpose of val.Type() != reflect.TypeOf([]int8{})
? The bson
test suite passes without this check.
@@ -99,6 +137,12 @@ func (sc *sliceCodec) DecodeValue(dc DecodeContext, vr ValueReader, val reflect. | |||
return ValueDecoderError{Name: "SliceDecodeValue", Kinds: []reflect.Kind{reflect.Slice}, Received: val} | |||
} | |||
|
|||
if vr.Type() == TypeBinary { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic defined in this block needs to be an extension of the TypeBinary switch case.
// DecodeVectorInt8 decodes a BSON Vector binary value (subtype 9) into a []int8 slice. | ||
// The binary data should be in the format: [<vector type> <padding> <data>] | ||
// For int8 vectors, the vector type is 0x01. | ||
func DecodeVectorInt8(data []byte) ([]int8, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DecodeVectorInt8
and DecodeVectorFloat32
should not be exported and they should be migrated to slice_codec.go.
var ( | ||
// TInt8 is the reflect.Type for int8 | ||
TInt8 = reflect.TypeOf(int8(0)) | ||
// TFloat32 is the reflect.Type for float32 | ||
TFloat32 = reflect.TypeOf(float32(0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TInt8
and TFloat32
should not be exported and they should be migrated to types.go
func TestUnmarshalVectorToSlices(t *testing.T) { | ||
t.Parallel() | ||
|
||
t.Run("int8 vector to []int8", func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC this is the use-case we should be testing for:
d := D{{
"v", NewVector([]int8{-2, 1, 2, 3, 4}),
}}
bsonData, err := Marshal(d)
var result struct{ V []int8 }
err = Unmarshal(bsonData, &result)
require.NoError(t, err)
require.Equal(t, []int8{-2, 1, 2, 3, 4}, result.V)
From the "Definition of Done" section on GODRIVER-3472:
Users must be able to decode int8 Vector data into a struct field that is type []int8.
Users must be able to decode float32 Vector data into a struct field that is type []float32.
Note that this drop-in does not pass with the current proposed solution.
=== CONT TestUnmarshalVectorToSlices/int8_vector_to_[]int8
vector_unmarshal_test.go:83:
Error Trace: /Users/preston.vasquez/Developer/mongo-go-driver/bson/vector_unmarshal_test.go:83
Error: Received unexpected error:
error decoding key v: invalid vector type: expected int8 vector (0x01)
Test: TestUnmarshalVectorToSlices/int8_vector_to_[]int8
--- FAIL: TestUnmarshalVectorToSlices (0.00s)
--- FAIL: TestUnmarshalVectorToSlices/int8_vector_to_[]int8 (0.00s)
FAIL
exit status 1
FAIL go.mongodb.org/mongo-driver/v2/bson 0.285s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, we should have a test to verify this works at the mongo
package layer:
package main
import (
"context"
"fmt"
"log"
"go.mongodb.org/mongo-driver/v2/bson"
"go.mongodb.org/mongo-driver/v2/mongo"
)
func main() {
client, err := mongo.Connect()
if err != nil {
log.Fatalf("failed to connect to MongoDB: %v", err)
}
defer func() {
if err := client.Disconnect(context.Background()); err != nil {
log.Fatalf("failed to disconnect from MongoDB: %v", err)
}
}()
coll := client.Database("test").Collection("myCollection")
_ = coll.Drop(context.Background())
type myStruct struct {
V []int8
}
_, err = coll.InsertOne(context.Background(), bson.D{{"v", bson.NewVector([]int8{1, 2, 3})}})
if err != nil {
log.Fatalf("failed to insert document: %v", err)
}
result := coll.FindOne(context.Background(), bson.D{})
if result.Err() != nil {
log.Fatalf("failed to find document: %v", result.Err())
}
ms := myStruct{}
if err := result.Decode(&ms); err != nil {
log.Fatalf("failed to decode document: %v", err)
}
fmt.Println(ms.V)
}
GODRIVER-3472
Summary
This change adds support for direct unmarshaling of BSON Vector binary (subtype 0x09)
values into native Go slice types
[]int8
and[]float32
.