From 9a07e7a88e4e7160426b88cea1eea4bd70fa943b Mon Sep 17 00:00:00 2001 From: Yevgeniy Miretskiy Date: Tue, 13 Sep 2022 12:56:22 -0400 Subject: [PATCH 1/2] tree: Add a JSON encoding benchmark Add a micro benchmark for `tree.AsJSON` method. Release note: None Release justification: test only change --- pkg/sql/sem/tree/BUILD.bazel | 1 + pkg/sql/sem/tree/json_test.go | 116 ++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 pkg/sql/sem/tree/json_test.go diff --git a/pkg/sql/sem/tree/BUILD.bazel b/pkg/sql/sem/tree/BUILD.bazel index 940ecaae9657..cda5d7950950 100644 --- a/pkg/sql/sem/tree/BUILD.bazel +++ b/pkg/sql/sem/tree/BUILD.bazel @@ -184,6 +184,7 @@ go_test( "function_name_test.go", "indexed_vars_test.go", "interval_test.go", + "json_test.go", "main_test.go", "name_part_test.go", "name_resolution_test.go", diff --git a/pkg/sql/sem/tree/json_test.go b/pkg/sql/sem/tree/json_test.go new file mode 100644 index 000000000000..89b4d9d472fd --- /dev/null +++ b/pkg/sql/sem/tree/json_test.go @@ -0,0 +1,116 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package tree_test + +import ( + "math/rand" + "testing" + "time" + + "github.com/cockroachdb/cockroach/pkg/sql/randgen" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" + "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/util/randutil" +) + +func BenchmarkAsJSON(b *testing.B) { + // Use fixed seed so that each invocation of this benchmark + // produces exactly the same types, and datums streams. + // This number can be changed to an arbitrary value; doing so + // would result in new types/datums being produced. + rng := randutil.NewTestRandWithSeed(-4365865412074131521) + + const numDatums = 1024 + makeDatums := func(typ *types.T) tree.Datums { + const allowNulls = true + res := make(tree.Datums, numDatums) + for i := 0; i < numDatums; i++ { + res[i] = randgen.RandDatum(rng, typ, allowNulls) + } + return res + } + + bench := func(b *testing.B, typ *types.T) { + b.ReportAllocs() + b.StopTimer() + datums := makeDatums(typ) + b.StartTimer() + + for i := 0; i < b.N; i++ { + _, err := tree.AsJSON(datums[i%numDatums], sessiondatapb.DataConversionConfig{}, time.UTC) + if err != nil { + b.Fatal(err) + } + } + } + + for _, typ := range testTypes(rng) { + b.Run(typ.String(), func(b *testing.B) { + bench(b, typ) + }) + + if randgen.IsAllowedForArray(typ) { + typ = types.MakeArray(typ) + b.Run(typ.String(), func(b *testing.B) { + bench(b, typ) + }) + } + } +} + +// testTypes returns list of types to test against. +func testTypes(rng *rand.Rand) (typs []*types.T) { + for _, typ := range randgen.SeedTypes { + switch typ { + case types.AnyTuple: + // Ignore AnyTuple -- it's not very interesting; we'll generate test tuples below. + case types.RegClass, types.RegNamespace, types.RegProc, types.RegProcedure, types.RegRole, types.RegType: + // Ignore a bunch of pseudo-OID types (just want regular OID). + case types.Geometry, types.Geography: + // Ignore geometry/geography: these types are insanely inefficient; + // AsJson(Geo) -> MarshalGeo -> go JSON bytes -> ParseJSON -> Go native -> json.JSON + // Benchmarking this generates too much noise. + // TODO(yevgeniy): fix this. + default: + typs = append(typs, typ) + } + } + + // Add tuple types. + var tupleTypes []*types.T + makeTupleType := func() *types.T { + contents := make([]*types.T, rng.Intn(6)) // Up to 6 fields + for i := range contents { + contents[i] = randgen.RandTypeFromSlice(rng, typs) + } + candidateTuple := types.MakeTuple(contents) + // Ensure tuple type is unique. + for _, t := range tupleTypes { + if t.Equal(candidateTuple) { + return nil + } + } + tupleTypes = append(tupleTypes, candidateTuple) + return candidateTuple + } + + const numTupleTypes = 5 + for i := 0; i < numTupleTypes; i++ { + var typ *types.T + for typ == nil { + typ = makeTupleType() + } + typs = append(typs, typ) + } + + return typs +} From 35cb15efa7f4d0edc1a08582c518b52d41632fb4 Mon Sep 17 00:00:00 2001 From: Yevgeniy Miretskiy Date: Wed, 14 Sep 2022 17:00:40 -0400 Subject: [PATCH 2/2] tree: Improve performance of tree.AsJSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve performance of `tree.AsJSON` method. These improvements are important for any query that produces large number of JSON objects, as well as to changefeeds, which rely on this function when producing JSON encoded feed. Most of the changes revolved around modifying underlying types (s.a. date/timestamp types, box2d, etc) to favor using functions that append to bytes buffer, instead of relying on slower functions, such as `fmt.Sprintf`. The conversion performance improved around 5-10% for most of the types, and as high as 50% for time types: ``` Benchmark old t/op new t/op delta AsJSON/box2d-10 578ns ± 3% 414ns ± 2% -28.49% (p=0.000 n=10+9) AsJSON/box2d[]-10 1.64µs ± 3% 1.19µs ± 4% -27.14% (p=0.000 n=10+10) AsJSON/time-10 232ns ± 2% 103ns ± 1% -55.61% (p=0.000 n=10+10) AsJSON/time[]-10 687ns ± 4% 342ns ± 4% -50.17% (p=0.000 n=10+10) ``` Note: Some types in the local benchmark show slight slow down in speed. No changes were made in those types, and in general, the encoding speed of these types might be too fast to reliable detect changes: ``` Benchmark old t/op new t/op delta AsJSON/bool[]-10 65.9ns ± 1% 67.7ns ± 2% +2.79% (p=0.001 n=8+9) ``` The emphasis was also placed on reducing allocations. By relying more heavily on a pooled FmtCtx, which contains bytes buffer, some conversions resulted in amortized elimination of allocations (time): ``` Benchmark old B/op new t/op delta AsJSON/timestamp-10 42.1B ± 3% 0.0B -100.00% (p=0.000 n=10+10) AsJSON/timestamp[]-10 174B ± 4% 60B ± 1% -65.75% (p=0.000 n=10+10) ``` Release Note: None Release Justification: performance improvement --- pkg/BUILD.bazel | 4 ++++ pkg/geo/bbox.go | 24 +++++++++++-------- pkg/geo/bbox_test.go | 4 ++++ pkg/sql/sem/tree/datum.go | 31 +++++++++++++++++-------- pkg/sql/sem/tree/format.go | 11 +++++++++ pkg/util/strutil/BUILD.bazel | 19 ++++++++++++++++ pkg/util/strutil/util.go | 34 +++++++++++++++++++++++++++ pkg/util/strutil/util_test.go | 34 +++++++++++++++++++++++++++ pkg/util/timeofday/BUILD.bazel | 1 + pkg/util/timeofday/time_of_day.go | 21 +++++++++++++---- pkg/util/timetz/BUILD.bazel | 1 + pkg/util/timetz/timetz.go | 38 +++++++++++++++++++------------ pkg/util/uint128/uint128.go | 7 ++++++ pkg/util/uuid/uuid.go | 7 ++++-- 14 files changed, 196 insertions(+), 40 deletions(-) create mode 100644 pkg/util/strutil/BUILD.bazel create mode 100644 pkg/util/strutil/util.go create mode 100644 pkg/util/strutil/util_test.go diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 5b25d8d1128d..2b6f769e6fbe 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -582,6 +582,7 @@ ALL_TESTS = [ "//pkg/util/span:span_test", "//pkg/util/stop:stop_test", "//pkg/util/stringarena:stringarena_test", + "//pkg/util/strutil:strutil_test", "//pkg/util/syncutil/singleflight:singleflight_test", "//pkg/util/syncutil:syncutil_test", "//pkg/util/sysutil:sysutil_test", @@ -2002,6 +2003,8 @@ GO_TARGETS = [ "//pkg/util/stringarena:stringarena", "//pkg/util/stringarena:stringarena_test", "//pkg/util/stringencoding:stringencoding", + "//pkg/util/strutil:strutil", + "//pkg/util/strutil:strutil_test", "//pkg/util/syncutil/singleflight:singleflight", "//pkg/util/syncutil/singleflight:singleflight_test", "//pkg/util/syncutil:syncutil", @@ -2951,6 +2954,7 @@ GET_X_DATA_TARGETS = [ "//pkg/util/stop:get_x_data", "//pkg/util/stringarena:get_x_data", "//pkg/util/stringencoding:get_x_data", + "//pkg/util/strutil:get_x_data", "//pkg/util/syncutil:get_x_data", "//pkg/util/syncutil/singleflight:get_x_data", "//pkg/util/system:get_x_data", diff --git a/pkg/geo/bbox.go b/pkg/geo/bbox.go index bc25ec25189d..b407bb80d37c 100644 --- a/pkg/geo/bbox.go +++ b/pkg/geo/bbox.go @@ -38,15 +38,21 @@ func NewCartesianBoundingBox() *CartesianBoundingBox { // Repr is the string representation of the CartesianBoundingBox. func (b *CartesianBoundingBox) Repr() string { - // fmt.Sprintf with %f does not truncate leading zeroes, so use - // FormatFloat instead. - return fmt.Sprintf( - "BOX(%s %s,%s %s)", - strconv.FormatFloat(b.LoX, 'f', -1, 64), - strconv.FormatFloat(b.LoY, 'f', -1, 64), - strconv.FormatFloat(b.HiX, 'f', -1, 64), - strconv.FormatFloat(b.HiY, 'f', -1, 64), - ) + return string(b.AppendFormat(nil)) +} + +// AppendFormat appends string representation of the CartesianBoundingBox +// to the buffer, and returns modified buffer. +func (b *CartesianBoundingBox) AppendFormat(buf []byte) []byte { + buf = append(buf, "BOX("...) + buf = strconv.AppendFloat(buf, b.LoX, 'f', -1, 64) + buf = append(buf, ' ') + buf = strconv.AppendFloat(buf, b.LoY, 'f', -1, 64) + buf = append(buf, ',') + buf = strconv.AppendFloat(buf, b.HiX, 'f', -1, 64) + buf = append(buf, ' ') + buf = strconv.AppendFloat(buf, b.HiY, 'f', -1, 64) + return append(buf, ')') } // ParseCartesianBoundingBox parses a box2d string into a bounding box. diff --git a/pkg/geo/bbox_test.go b/pkg/geo/bbox_test.go index 0ef94e9d7e6b..f099fb1e6bc7 100644 --- a/pkg/geo/bbox_test.go +++ b/pkg/geo/bbox_test.go @@ -14,6 +14,7 @@ import ( "fmt" "math" "strconv" + "strings" "testing" "github.com/cockroachdb/cockroach/pkg/geo/geopb" @@ -63,6 +64,9 @@ func TestParseCartesianBoundingBox(t *testing.T) { } else { require.NoError(t, err) require.Equal(t, tc.expected, ret) + // Test Repr/AppendFormat round trip. + require.Equal(t, strings.ToUpper(tc.s), ret.Repr()) + require.Equal(t, strings.ToUpper(tc.s), string(ret.AppendFormat(nil))) } }) } diff --git a/pkg/sql/sem/tree/datum.go b/pkg/sql/sem/tree/datum.go index 2cb20e605141..0f41f072344f 100644 --- a/pkg/sql/sem/tree/datum.go +++ b/pkg/sql/sem/tree/datum.go @@ -1782,7 +1782,11 @@ func (d *DUuid) Format(ctx *FmtCtx) { if !bareStrings { ctx.WriteByte('\'') } - ctx.WriteString(d.UUID.String()) + + buf := ctx.scratch[:uuid.RFC4122StrSize] + d.UUID.StringBytes(buf) + ctx.Write(buf) + if !bareStrings { ctx.WriteByte('\'') } @@ -2344,7 +2348,7 @@ func (d *DTime) Format(ctx *FmtCtx) { if !bareStrings { ctx.WriteByte('\'') } - ctx.WriteString(timeofday.TimeOfDay(*d).String()) + ctx.Write(timeofday.TimeOfDay(*d).AppendFormat(ctx.scratch[:0])) if !bareStrings { ctx.WriteByte('\'') } @@ -2520,7 +2524,7 @@ func (d *DTimeTZ) Format(ctx *FmtCtx) { if !bareStrings { ctx.WriteByte('\'') } - ctx.WriteString(d.TimeTZ.String()) + ctx.Write(d.TimeTZ.AppendFormat(ctx.scratch[:0])) if !bareStrings { ctx.WriteByte('\'') } @@ -3603,7 +3607,7 @@ func (d *DBox2D) Format(ctx *FmtCtx) { if !bareStrings { ctx.WriteByte('\'') } - ctx.WriteString(d.CartesianBoundingBox.Repr()) + ctx.Write(d.CartesianBoundingBox.AppendFormat(ctx.scratch[:0])) if !bareStrings { ctx.WriteByte('\'') } @@ -3731,10 +3735,10 @@ func AsJSON( // without the T separator. This causes some compatibility problems // with certain JSON consumers, so we'll use an alternate formatting // path here to maintain consistency with PostgreSQL. - return json.FromString(t.Time.In(loc).Format(time.RFC3339Nano)), nil + return json.FromString(formatTime(t.Time.In(loc), time.RFC3339Nano)), nil case *DTimestamp: // This is RFC3339Nano, but without the TZ fields. - return json.FromString(t.UTC().Format("2006-01-02T15:04:05.999999999")), nil + return json.FromString(formatTime(t.UTC(), "2006-01-02T15:04:05.999999999")), nil case *DDate, *DUuid, *DOid, *DInterval, *DBytes, *DIPAddr, *DTime, *DTimeTZ, *DBitArray, *DBox2D: return json.FromString(AsStringWithFlags(t, FmtBareStrings, FmtDataConversionConfig(dcc))), nil case *DGeometry: @@ -3752,6 +3756,16 @@ func AsJSON( } } +// formatTime formats time with specified layout. +// TODO(yuzefovich): consider using this function in more places. +func formatTime(t time.Time, layout string) string { + // We only need FmtCtx to access its buffer so + // that we get 0 amortized allocations. + ctx := NewFmtCtx(FmtSimple) + ctx.Write(t.AppendFormat(ctx.scratch[:0], layout)) + return ctx.CloseAndGetString() +} + // ResolvedType implements the TypedExpr interface. func (*DJSON) ResolvedType() *types.T { return types.Jsonb @@ -5060,14 +5074,13 @@ func (d *DOid) CompareError(ctx CompareContext, other Datum) (int, error) { // Format implements the Datum interface. func (d *DOid) Format(ctx *FmtCtx) { - s := strconv.FormatUint(uint64(d.Oid), 10) if d.semanticType.Oid() == oid.T_oid || d.name == "" { - ctx.WriteString(s) + ctx.Write(strconv.AppendUint(ctx.scratch[:0], uint64(d.Oid), 10)) } else if ctx.HasFlags(fmtDisambiguateDatumTypes) { ctx.WriteString("crdb_internal.create_") ctx.WriteString(d.semanticType.SQLStandardName()) ctx.WriteByte('(') - ctx.WriteString(s) + ctx.Write(strconv.AppendUint(ctx.scratch[:0], uint64(d.Oid), 10)) ctx.WriteByte(',') lexbase.EncodeSQLStringWithFlags(&ctx.Buffer, d.name, lexbase.EncNoFlags) ctx.WriteByte(')') diff --git a/pkg/sql/sem/tree/format.go b/pkg/sql/sem/tree/format.go index 192354f711e6..457c106b20ef 100644 --- a/pkg/sql/sem/tree/format.go +++ b/pkg/sql/sem/tree/format.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util" + "github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/cockroachdb/errors" "github.com/cockroachdb/redact" ) @@ -273,6 +274,8 @@ type FmtCtx struct { // indexedTypeFormatter is an optional interceptor for formatting // IDTypeReferences differently than normal. indexedTypeFormatter func(*FmtCtx, *OIDTypeReference) + // small scratch buffer to reduce allocations. + scratch [64]byte } // FmtCtxOption is an option to pass into NewFmtCtx. @@ -673,3 +676,11 @@ func (ctx *FmtCtx) formatNodeMaybeMarkRedaction(n NodeFormatter) { n.Format(ctx) } } + +func init() { + ctx := NewFmtCtx(FmtSimple) + if len(ctx.scratch) < uuid.RFC4122StrSize { + panic(errors.AssertionFailedf("FmtCtx scratch must be at least %d bytes", uuid.RFC4122StrSize)) + } + ctx.Close() +} diff --git a/pkg/util/strutil/BUILD.bazel b/pkg/util/strutil/BUILD.bazel new file mode 100644 index 000000000000..3d6bf1ae2fe5 --- /dev/null +++ b/pkg/util/strutil/BUILD.bazel @@ -0,0 +1,19 @@ +load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "strutil", + srcs = ["util.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/util/strutil", + visibility = ["//visibility:public"], +) + +go_test( + name = "strutil_test", + srcs = ["util_test.go"], + args = ["-test.timeout=295s"], + embed = [":strutil"], + deps = ["@com_github_stretchr_testify//require"], +) + +get_x_data(name = "get_x_data") diff --git a/pkg/util/strutil/util.go b/pkg/util/strutil/util.go new file mode 100644 index 000000000000..a1788a07b661 --- /dev/null +++ b/pkg/util/strutil/util.go @@ -0,0 +1,34 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package strutil + +import "strconv" + +// AppendInt appends the decimal form of x to b and returns the result. +// If the decimal form is shorter than width, the result is padded with leading 0's. +// If the decimal is longer than width, returns formatted decimal without +// any truncation. +func AppendInt(b []byte, x int, width int) []byte { + if x < 0 { + width-- + x = -x + b = append(b, '-') + } + + var scratch [16]byte + xb := strconv.AppendInt(scratch[:0], int64(x), 10) + + // Add 0-padding. + for w := len(xb); w < width; w++ { + b = append(b, '0') + } + return append(b, xb...) +} diff --git a/pkg/util/strutil/util_test.go b/pkg/util/strutil/util_test.go new file mode 100644 index 000000000000..8a94ccfe94f6 --- /dev/null +++ b/pkg/util/strutil/util_test.go @@ -0,0 +1,34 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. +package strutil + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestAppendInt(t *testing.T) { + for _, tc := range []struct { + n int + width int + fmt string + }{ + {0, 3, "%03d"}, + {5, 2, "%02d"}, + {10, 1, "%01d"}, + {-11, 4, "%04d"}, + {1234, 6, "%06d"}, + {-321, 2, "%02d"}, + } { + require.Equal(t, fmt.Sprintf(tc.fmt, tc.n), string(AppendInt(nil, tc.n, tc.width))) + } +} diff --git a/pkg/util/timeofday/BUILD.bazel b/pkg/util/timeofday/BUILD.bazel index a9424abf9319..0c723b337c29 100644 --- a/pkg/util/timeofday/BUILD.bazel +++ b/pkg/util/timeofday/BUILD.bazel @@ -8,6 +8,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/util/duration", + "//pkg/util/strutil", "//pkg/util/timeutil", ], ) diff --git a/pkg/util/timeofday/time_of_day.go b/pkg/util/timeofday/time_of_day.go index d3de2de87c96..04694e4de4fb 100644 --- a/pkg/util/timeofday/time_of_day.go +++ b/pkg/util/timeofday/time_of_day.go @@ -11,12 +11,12 @@ package timeofday import ( - "fmt" + "bytes" "math/rand" - "strings" "time" "github.com/cockroachdb/cockroach/pkg/util/duration" + "github.com/cockroachdb/cockroach/pkg/util/strutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" ) @@ -52,12 +52,23 @@ func New(hour, min, sec, micro int) TimeOfDay { } func (t TimeOfDay) String() string { + return string(t.AppendFormat(nil)) +} + +// AppendFormat appends this TimeOfDay format to the specified buffer. +func (t TimeOfDay) AppendFormat(buf []byte) []byte { + buf = strutil.AppendInt(buf, t.Hour(), 2) + buf = append(buf, ':') + buf = strutil.AppendInt(buf, t.Minute(), 2) + buf = append(buf, ':') + buf = strutil.AppendInt(buf, t.Second(), 2) micros := t.Microsecond() if micros > 0 { - s := fmt.Sprintf("%02d:%02d:%02d.%06d", t.Hour(), t.Minute(), t.Second(), micros) - return strings.TrimRight(s, "0") + buf = append(buf, '.') + buf = strutil.AppendInt(buf, micros, 6) + buf = bytes.TrimRight(buf, "0") } - return fmt.Sprintf("%02d:%02d:%02d", t.Hour(), t.Minute(), t.Second()) + return buf } // FromInt constructs a TimeOfDay from an int64, representing microseconds since diff --git a/pkg/util/timetz/BUILD.bazel b/pkg/util/timetz/BUILD.bazel index 9d99cc9b2e8d..4482dbdf540d 100644 --- a/pkg/util/timetz/BUILD.bazel +++ b/pkg/util/timetz/BUILD.bazel @@ -9,6 +9,7 @@ go_library( deps = [ "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", + "//pkg/util/strutil", "//pkg/util/timeofday", "//pkg/util/timeutil", "//pkg/util/timeutil/pgdate", diff --git a/pkg/util/timetz/timetz.go b/pkg/util/timetz/timetz.go index 9969649fc190..71d62fcc48ba 100644 --- a/pkg/util/timetz/timetz.go +++ b/pkg/util/timetz/timetz.go @@ -11,12 +11,12 @@ package timetz import ( - "fmt" "regexp" "time" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/util/strutil" "github.com/cockroachdb/cockroach/pkg/util/timeofday" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil/pgdate" @@ -150,25 +150,33 @@ func ParseTimeTZ( // String implements the Stringer interface. func (t *TimeTZ) String() string { + return string(t.AppendFormat(nil)) +} + +// AppendFormat appends TimeTZ to the buffer, and returns modified buffer. +func (t *TimeTZ) AppendFormat(buf []byte) []byte { tTime := t.ToTime() - timeComponent := tTime.Format("15:04:05.999999") - // 24:00:00 gets returned as 00:00:00, which is incorrect. if t.TimeOfDay == timeofday.Time2400 { - timeComponent = "24:00:00" + // 24:00:00 gets returned as 00:00:00, which is incorrect. + buf = append(buf, "24:00:00"...) + } else { + buf = tTime.AppendFormat(buf, "15:04:05.999999") } - timeZoneComponent := tTime.Format("Z07:00:00") - // If it is UTC, .Format converts it to "Z". - // Fully expand this component. + if t.OffsetSecs == 0 { - timeZoneComponent = "+00:00:00" - } - // Go's time.Format functionality does not work for offsets which - // in the range -0s < offsetSecs < -60s, e.g. -22s offset prints as 00:00:-22. - // Manually correct for this. - if 0 < t.OffsetSecs && t.OffsetSecs < 60 { - timeZoneComponent = fmt.Sprintf("-00:00:%02d", t.OffsetSecs) + // If it is UTC, .Format converts it to "Z". + // Fully expand this component. + buf = append(buf, "+00:00:00"...) + } else if 0 < t.OffsetSecs && t.OffsetSecs < 60 { + // Go's time.Format functionality does not work for offsets which + // in the range -0s < offsetSecs < -60s, e.g. -22s offset prints as 00:00:-22. + // Manually correct for this. + buf = append(buf, "-00:00:"...) + buf = strutil.AppendInt(buf, int(t.OffsetSecs), 2) + } else { + buf = tTime.AppendFormat(buf, "Z07:00:00") } - return timeComponent + timeZoneComponent + return buf } // ToTime converts a DTimeTZ to a time.Time, corrected to the given location. diff --git a/pkg/util/uint128/uint128.go b/pkg/util/uint128/uint128.go index e54e24cac611..86fff00e7b62 100644 --- a/pkg/util/uint128/uint128.go +++ b/pkg/util/uint128/uint128.go @@ -30,6 +30,13 @@ func (u Uint128) GetBytes() []byte { return buf } +// AppendBytes appends big-endian byte representation to the +// buffer and returns the buffer. +func (u Uint128) AppendBytes(buf []byte) []byte { + buf = binary.BigEndian.AppendUint64(buf, u.Hi) + return binary.BigEndian.AppendUint64(buf, u.Lo) +} + // String returns a hexadecimal string representation. func (u Uint128) String() string { return hex.EncodeToString(u.GetBytes()) diff --git a/pkg/util/uuid/uuid.go b/pkg/util/uuid/uuid.go index f161a6b72b87..fd0951511ac4 100644 --- a/pkg/util/uuid/uuid.go +++ b/pkg/util/uuid/uuid.go @@ -30,6 +30,9 @@ import ( // Size of a UUID in bytes. const Size = 16 +// RFC4122StrSize of a size of the RFC-4122 string representation of UUID. +const RFC4122StrSize = 36 + // Bytes represents a byte slice which is intended to be interpreted as a binary // encoding of a UUID. type Bytes []byte @@ -144,7 +147,7 @@ func (u *UUID) bytesMut() []byte { // String returns a canonical RFC-4122 string representation of the UUID: // xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx. func (u UUID) String() string { - buf := make([]byte, 36) + buf := make([]byte, RFC4122StrSize) u.StringBytes(buf) return string(buf) } @@ -152,7 +155,7 @@ func (u UUID) String() string { // StringBytes writes the result of String directly into a buffer, which must // have a length of at least 36. func (u UUID) StringBytes(buf []byte) { - _ = buf[:36] + _ = buf[:RFC4122StrSize] hex.Encode(buf[0:8], u[0:4]) buf[8] = '-' hex.Encode(buf[9:13], u[4:6])