diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_decoding.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_decoding.go index b0e9b78dba5a..14aa54d545cf 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_decoding.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_decoding.go @@ -55,16 +55,18 @@ func DecodeStmtStatsStatisticsJSON(jsonVal json.JSON, result *roachpb.StatementS func JSONToExplainTreePlanNode(jsonVal json.JSON) (*roachpb.ExplainTreePlanNode, error) { node := roachpb.ExplainTreePlanNode{} - nameAttr, err := safeFetchVal(jsonVal, "Name") + nameAttr, err := jsonVal.FetchValKey("Name") if err != nil { return nil, err } - str, err := nameAttr.AsText() - if err != nil { - return nil, err + if nameAttr != nil { + str, err := nameAttr.AsText() + if err != nil { + return nil, err + } + node.Name = *str } - node.Name = *str iter, err := jsonVal.ObjectIter() if err != nil { @@ -86,15 +88,17 @@ func JSONToExplainTreePlanNode(jsonVal json.JSON) (*roachpb.ExplainTreePlanNode, if key == "Children" { for childIdx := 0; childIdx < value.Len(); childIdx++ { - childJSON, err := safeFetchValIdx(value, childIdx) + childJSON, err := value.FetchValIdx(childIdx) if err != nil { return nil, err } - child, err := JSONToExplainTreePlanNode(childJSON) - if err != nil { - return nil, err + if childJSON != nil { + child, err := JSONToExplainTreePlanNode(childJSON) + if err != nil { + return nil, err + } + node.Children = append(node.Children, child) } - node.Children = append(node.Children, child) } } else { str, err := value.AsText() diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go index ec0fb71d5592..975bb8f7cf01 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go @@ -146,6 +146,121 @@ func TestSQLStatsJsonEncoding(t *testing.T) { require.Equal(t, input, actualJSONUnmarshalled) }) + // When a new statistic is added to a statement payload, older versions won't have the + // new parameter, so this test is to confirm that all other parameters will be set and + // the new one will be empty, without breaking the decoding process. + t.Run("statement_statistics with new parameter", func(t *testing.T) { + data := genRandomData() + expectedStatistics := roachpb.CollectedStatementStatistics{} + + expectedMetadataStrTemplate := ` + { + "stmtTyp": "{{.String}}", + "query": "{{.String}}", + "db": "{{.String}}", + "distsql": {{.Bool}}, + "failed": {{.Bool}}, + "implicitTxn": {{.Bool}}, + "vec": {{.Bool}}, + "fullScan": {{.Bool}} + } + ` + expectedStatisticsStrTemplate := ` + { + "statistics": { + "cnt": {{.Int64}}, + "firstAttemptCnt": {{.Int64}}, + "maxRetries": {{.Int64}}, + "lastExecAt": "{{stringifyTime .Time}}", + "numRows": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "parseLat": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "planLat": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "runLat": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "svcLat": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "ovhLat": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "bytesRead": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "rowsRead": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "rowsWritten": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "nodes": [{{joinInts .IntArray}}] + }, + "execution_statistics": { + "cnt": {{.Int64}}, + "networkBytes": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "maxMemUsage": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "contentionTime": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "networkMsgs": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + }, + "maxDiskUsage": { + "mean": {{.Float}}, + "sqDiff": {{.Float}} + } + } + } + ` + + fillTemplate(t, expectedMetadataStrTemplate, data) + fillTemplate(t, expectedStatisticsStrTemplate, data) + fillObject(t, reflect.ValueOf(&expectedStatistics), &data) + + actualMetadataJSON, err := BuildStmtMetadataJSON(&expectedStatistics) + require.NoError(t, err) + actualStatisticsJSON, err := BuildStmtStatisticsJSON(&expectedStatistics.Stats) + require.NoError(t, err) + + var actualJSONUnmarshalled roachpb.CollectedStatementStatistics + + err = DecodeStmtStatsMetadataJSON(actualMetadataJSON, &actualJSONUnmarshalled) + require.NoError(t, err) + + // Remove one of the statistics on the object so its value doesn't get populated on + // the final actualJSONUnmarshalled.Stats. + actualStatisticsJSON, _, _ = actualStatisticsJSON.RemovePath([]string{"statistics", "numRows"}) + // Initialize the field again to remove the existing value. + expectedStatistics.Stats.NumRows = roachpb.NumericStat{} + + err = DecodeStmtStatsStatisticsJSON(actualStatisticsJSON, &actualJSONUnmarshalled.Stats) + require.NoError(t, err) + require.Equal(t, expectedStatistics, actualJSONUnmarshalled) + }) + t.Run("transaction_statistics", func(t *testing.T) { data := genRandomData() diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go index eabd5fe97e80..9dae476ee0bd 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_impl.go @@ -296,13 +296,15 @@ func (jf jsonFields) decodeJSON(js json.JSON) (err error) { for i := range jf { fieldName = jf[i].field - field, err := safeFetchVal(js, fieldName) + field, err := js.FetchValKey(fieldName) if err != nil { return err } - err = jf[i].val.decodeJSON(field) - if err != nil { - return err + if field != nil { + err = jf[i].val.decodeJSON(field) + if err != nil { + return err + } } } @@ -437,25 +439,3 @@ func (d *decimal) decodeJSON(js json.JSON) error { func (d *decimal) encodeJSON() (json.JSON, error) { return json.FromDecimal(*(*apd.Decimal)(d)), nil } - -func safeFetchVal(jsonVal json.JSON, key string) (json.JSON, error) { - field, err := jsonVal.FetchValKey(key) - if err != nil { - return nil, err - } - if field == nil { - return nil, errors.Newf("%s field is not found in the JSON payload", key) - } - return field, nil -} - -func safeFetchValIdx(jsonVal json.JSON, idx int) (json.JSON, error) { - field, err := jsonVal.FetchValIdx(idx) - if err != nil { - return nil, err - } - if field == nil { - return nil, errors.Newf("%dth element is not found in the JSON payload", idx) - } - return field, nil -}