From 0c5fe88fb3426438e8f789a471ea1beed8290ea3 Mon Sep 17 00:00:00 2001 From: rahul yadav Date: Thu, 14 Dec 2023 16:03:36 +0530 Subject: [PATCH] incorporate suggestions --- spanner/client.go | 14 +------------- spanner/integration_test.go | 31 +++++++++++-------------------- spanner/value.go | 21 ++++++++++----------- 3 files changed, 22 insertions(+), 44 deletions(-) diff --git a/spanner/client.go b/spanner/client.go index 782765d40092..52cc320a74ff 100644 --- a/spanner/client.go +++ b/spanner/client.go @@ -28,7 +28,6 @@ import ( "cloud.google.com/go/internal/trace" sppb "cloud.google.com/go/spanner/apiv1/spannerpb" "github.com/googleapis/gax-go/v2" - jsoniter "github.com/json-iterator/go" "google.golang.org/api/iterator" "google.golang.org/api/option" "google.golang.org/api/option/internaloption" @@ -187,10 +186,6 @@ type ClientConfig struct { // BatchTimeout specifies the timeout for a batch of sessions managed sessionClient. BatchTimeout time.Duration - - // UseNumber causes the Decoder to unmarshal a number into an interface{} as a - // Number instead of as a float64. - UseNumber bool } func contextWithOutgoingMetadata(ctx context.Context, md metadata.MD, disableRouteToLeader bool) context.Context { @@ -270,14 +265,7 @@ func NewClientWithConfig(ctx context.Context, database string, config ClientConf if config.BatchTimeout == 0 { config.BatchTimeout = time.Minute } - once.Do(func() { - // Initialize json provider. - jsonProvider = jsoniter.Config{ - EscapeHTML: true, - SortMapKeys: true, // Sort map keys to ensure deterministic output, to be consistent with encoding. - UseNumber: config.UseNumber, - }.Froze() - }) + md := metadata.Pairs(resourcePrefixHeader, database) if config.Compression == gzip.Name { md.Append(requestsCompressionHeader, gzip.Name) diff --git a/spanner/integration_test.go b/spanner/integration_test.go index 5e2522b32d5e..18d4bbad5b4d 100644 --- a/spanner/integration_test.go +++ b/spanner/integration_test.go @@ -2000,6 +2000,8 @@ func TestIntegration_BasicTypes(t *testing.T) { JSONArray ARRAY ) PRIMARY KEY (RowID)`, } + client, _, cleanup := prepareIntegrationTest(ctx, t, DefaultSessionPoolConfig, stmts) + defer cleanup() t1, _ := time.Parse(time.RFC3339Nano, "2016-11-15T15:04:05.999999999Z") // Boundaries @@ -2167,20 +2169,10 @@ func TestIntegration_BasicTypes(t *testing.T) { }...) } - var dbPath string for idx := 0; idx < 2; idx++ { - var c *Client - if idx == 0 { - var cleanup func() - c, dbPath, cleanup = prepareIntegrationTest(ctx, t, DefaultSessionPoolConfig, stmts) - defer cleanup() - } else { - var err error - c, err = createClient(ctx, dbPath, ClientConfig{UseNumber: true, SessionPoolConfig: DefaultSessionPoolConfig}) - require.NoError(t, err) - defer c.Close() - enableJSONProviderNumberConfig(true) - defer enableJSONProviderNumberConfig(false) + if idx == 1 { + UseNumberWithJSONDecoderEncoder() + defer testSetJSONProviderNumberConfig(false) } // Write rows into table first using DML. statements := make([]Statement, 0) @@ -2192,7 +2184,7 @@ func TestIntegration_BasicTypes(t *testing.T) { stmt.Params["value"] = test.val statements = append(statements, stmt) } - _, err := c.ReadWriteTransaction(ctx, func(ctx context.Context, tx *ReadWriteTransaction) error { + _, err := client.ReadWriteTransaction(ctx, func(ctx context.Context, tx *ReadWriteTransaction) error { rowCounts, err := tx.BatchUpdate(ctx, statements) if err != nil { return err @@ -2211,7 +2203,7 @@ func TestIntegration_BasicTypes(t *testing.T) { t.Fatalf("failed to insert values using DML: %v", err) } // Delete all the rows so we can insert them using mutations as well. - _, err = c.Apply(ctx, []*Mutation{Delete("Types", AllKeys())}) + _, err = client.Apply(ctx, []*Mutation{Delete("Types", AllKeys())}) if err != nil { t.Fatalf("failed to delete all rows: %v", err) } @@ -2221,12 +2213,12 @@ func TestIntegration_BasicTypes(t *testing.T) { for i, test := range tests { muts = append(muts, InsertOrUpdate("Types", []string{"RowID", test.col}, []interface{}{i, test.val})) } - if _, err := c.Apply(ctx, muts, ApplyAtLeastOnce()); err != nil { + if _, err := client.Apply(ctx, muts, ApplyAtLeastOnce()); err != nil { t.Fatal(err) } for i, test := range tests { - row, err := c.Single().ReadRow(ctx, "Types", []interface{}{i}, []string{test.col}) + row, err := client.Single().ReadRow(ctx, "Types", []interface{}{i}, []string{test.col}) if err != nil { t.Fatalf("Unable to fetch row %v: %v", i, err) } @@ -2258,10 +2250,9 @@ func TestIntegration_BasicTypes(t *testing.T) { } } // cleanup - _, err = c.Apply(ctx, []*Mutation{Delete("Types", AllKeys())}) + _, err = client.Apply(ctx, []*Mutation{Delete("Types", AllKeys())}) require.NoError(t, err) } - } // Test decoding Cloud Spanner STRUCT type. @@ -5549,7 +5540,7 @@ func checkCommonTagsGFELatency(t *testing.T, m map[tag.Key]string) { } // helper method to enable json provider with useNumber flag, only for testing. -func enableJSONProviderNumberConfig(useNumber bool) { +func testSetJSONProviderNumberConfig(useNumber bool) { jsonProvider = jsoniter.Config{ EscapeHTML: true, SortMapKeys: true, // Sort map keys to ensure deterministic output, to be consistent with encoding. diff --git a/spanner/value.go b/spanner/value.go index eb26bfed7a4f..9245a714551a 100644 --- a/spanner/value.go +++ b/spanner/value.go @@ -28,9 +28,7 @@ import ( "strconv" "strings" "sync" - "sync/atomic" "time" - "unsafe" "cloud.google.com/go/civil" "cloud.google.com/go/internal/fields" @@ -117,21 +115,22 @@ var ( jsonNullBytes = []byte("null") - jsonProvider jsoniter.API + jsonProvider = jsoniter.ConfigCompatibleWithStandardLibrary once sync.Once ) -// UseNumber specifies whether number values inside a Cloud Spanner JSON value -// should be decoded as a Number or a float64. -// Decoding to a Number guarantees that the precision used by Cloud Spanner is preserved. -// Decoding to a float64 can cause loss of precision. -// The default JSON decode function in Go uses float64, This is therefore also the default used by this client library. -// Change this value to true to prevent loss of precision. +// UseNumberWithJSONDecoderEncoder specifies whether Cloud Spanner JSON numbers are decoded +// as Number (preserving precision) or float64 (risking loss). +// Defaults to float64, call this method for lossless precision. +// NOTE: This change affects all clients created by this library, both existing and future ones. func UseNumberWithJSONDecoderEncoder() { - atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&jsonProvider))) once.Do(func() { - jsonProvider = jsoniter.ConfigCompatibleWithStandardLibrary + jsonProvider = jsoniter.Config{ + EscapeHTML: true, + SortMapKeys: true, // Sort map keys to ensure deterministic output, to be consistent with encoding. + UseNumber: true, + }.Froze() }) }