Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(spanner): enable row.ToStructLenient to work with STRUCT data type #5944

Merged
merged 4 commits into from Apr 27, 2022
Merged

feat(spanner): enable row.ToStructLenient to work with STRUCT data type #5944

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81505e6
feat(spanner): enable row.ToStructLenient to work with STRUCT data type
rahul2393 Apr 26, 2022
b19d87f
incorporate requested changes
rahul2393 Apr 26, 2022
f6328f8
incorporate requested changes
rahul2393 Apr 26, 2022
5a806f8
Merge branch 'main' into lenient-struct-arrray-decode
rahul2393 Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 10 additions & 5 deletions spanner/value.go
View file
Open in desktop
Expand Up @@ -986,7 +986,7 @@ func parseNullTime(v *proto3.Value, p *NullTime, code sppb.TypeCode, isNull bool

// decodeValue decodes a protobuf Value into a pointer to a Go value, as
// specified by sppb.Type.
func decodeValue(v *proto3.Value, t *sppb.Type, ptr interface{}) error {
func decodeValue(v *proto3.Value, t *sppb.Type, ptr interface{}, opts ...bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it maybe be better to create a decodeValueOption interface and pass that in instead of booleans? This means that we can only have boolean options, which both limit the types of options that we can have, but also makes them very 'magical'. Imagine that we support 3 options; no-one will understand what decodeValue(v, t, true, false, false) means.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated the PR please take a look again.

if v == nil {
return errNilSrc()
}
Expand Down Expand Up @@ -1891,7 +1891,12 @@ func decodeValue(v *proto3.Value, t *sppb.Type, ptr interface{}) error {
if err != nil {
return err
}
if err = decodeStructArray(t.ArrayElementType.StructType, x, p); err != nil {
lenient := false
if len(opts) > 0 {
// first bool option is to check if struct array decode should be lenient.
lenient = opts[0]
}
if err = decodeStructArray(t.ArrayElementType.StructType, x, p, lenient); err != nil {
return err
}
}
Expand Down Expand Up @@ -3088,7 +3093,7 @@ func decodeStruct(ty *sppb.StructType, pb *proto3.ListValue, ptr interface{}, le
return errDupSpannerField(f.Name, ty)
}
// Try to decode a single field.
if err := decodeValue(pb.Values[i], f.Type, v.FieldByIndex(sf.Index).Addr().Interface()); err != nil {
if err := decodeValue(pb.Values[i], f.Type, v.FieldByIndex(sf.Index).Addr().Interface(), lenient); err != nil {
return errDecodeStructField(ty, f.Name, err)
}
// Mark field f.Name as processed.
Expand All @@ -3113,7 +3118,7 @@ func isPtrStructPtrSlice(t reflect.Type) bool {
// decodeStructArray decodes proto3.ListValue pb into struct slice referenced by
// pointer ptr, according to the
// structural information given in a sppb.StructType.
func decodeStructArray(ty *sppb.StructType, pb *proto3.ListValue, ptr interface{}) error {
func decodeStructArray(ty *sppb.StructType, pb *proto3.ListValue, ptr interface{}, lenient bool) error {
if pb == nil {
return errNilListValue("STRUCT")
}
Expand All @@ -3139,7 +3144,7 @@ func decodeStructArray(ty *sppb.StructType, pb *proto3.ListValue, ptr interface{
return errDecodeArrayElement(i, pv, "STRUCT", err)
}
// Decode proto3.ListValue l into struct referenced by s.Interface().
if err = decodeStruct(ty, l, s.Interface(), false); err != nil {
if err = decodeStruct(ty, l, s.Interface(), lenient); err != nil {
return errDecodeArrayElement(i, pv, "STRUCT", err)
}
// Append the decoded struct back into the slice.
Expand Down
64 changes: 64 additions & 0 deletions spanner/value_test.go
View file
Open in desktop
Expand Up @@ -2273,6 +2273,70 @@ func TestDecodeStructWithPointers(t *testing.T) {
}
}

func TestDecodeStructArray(t *testing.T) {
stype := &sppb.StructType{Fields: []*sppb.StructType_Field{
{Name: "C", Type: &sppb.Type{Code: sppb.TypeCode_ARRAY,
ArrayElementType: &sppb.Type{
Code: sppb.TypeCode_STRUCT,
StructType: &sppb.StructType{Fields: []*sppb.StructType_Field{
{Name: "A", Type: intType()},
{Name: "B", Type: intType()},
}},
},
},
},
},
}
lv := listValueProto(listProto(listProto(intProto(1), intProto(2))))

type (
// inner struct
S2 struct {
A int64 `spanner:"A"`
}

S1 struct {
C []*S2 `spanner:"C"`
}
)

var (
test1 S1
test2 S1
)
for _, test := range []struct {
desc string
lenient bool
ptr interface{}
want interface{}
fail bool
}{
{
// when the Spanner returns more fields in inner struct compared to Go inner struct
desc: "decode to S1 with lenient enabled",
ptr: &test1,
want: &S1{C: []*S2{{A: 1}}},
lenient: true,
},
{
desc: "decode to S1 with lenient disabled",
ptr: &test2,
fail: true,
lenient: false,
},
} {
err := decodeStruct(stype, lv, test.ptr, test.lenient)
if (err != nil) != test.fail {
t.Errorf("%s: got error %v, wanted fail: %v", test.desc, err, test.fail)
}
if err == nil {
if !testutil.Equal(test.ptr, test.want) {
t.Errorf("%s: got %+v, want %+v", test.desc, test.ptr, test.want)
}
}
}
}

func TestEncodeStructValueDynamicStructs(t *testing.T) {
dynStructType := reflect.StructOf([]reflect.StructField{
{Name: "A", Type: reflect.TypeOf(0), Tag: `spanner:"a"`},
Expand Down