Merge pull request #113800 from DrewKimball/backport22.2.17-rc-113712

release-22.2.17-rc: sql/stats: don't use linear regression with NaN for stats forecasting

pkg/sql/stats/forecast.go

@@ -411,6 +411,9 @@ func predictHistogram(
 	// Construct a linear regression model of quantile functions over time, and
 	// use it to predict a quantile function at the given time.
 	yₙ, r2 := quantileSimpleLinearRegression(createdAts, quantiles, forecastAt)
+	if yₙ.isInvalid() {
+		return histogram{}, errors.Newf("predicted histogram contains overflow values")
+	}
 	yₙ = yₙ.fixMalformed()
 	log.VEventf(
 		ctx, 3, "forecast for table %v columns %v predicted quantile %v R² %v",

pkg/sql/stats/quantile.go

@@ -656,6 +656,8 @@ func (q quantile) integrateSquared() float64 {
 // This function fixes the negative slope pieces by "moving" p from the
 // overlapping places to where it should be. No p is lost in the making of these
 // calculations.
+//
+// fixMalformed should not be called without checking that isInvalid() is false.
 func (q quantile) fixMalformed() quantile {
 	// Check for the happy case where q is already well-formed.
 	if q.isWellFormed() {
@@ -803,6 +805,9 @@ func (q quantile) fixMalformed() quantile {
 		// "below" our horizontal line (<= v) and ends "above" our horizontal line
 		// (> v) and we always add two intersectionPs for "double roots" touching
 		// our line from above (one endpoint > v, one endpoint = v).
+		if len(intersectionPs) == 0 {
+			return q
+		}
 		lessEqP := intersectionPs[0]
 		for j := 1; j < len(intersectionPs); j += 2 {
 			lessEqP += intersectionPs[j+1] - intersectionPs[j]
@@ -821,6 +826,16 @@ func (q quantile) fixMalformed() quantile {
 	return fixed
 }
 
+// isInvalid returns true if q contains NaN or Inf values.
+func (q quantile) isInvalid() bool {
+	for i := range q {
+		if math.IsNaN(q[i].v) || math.IsInf(q[i].v, 0) {
+			return true
+		}
+	}
+	return false
+}
+
 // isWellFormed returns true if q is well-formed (i.e. is non-decreasing in v).
 func (q quantile) isWellFormed() bool {
 	for i := 1; i < len(q); i++ {

pkg/sql/stats/quantile_test.go

@@ -1417,3 +1417,107 @@ func TestQuantileOpsRandom(t *testing.T) {
 		})
 	}
 }
+
+// TestQuantileInvalid tests the quantile.invalid() method.
+func TestQuantileInvalid(t *testing.T) {
+	testCases := []struct {
+		q       quantile
+		invalid bool
+	}{
+		// Valid quantiles.
+		{
+			q: zeroQuantile,
+		},
+		{
+			q: quantile{{0, 100}, {1, 200}},
+		},
+		{
+			q: quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, 400}, {1, 400}},
+		},
+		{
+			q: quantile{{0, -811}, {0.125, -811}, {0.125, -813}, {0.25, -813}, {0.25, -815}, {1, -821}, {1, -721}},
+		},
+		{
+			q: quantile{{0, 102}, {0.125, 102.25}, {0.25, 202.5}, {0.375, 302.75}, {0.375, 202.75}, {0.5, 103}, {0.625, 103.25}, {0.75, 3.5}, {0.875, 3.75}, {0.875, 103.75}, {0.875, 403.75}, {1, 104}},
+		},
+		// Invalid quantiles with NaN.
+		{
+			q:       quantile{{0, 100}, {1, math.NaN()}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, math.NaN()}, {1, 200}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, 400}, {1, math.NaN()}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, math.NaN()}, {1, math.NaN()}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, math.NaN()}, {0.125, -811}, {0.125, -813}, {0.25, -813}, {0.25, -815}, {1, -821}, {1, -721}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, math.NaN()}, {0.125, math.NaN()}, {0.125, -813}, {0.25, -813}, {0.25, -815}, {1, -821}, {1, -721}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 102}, {0.125, 102.25}, {0.25, math.NaN()}, {0.375, 302.75}, {0.375, 202.75}, {0.5, 103}, {0.625, 103.25}, {0.75, 3.5}, {0.875, 3.75}, {0.875, 103.75}, {0.875, 403.75}, {1, 104}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 102}, {0.125, 102.25}, {0.25, math.NaN()}, {0.375, 302.75}, {0.375, math.NaN()}, {0.5, 103}, {0.625, math.NaN()}, {0.75, 3.5}, {0.875, 3.75}, {0.875, math.NaN()}, {0.875, 403.75}, {1, 104}},
+			invalid: true,
+		},
+		// Invalid quantiles with Inf.
+		{
+			q:       quantile{{0, 100}, {1, math.Inf(1)}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, math.Inf(1)}, {1, 200}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, 400}, {1, math.Inf(1)}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, math.Inf(1)}, {1, math.Inf(1)}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {1, math.Inf(-1)}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, math.Inf(-1)}, {1, 200}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, 400}, {1, math.Inf(-1)}},
+			invalid: true,
+		},
+		{
+			q:       quantile{{0, 100}, {0, 200}, {0, 200}, {0.5, 300}, {0.5, 300}, {0.5, math.Inf(-1)}, {1, math.Inf(-1)}},
+			invalid: true,
+		},
+		// Invalid quantiles with NaN and Inf.
+		{
+			q:       quantile{{0, math.Inf(-1)}, {0.125, 102.25}, {0.25, math.NaN()}, {0.375, 302.75}, {0.375, math.Inf(1)}, {0.5, 103}, {0.625, 103.25}, {0.75, math.NaN()}, {0.875, math.Inf(1)}, {0.875, math.Inf(-1)}, {0.875, 403.75}, {1, 104}},
+			invalid: true,
+		},
+	}
+	for i, tc := range testCases {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			invalid := tc.q.isInvalid()
+			if invalid != tc.invalid {
+				t.Errorf("test case %d expected invalid to be %v, but was %v", i, tc.invalid, invalid)
+			}
+		})
+	}
+}

pkg/sql/stats/stats_cache.go

@@ -35,6 +35,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/cache"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -222,11 +223,24 @@ func decodeTableStatisticsKV(
 // The statistics are ordered by their CreatedAt time (newest-to-oldest).
 func (sc *TableStatisticsCache) GetTableStats(
 	ctx context.Context, table catalog.TableDescriptor,
-) ([]*TableStatistic, error) {
+) (stats []*TableStatistic, err error) {
 	if !statsUsageAllowed(table, sc.Settings) {
 		return nil, nil
 	}
 	forecast := forecastAllowed(table, sc.Settings)
+	defer func() {
+		if r := recover(); r != nil {
+			// In the event of a "safe" panic, we only want to log the error and
+			// continue executing the query without stats for this table.
+			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)
+			}
+		}
+	}()
 	return sc.getTableStatsFromCache(ctx, table.GetID(), &forecast)
 }