-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(spanner): enable row.ToStructLenient to work with STRUCT data type #5944
Conversation
spanner/value.go
Outdated
@@ -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 { |
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.
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.
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.
Thanks, updated the PR please take a look again.
spanner/value.go
Outdated
@@ -3043,6 +3049,22 @@ func errDecodeStructField(ty *sppb.StructType, f string, err error) error { | |||
return se | |||
} | |||
|
|||
// DecodeSetting contains all the settings for decoding from spanner struct | |||
type DecodeSetting struct { |
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.
nit: I think we can make this unexported
Fixes: #5938