From 4d0c38623d505529d9b3796a9d275c837a8ad8bc Mon Sep 17 00:00:00 2001 From: Azhng Date: Wed, 15 Sep 2021 16:55:38 -0400 Subject: [PATCH] sql: fix JSON deserialization for sql stats lastExecAt field Previsouly, lastExecAt field for roachpb.CollectedStatementStatistics was not properly updated. This caused the status API to return empty data for that field. This commit fixes the deserialization and extended the randomize testing framework to also test time.Time type. Partially Resolves #69675 Release Justification: Bug fixes and low-risk updates to new functionality Release note (bug fix): Last Execution Timestamp is now properly updating. --- .../sqlstatsutil/BUILD.bazel | 1 + .../sqlstatsutil/json_encoding_test.go | 2 +- .../sqlstatsutil/json_impl.go | 2 +- .../sqlstatsutil/testutils.go | 36 +++++++++++++------ 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/BUILD.bazel b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/BUILD.bazel index 168347bf8e74..ebcd42563330 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/BUILD.bazel +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//pkg/sql/sem/tree", "//pkg/util/encoding", "//pkg/util/json", + "//pkg/util/timeutil", "@com_github_cockroachdb_apd_v2//:apd", "@com_github_cockroachdb_errors//:errors", "@com_github_stretchr_testify//require", diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go index cc8918c0be59..1284fb3729be 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go @@ -59,7 +59,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { "cnt": {{.Int64}}, "firstAttemptCnt": {{.Int64}}, "maxRetries": {{.Int64}}, - "lastExecAt": "0001-01-01T00:00:00Z", + "lastExecAt": "{{stringifyTime .Time}}", "numRows": { "mean": {{.Float}}, "sqDiff": {{.Float}} diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go index ddee5e7b103f..cc040f3c97a0 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go @@ -334,7 +334,7 @@ func (t *jsonTime) decodeJSON(js json.JSON) error { return err } - tm := (time.Time)(*t) + tm := (*time.Time)(t) if err := tm.UnmarshalText([]byte(s)); err != nil { return err } diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go index 59d4094dcf4c..cc4364d878e1 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go @@ -17,8 +17,10 @@ import ( "strconv" "strings" "testing" + "time" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/stretchr/testify/require" ) @@ -42,6 +44,7 @@ type randomData struct { Int64 int64 Float float64 IntArray []int64 + Time time.Time } var alphabet = []rune("abcdefghijklmkopqrstuvwxyz") @@ -67,6 +70,7 @@ func genRandomData() randomData { r.IntArray[i] = rand.Int63() } + r.Time = timeutil.Now() return r } @@ -78,10 +82,16 @@ func fillTemplate(t *testing.T, tmplStr string, data randomData) string { } return strings.Join(strArr, ",") } + stringifyTime := func(tm time.Time) string { + s, err := tm.MarshalText() + require.NoError(t, err) + return string(s) + } tmpl, err := template. New(""). Funcs(template.FuncMap{ - "joinInts": joinInts, + "joinInts": joinInts, + "stringifyTime": stringifyTime, }). Parse(tmplStr) require.NoError(t, err) @@ -101,7 +111,6 @@ var fieldBlacklist = map[string]struct{}{ "SensitiveInfo": {}, "LegacyLastErr": {}, "LegacyLastErrRedacted": {}, - "LastExecTimestamp": {}, "StatementFingerprintIDs": {}, "AggregatedTs": {}, } @@ -130,15 +139,22 @@ func fillObject(t *testing.T, val reflect.Value, data *randomData) { val.Set(reflect.Append(val, reflect.ValueOf(randInt))) } case reflect.Struct: - numFields := val.NumField() - for i := 0; i < numFields; i++ { - fieldName := val.Type().Field(i).Name - fieldAddr := val.Field(i).Addr() - if _, ok := fieldBlacklist[fieldName]; ok { - continue + switch val.Type().Name() { + // Special handling time.Time. + case "Time": + val.Set(reflect.ValueOf(data.Time)) + return + default: + numFields := val.NumField() + for i := 0; i < numFields; i++ { + fieldName := val.Type().Field(i).Name + fieldAddr := val.Field(i).Addr() + if _, ok := fieldBlacklist[fieldName]; ok { + continue + } + + fillObject(t, fieldAddr, data) } - - fillObject(t, fieldAddr, data) } default: t.Fatalf("unsupported type: %s", val.Kind().String())