Skip to content

Commit

Permalink
Merge pull request #58263 from RaduBerinde/backport20.2-58221
Browse files Browse the repository at this point in the history
release-20.2: sql: catch panics from SHOW STATISTICS code
  • Loading branch information
RaduBerinde authored Dec 24, 2020
2 parents 49115ed + 6628daf commit a1768fe
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
20 changes: 19 additions & 1 deletion pkg/sql/show_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/stats"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/errorutil"
"github.com/cockroachdb/cockroach/pkg/util/json"
"github.com/cockroachdb/errors"
)
Expand Down Expand Up @@ -58,7 +59,7 @@ func (p *planner) ShowTableStats(ctx context.Context, n *tree.ShowTableStats) (p
return &delayedNode{
name: n.String(),
columns: columns,
constructor: func(ctx context.Context, p *planner) (planNode, error) {
constructor: func(ctx context.Context, p *planner) (_ planNode, err error) {
// We need to query the table_statistics and then do some post-processing:
// - convert column IDs to column names
// - if the statistic has a histogram, we return the statistic ID as a
Expand Down Expand Up @@ -96,6 +97,23 @@ func (p *planner) ShowTableStats(ctx context.Context, n *tree.ShowTableStats) (p
numCols
)

// Guard against crashes in the code below (e.g. #56356).
defer func() {
if r := recover(); r != nil {
// This code allows us to propagate internal errors without having to add
// error checks everywhere throughout the code. This is only possible
// because the code does not update shared state and does not manipulate
// locks.
if ok, e := errorutil.ShouldCatch(r); ok {
err = e
} else {
// Other panic objects can't be considered "safe" and thus are
// propagated as crashes that terminate the session.
panic(r)
}
}
}()

v := p.newContainerValuesNode(columns, 0)
if n.UsingJSON {
result := make([]stats.JSONStatistic, len(rows))
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/stats/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ type JSONHistoBucket struct {
// SetHistogram fills in the HistogramColumnType and HistogramBuckets fields.
func (js *JSONStatistic) SetHistogram(h *HistogramData) error {
typ := h.ColumnType
if typ == nil {
return fmt.Errorf("histogram type is unset")
}
js.HistogramColumnType = typ.SQLString()
js.HistogramBuckets = make([]JSONHistoBucket, len(h.Buckets))
var a rowenc.DatumAlloc
Expand All @@ -63,6 +66,9 @@ func (js *JSONStatistic) SetHistogram(h *HistogramData) error {
js.HistogramBuckets[i].NumRange = b.NumRange
js.HistogramBuckets[i].DistinctRange = b.DistinctRange

if b.UpperBound == nil {
return fmt.Errorf("histogram bucket upper bound is unset")
}
datum, _, err := rowenc.DecodeTableKey(&a, typ, b.UpperBound, encoding.Ascending)
if err != nil {
return err
Expand Down

0 comments on commit a1768fe

Please sign in to comment.