Merge #143858

143858: stats: harden bucket adjustment for over-estimates r=yuzefovich a=yuzefovich

We recently saw a case where we produced histogram buckets that contained NaNs. This happened because we had an over-estimate of the distinct count which was impossible to satisfy (3+ distinct count for boolean column). This commit hardens the bucket adjustment code to avoid generating NaNs in such scenarios. In particular, it audits all division arithmetic we do when building histograms to ensure that we never attempt to divide by zero - I found only one spot where it seems plausible in such "impossible to satisfy" request.

Fixes: #142022.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/sql/stats/histogram.go

@@ -448,6 +448,10 @@ type histogram struct {
 // to equal the total row count and estimated distinct count. The total row
 // count and estimated distinct count should not include NULL values, and the
 // histogram should not contain any buckets for NULL values.
+//
+// NB: it is **not** guaranteed that the returned histogram will contain the
+// specified distinct count (e.g., if distinctCountTotal of 3 or more is asked
+// from the boolean histogram).
 func (h *histogram) adjustCounts(
 	ctx context.Context,
 	compareCtx tree.CompareContext,
@@ -550,7 +554,7 @@ func (h *histogram) adjustCounts(
 					// values in the bucket.
 					inc = remDistinctCount * (maxDistRange / maxDistinctCountRange)
 					// If the bucket has DistinctRange > maxDistRange (a rare but possible
-					// occurence, see #93892) then inc will be negative. Prevent this.
+					// occurrence, see #93892) then inc will be negative. Prevent this.
 					if inc < 0 {
 						inc = 0
 					}
@@ -707,6 +711,9 @@ func getMaxVal(
 // histogram to include the remaining distinct values in remDistinctCount. It
 // also increments the counters rowCountEq, distinctCountEq, rowCountRange, and
 // distinctCountRange as needed.
+//
+// NB: it is **not** guaranteed that all remaining distinct values will be
+// covered.
 func (h *histogram) addOuterBuckets(
 	ctx context.Context,
 	compareCtx tree.CompareContext,
@@ -777,9 +784,13 @@ func (h *histogram) addOuterBuckets(
 
 		inc := avgRemPerBucket
 		if countable {
-			// Set the increment proportional to the remaining number of distinct
-			// values in the bucket.
-			inc = remDistinctCount * (maxDistRange / maxDistinctCountExtraBuckets)
+			if maxDistinctCountExtraBuckets > 0 {
+				// Set the increment proportional to the remaining number of distinct
+				// values in the bucket.
+				inc = remDistinctCount * (maxDistRange / maxDistinctCountExtraBuckets)
+			} else {
+				inc = 0
+			}
 			if inc < 0 {
 				inc = 0
 			}

pkg/sql/stats/histogram_test.go

@@ -940,6 +940,31 @@ func TestAdjustCounts(t *testing.T) {
 				{NumRange: 0, NumEq: 100, DistinctRange: 0, UpperBound: d(-60)},
 			},
 		},
+		{ // Over-estimate of distinct count for bools when both values were
+			// already sampled.
+			h: []cat.HistogramBucket{
+				{NumRange: 0, NumEq: 1, DistinctRange: 0, UpperBound: tree.DBoolFalse},
+				{NumRange: 0, NumEq: 1, DistinctRange: 0, UpperBound: tree.DBoolTrue},
+			},
+			rowCount:      4,
+			distinctCount: 4,
+			expected: []cat.HistogramBucket{
+				{NumRange: 0, NumEq: 2, DistinctRange: 0, UpperBound: tree.DBoolFalse},
+				{NumRange: 0, NumEq: 2, DistinctRange: 0, UpperBound: tree.DBoolTrue},
+			},
+		},
+		{ // Over-estimate of distinct count for bools when only 'false' was
+			// already sampled (#142022).
+			h: []cat.HistogramBucket{
+				{NumRange: 0, NumEq: 1, DistinctRange: 0, UpperBound: tree.DBoolFalse},
+			},
+			rowCount:      4,
+			distinctCount: 4,
+			expected: []cat.HistogramBucket{
+				{NumRange: 0, NumEq: 2, DistinctRange: 0, UpperBound: tree.DBoolFalse},
+				{NumRange: 0, NumEq: 2, DistinctRange: 0, UpperBound: tree.DBoolTrue},
+			},
+		},
 	}
 
 	ctx := context.Background()