Skip to content

Commit

Permalink
fix(spanner): remove json-iterator dependency (#10099)
Browse files Browse the repository at this point in the history
json-iterator heavily relies on an unmaintained modern-go/reflect2
that pokes at the internals of Go runtime. This was shown to cause
failures in golang/go#54766 (comment).

Switching to encoding/json does come with a performance penalty.

Updates #9380
  • Loading branch information
egonelbre committed May 3, 2024
1 parent a0c097c commit 3917cca
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 44 deletions.
3 changes: 0 additions & 3 deletions spanner/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/GoogleCloudPlatform/grpc-gcp-go/grpcgcp v1.5.0
github.com/google/go-cmp v0.6.0
github.com/googleapis/gax-go/v2 v2.12.3
github.com/json-iterator/go v1.1.12
go.opencensus.io v0.24.0
go.opentelemetry.io/otel v1.24.0
go.opentelemetry.io/otel/metric v1.24.0
Expand Down Expand Up @@ -39,8 +38,6 @@ require (
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/s2a-go v0.1.7 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/stretchr/testify v1.9.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect
Expand Down
9 changes: 0 additions & 9 deletions spanner/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,6 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
Expand Down Expand Up @@ -817,8 +816,6 @@ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ
github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU=
github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk=
github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
Expand All @@ -843,11 +840,6 @@ github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/
github.com/mattn/go-sqlite3 v1.14.14/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU=
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8/go.mod h1:mC1jAcsrzbxHt8iiaC+zU4b1ylILSosueou12R++wfY=
github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3/go.mod h1:RagcQ7I8IeTMnF8JTXieKnO4Z6JCsikNEzj0DwauVzE=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M=
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/phpdave11/gofpdf v1.4.2/go.mod h1:zpO6xFn9yxo3YLyMvW8HcKWVdbNqgIfOOp2dXMnm1mY=
github.com/phpdave11/gofpdi v1.0.12/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=
github.com/phpdave11/gofpdi v1.0.13/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=
Expand Down Expand Up @@ -877,7 +869,6 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
Expand Down
3 changes: 0 additions & 3 deletions spanner/test/opentelemetry/test/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ require (
github.com/google/s2a-go v0.1.7 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
github.com/googleapis/gax-go/v2 v2.12.3 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect
Expand Down
9 changes: 0 additions & 9 deletions spanner/test/opentelemetry/test/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1531,7 +1531,6 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-pkcs11 v0.2.0/go.mod h1:6eQoGcuNJpa7jnd5pMGdkSaQpNDYvPlXWMcjXXThLlY=
github.com/google/go-pkcs11 v0.2.1-0.20230907215043-c6f79328ddf9/go.mod h1:6eQoGcuNJpa7jnd5pMGdkSaQpNDYvPlXWMcjXXThLlY=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/martian/v3 v3.2.1/go.mod h1:oBOf6HBosgwRXnUGWUB05QECsc6uvmMiJ3+6W4l/CUk=
github.com/google/martian/v3 v3.3.2/go.mod h1:oBOf6HBosgwRXnUGWUB05QECsc6uvmMiJ3+6W4l/CUk=
github.com/google/martian/v3 v3.3.3/go.mod h1:iEPrYcgCF7jA9OtScMFQyAlZZ4YXTKEtJ1E6RWzmBA0=
Expand Down Expand Up @@ -1587,8 +1586,6 @@ github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSo
github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/iancoleman/strcase v0.3.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/ianlancetaylor/demangle v0.0.0-20220319035150-800ac71e25c2/go.mod h1:aYm2/VgdVmcIU8iMfdMvDMsRAQjcfZSKFby6HOFvi/w=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
Expand Down Expand Up @@ -1625,11 +1622,6 @@ github.com/mattn/go-sqlite3 v1.14.15/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S
github.com/mattn/go-sqlite3 v1.14.16/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg=
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8/go.mod h1:mC1jAcsrzbxHt8iiaC+zU4b1ylILSosueou12R++wfY=
github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3/go.mod h1:RagcQ7I8IeTMnF8JTXieKnO4Z6JCsikNEzj0DwauVzE=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M=
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/phpdave11/gofpdf v1.4.2/go.mod h1:zpO6xFn9yxo3YLyMvW8HcKWVdbNqgIfOOp2dXMnm1mY=
github.com/phpdave11/gofpdi v1.0.12/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=
github.com/phpdave11/gofpdi v1.0.13/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=
Expand Down Expand Up @@ -1665,7 +1657,6 @@ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSS
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
Expand Down
44 changes: 24 additions & 20 deletions spanner/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"database/sql"
"database/sql/driver"
"encoding/base64"
"encoding/json"
"fmt"
"math"
"math/big"
Expand All @@ -32,7 +33,6 @@ import (
"cloud.google.com/go/civil"
"cloud.google.com/go/internal/fields"
sppb "cloud.google.com/go/spanner/apiv1/spannerpb"
jsoniter "github.com/json-iterator/go"
"google.golang.org/grpc/codes"
"google.golang.org/protobuf/proto"
proto3 "google.golang.org/protobuf/types/known/structpb"
Expand Down Expand Up @@ -114,7 +114,7 @@ var (

jsonNullBytes = []byte("null")

jsonProvider = jsoniter.ConfigCompatibleWithStandardLibrary
jsonUseNumber bool
)

// UseNumberWithJSONDecoderEncoder specifies whether Cloud Spanner JSON numbers are decoded
Expand All @@ -124,11 +124,15 @@ var (
// NOTE 1: Calling this method affects the behavior of all clients created by this library, both existing and future instances.
// NOTE 2: This method sets a global variable that is used by the client to encode/decode JSON numbers. Access to the global variable is not synchronized. You should only call this method when there are no goroutines encoding/decoding Cloud Spanner JSON values. It is recommended to only call this method during the initialization of your application, and preferably before you create any Cloud Spanner clients, and/or in tests when there are no queries being executed.
func UseNumberWithJSONDecoderEncoder(useNumber bool) {
jsonProvider = jsoniter.Config{
EscapeHTML: true,
SortMapKeys: true, // Sort map keys to ensure deterministic output, to be consistent with encoding.
UseNumber: useNumber,
}.Froze()
jsonUseNumber = useNumber
}

func jsonUnmarshal(data []byte, v any) error {
dec := json.NewDecoder(bytes.NewReader(data))
if jsonUseNumber {
dec.UseNumber()
}
return dec.Decode(v)
}

// Encoder is the interface implemented by a custom type that can be encoded to
Expand Down Expand Up @@ -297,7 +301,7 @@ func (n *NullString) UnmarshalJSON(payload []byte) error {
return nil
}
var s *string
if err := jsonProvider.Unmarshal(payload, &s); err != nil {
if err := jsonUnmarshal(payload, &s); err != nil {
return err
}
if s != nil {
Expand Down Expand Up @@ -866,7 +870,7 @@ func (n NullJSON) String() string {
if !n.Valid {
return nullString
}
b, err := jsonProvider.Marshal(n.Value)
b, err := json.Marshal(n.Value)
if err != nil {
return fmt.Sprintf("error: %v", err)
}
Expand All @@ -888,7 +892,7 @@ func (n *NullJSON) UnmarshalJSON(payload []byte) error {
return nil
}
var v interface{}
err := jsonProvider.Unmarshal(payload, &v)
err := jsonUnmarshal(payload, &v)
if err != nil {
return fmt.Errorf("payload cannot be converted to a struct: got %v, err: %w", string(payload), err)
}
Expand Down Expand Up @@ -972,7 +976,7 @@ func (n PGJsonB) String() string {
if !n.Valid {
return nullString
}
b, err := jsonProvider.Marshal(n.Value)
b, err := json.Marshal(n.Value)
if err != nil {
return fmt.Sprintf("error: %v", err)
}
Expand All @@ -994,7 +998,7 @@ func (n *PGJsonB) UnmarshalJSON(payload []byte) error {
return nil
}
var v interface{}
err := jsonProvider.Unmarshal(payload, &v)
err := jsonUnmarshal(payload, &v)
if err != nil {
return fmt.Errorf("payload cannot be converted to a struct: got %v, err: %w", string(payload), err)
}
Expand All @@ -1007,7 +1011,7 @@ func nulljson(valid bool, v interface{}) ([]byte, error) {
if !valid {
return jsonNullBytes, nil
}
return jsonProvider.Marshal(v)
return json.Marshal(v)
}

// GenericColumnValue represents the generic encoded value and type of the
Expand Down Expand Up @@ -1714,7 +1718,7 @@ func decodeValue(v *proto3.Value, t *sppb.Type, ptr interface{}, opts ...DecodeO
}
x := v.GetStringValue()
var y interface{}
err := jsonProvider.Unmarshal([]byte(x), &y)
err := jsonUnmarshal([]byte(x), &y)
if err != nil {
return err
}
Expand Down Expand Up @@ -1873,7 +1877,7 @@ func decodeValue(v *proto3.Value, t *sppb.Type, ptr interface{}, opts ...DecodeO
}
x := v.GetStringValue()
var y interface{}
err := jsonProvider.Unmarshal([]byte(x), &y)
err := jsonUnmarshal([]byte(x), &y)
if err != nil {
return err
}
Expand Down Expand Up @@ -2566,7 +2570,7 @@ func (dsc decodableSpannerType) decodeValueToCustomType(v *proto3.Value, t *sppb
}
x := v.GetStringValue()
var y interface{}
err := jsonProvider.Unmarshal([]byte(x), &y)
err := jsonUnmarshal([]byte(x), &y)
if err != nil {
return err
}
Expand All @@ -2581,7 +2585,7 @@ func (dsc decodableSpannerType) decodeValueToCustomType(v *proto3.Value, t *sppb
}
x := v.GetStringValue()
var y interface{}
err := jsonProvider.Unmarshal([]byte(x), &y)
err := jsonUnmarshal([]byte(x), &y)
if err != nil {
return err
}
Expand Down Expand Up @@ -3272,7 +3276,7 @@ func decodeNullJSONArrayToNullJSON(pb *proto3.ListValue) (*NullJSON, error) {
}
s := fmt.Sprintf("[%s]", strings.Join(strs, ","))
var y interface{}
err := jsonProvider.Unmarshal([]byte(s), &y)
err := jsonUnmarshal([]byte(s), &y)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -3929,7 +3933,7 @@ func encodeValue(v interface{}) (*proto3.Value, *sppb.Type, error) {
pt = listType(pgNumericType())
case NullJSON:
if v.Valid {
b, err := jsonProvider.Marshal(v.Value)
b, err := json.Marshal(v.Value)
if err != nil {
return nil, nil, err
}
Expand All @@ -3946,7 +3950,7 @@ func encodeValue(v interface{}) (*proto3.Value, *sppb.Type, error) {
pt = listType(jsonType())
case PGJsonB:
if v.Valid {
b, err := jsonProvider.Marshal(v.Value)
b, err := json.Marshal(v.Value)
if err != nil {
return nil, nil, err
}
Expand Down

0 comments on commit 3917cca

Please sign in to comment.