-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Spanner: NullString
returning quoted strings
#1610
Comments
NullString
returning quoted stringsNullString
returning quoted strings
@arminm Thanks for your detailed report. You're observation is correct, but I don't think this is something we would want to change. The Could you elaborate a little bit on why you would want this changed and/or what other means might also mitigate any (potential) problems that you are running into with this? |
I also can't pass a variable with It's just quite a strange decision made by Google here, to assume that the I personally think a better solution to handle nullable fields is to fix the type Model struct {
ID string `spanner:"id"`
Name string `spanner:"name,nullable"`
} and just handle that to return |
@arminm After an internal discussion we are inclined to agree with you that the current implementation of So we'll look into two separate changes:
|
Thank you for being so receptive of my feedback. I'll keep an eye open for the changes in the upcoming versions. 🙏 |
The String() method of NullString, NullTime and NullDate should all return unquoted strings, instead of adding double quotes around the returned value. This makes it easier to use the values in standard methods that expect a Stringer, as the returned value corresponds to the actual underlying value. Updates #1610. Change-Id: I20b6d5d25ee8b9325e6f64336dca6b89a2d396db Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/47192 Reviewed-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Shanika Kuruppu <skuruppu@google.com> Reviewed-by: Jean de Klerk <deklerk@google.com>
This change adds support for decoding result set values from Spanner into custom types that point back to supported types. This allows decoding a result set into for example a struct like this: type blobField []byte type dbProductInfo struct { ProductID int64 `spanner:"ProductID"` UpdateDate time.Time `spanner:"UpdateDate"` Data blobField `spanner:"Data"` } Decoding Spanner column values to one of the known types (string, []byte, int64, float64, time.Timestamp, civil.Date) and their known Null<type> versions is done using a type switch statement. Decoding to a custom type is done with reflection. The reason for not using reflection for all types, including the known base types, is the performance difference. Benchmark results executed locally showing the difference between the two are shown below based on decoding a single string value to a standard string or a custom string type. goos: linux goarch: amd64 pkg: cloud.google.com/go/spanner BenchmarkDecodeString-8 200000000 8.21 ns/op BenchmarkDecodeCustomString-8 10000000 135 ns/op Updates #1610. Fixes #853. Change-Id: Iad689b3704acadb83809375c762a7e0b564a7b2f Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/48090 Reviewed-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Tyler Bui-Palsulich <tbp@google.com> Reviewed-by: Hengfeng Li <hengfeng@google.com>
Given that the work seems to have been done, I am closing this issue. If you feel that there is more that needs to be done, please feel free to reopen 👍 |
@skuruppu it definitely helped! I quickly updated to the version that added the native support for JSON, and refactored my code to use that instead of my custom translations. 🙏 |
The
NullString
'sString()
function is returning quoted strings instead of just the string value. The implementation is quite strange where it casts strings into a%q
format instead of just a%s
, or not usingfmt.Sprintf
at all!NullString.StringVal
is already astring
type:but its
String()
function is using a quoted format to return the value:NullString
reference: https://github.com/googleapis/google-cloud-go/blob/master/spanner/value.go#L67-L73There's also
NullTime
that is also returning a quoted string when it doesn't need to format the response:NullTime
: https://github.com/googleapis/google-cloud-go/blob/master/spanner/value.go#L109-L115and
NullDate
where probably%s
should be used.NullDate
: https://github.com/googleapis/google-cloud-go/blob/master/spanner/value.go#L123-L129The text was updated successfully, but these errors were encountered: