From c154f0cdb087ad37014946ba6164f7cba1f24865 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Fri, 9 Aug 2024 11:46:02 +0300 Subject: [PATCH 1/2] perf(spanner): compare faster with json null value By using a string comparison the compiler can significantly optimize the check. --- spanner/value.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/spanner/value.go b/spanner/value.go index b258678748bf..33a78352509b 100644 --- a/spanner/value.go +++ b/spanner/value.go @@ -140,6 +140,11 @@ func jsonUnmarshal(data []byte, v any) error { return dec.Decode(v) } +// jsonIsNull returns whether v matches JSON null literal +func jsonIsNull(v []byte) bool { + return string(v) == "null" +} + // Encoder is the interface implemented by a custom type that can be encoded to // a supported type by Spanner. A code example: // @@ -220,7 +225,7 @@ func (n *NullInt64) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Int64 = int64(0) n.Valid = false return nil @@ -300,7 +305,7 @@ func (n *NullString) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.StringVal = "" n.Valid = false return nil @@ -385,7 +390,7 @@ func (n *NullFloat64) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Float64 = float64(0) n.Valid = false return nil @@ -465,7 +470,7 @@ func (n *NullFloat32) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Float32 = float32(0) n.Valid = false return nil @@ -545,7 +550,7 @@ func (n *NullBool) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Bool = false n.Valid = false return nil @@ -625,7 +630,7 @@ func (n *NullTime) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Time = time.Time{} n.Valid = false return nil @@ -710,7 +715,7 @@ func (n *NullDate) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Date = civil.Date{} n.Valid = false return nil @@ -795,7 +800,7 @@ func (n *NullNumeric) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Numeric = big.Rat{} n.Valid = false return nil @@ -892,7 +897,7 @@ func (n *NullJSON) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Valid = false return nil } @@ -940,7 +945,7 @@ func (n *PGNumeric) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Numeric = "" n.Valid = false return nil @@ -987,7 +992,7 @@ func (n *NullProtoMessage) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.ProtoMessageVal = nil n.Valid = false return nil @@ -1033,7 +1038,7 @@ func (n *NullProtoEnum) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.ProtoEnumVal = nil n.Valid = false return nil @@ -1094,7 +1099,7 @@ func (n *PGJsonB) UnmarshalJSON(payload []byte) error { if payload == nil { return fmt.Errorf("payload should not be nil") } - if bytes.Equal(payload, jsonNullBytes) { + if jsonIsNull(payload) { n.Valid = false return nil } From 416a7dbb25c1f04960c978ad2284f4601dc99eeb Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Fri, 9 Aug 2024 11:50:44 +0300 Subject: [PATCH 2/2] fix(spanner): avoid reusing []byte("null") The caller can change the result from these calls and this may cause weird bugs down the line. Instead allocate a new slice. --- spanner/value.go | 8 +++----- spanner/value_test.go | 9 +++++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/spanner/value.go b/spanner/value.go index 33a78352509b..cd22f53e7b06 100644 --- a/spanner/value.go +++ b/spanner/value.go @@ -114,8 +114,6 @@ var ( CommitTimestamp = commitTimestamp commitTimestamp = time.Unix(0, 0).In(time.FixedZone("CommitTimestamp placeholder", 0xDB)) - jsonNullBytes = []byte("null") - jsonUseNumber bool protoMsgReflectType = reflect.TypeOf((*proto.Message)(nil)).Elem() @@ -984,7 +982,7 @@ func (n NullProtoMessage) MarshalJSON() ([]byte, error) { if n.Valid { return json.Marshal(n.ProtoMessageVal) } - return jsonNullBytes, nil + return []byte("null"), nil } // UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON for NullProtoMessage. @@ -1030,7 +1028,7 @@ func (n NullProtoEnum) MarshalJSON() ([]byte, error) { if n.Valid && n.ProtoEnumVal != nil { return []byte(fmt.Sprintf("%v", n.ProtoEnumVal.Number())), nil } - return jsonNullBytes, nil + return []byte("null"), nil } // UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON for NullProtoEnum. @@ -1115,7 +1113,7 @@ func (n *PGJsonB) UnmarshalJSON(payload []byte) error { func nulljson(valid bool, v interface{}) ([]byte, error) { if !valid { - return jsonNullBytes, nil + return []byte("null"), nil } return json.Marshal(v) } diff --git a/spanner/value_test.go b/spanner/value_test.go index 2890b0a27633..b1b2e33e87f9 100644 --- a/spanner/value_test.go +++ b/spanner/value_test.go @@ -3266,3 +3266,12 @@ func expectUnmarshalNullableTypes(t *testing.T, err error, v interface{}, isNull t.Fatalf("Incorrect unmarshalling a json string to nullable types: got %q, want %q", v, expect) } } + +func TestNullJson(t *testing.T) { + v, _ := nulljson(false, nil) + v[0] = 'X' + v, _ = nulljson(false, nil) + if string(v) != "null" { + t.Fatalf("expected null, got %s", v) + } +}