sql/stats: avoid mutating input buckets in stripOuterBuckets

This commit changes `stripOuterBuckets` to modify and return a copy of
the given histogram buckets if it finds outer buckets to remove.
Previously, we would mutate the caller's histograms with leading outer
buckets in-place by zeroing the range counts on the first non-outer
bucket. This effectively corrupts the first histogram bucket in stats
passed in from the stats cache.

Although this bug has existed since 90e311d (which zeroes the first
buckets range counts), it would only impact full statistics that we
tried merging with partial stats, as that was the only case in which
`stripOuterBuckets` was called. The surface area of this bug increased
after db9a344, which calls `stripOuterBuckets` on every full statistic,
regardless of whether we end up merging it with partial stats or not.

Fixes: #155184

Release note (bug fix): Previously, we could corrupt the first bucket of
table statistic histograms in certain cases, causing underestimates for
range counts near the lower end of the domain, which is now fixed.

Author: Uzair5162

pkg/sql/logictest/testdata/logic_test/distsql_stats

@@ -3890,3 +3890,64 @@ SELECT jsonb_pretty(statistics->0->'histo_buckets') FROM
         "upper_bound": "c"
     }
 ]
+
+# Ensure that stripOuterBuckets doesn't overwrite statistics (see #155184).
+
+statement ok
+CREATE TABLE t155184 (
+  a INT PRIMARY KEY
+) WITH (sql_stats_automatic_collection_enabled = false, sql_stats_histogram_samples_count = 2)
+
+# These stats were created with the following statements:
+#
+#   INSERT INTO t155184 SELECT generate_series(1,10)
+#   ANALYZE t155184
+
+statement ok
+ALTER TABLE t155184 INJECT STATISTICS '[
+    {
+        "avg_size": 1,
+        "columns": [
+            "a"
+        ],
+        "created_at": "2025-10-10 13:41:08.711908",
+        "distinct_count": 10,
+        "histo_buckets": [
+            {
+                "distinct_range": 0,
+                "num_eq": 0,
+                "num_range": 0,
+                "upper_bound": "-9223372036854775808"
+            },
+            {
+                "distinct_range": 3.5,
+                "num_eq": 1,
+                "num_range": 4,
+                "upper_bound": "8"
+            },
+            {
+                "distinct_range": 1,
+                "num_eq": 1,
+                "num_range": 1,
+                "upper_bound": "10"
+            },
+            {
+                "distinct_range": 3.5,
+                "num_eq": 0,
+                "num_range": 4,
+                "upper_bound": "9223372036854775807"
+            }
+        ],
+        "histo_col_type": "INT8",
+        "histo_version": 3,
+        "id": 1114221014922133505,
+        "null_count": 0,
+        "row_count": 10
+    }
+]'
+
+# All we care about is the row counts, so don't error if other parts of the explain output change
+query T
+SELECT info FROM [EXPLAIN SELECT * FROM t155184 WHERE a < 8] WHERE info LIKE '%estimated row count:%'
+----
+  estimated row count: 4 (36% of the table; stats collected <hidden> ago)

pkg/sql/stats/merge.go

@@ -86,8 +86,9 @@ func MergedStatistics(
 	return mergedStats
 }
 
-// stripOuterBuckets removes the outer buckets from a histogram without a
-// leading NULL bucket.
+// stripOuterBuckets returns a copy of the histogram buckets with any outer
+// buckets that may have been added by addOuterBuckets removed. histogram must
+// not have a leading NULL bucket.
 func stripOuterBuckets(
 	ctx context.Context, evalCtx *eval.Context, histogram []cat.HistogramBucket,
 ) []cat.HistogramBucket {
@@ -96,17 +97,28 @@ func stripOuterBuckets(
 	}
 	startIdx := 0
 	endIdx := len(histogram)
-	if histogram[0].UpperBound.IsMin(ctx, evalCtx) && histogram[0].NumEq == 0 {
+	hasLowerOuter := histogram[0].UpperBound.IsMin(ctx, evalCtx) && histogram[0].NumEq == 0
+	if hasLowerOuter {
 		startIdx = 1
-		// Set the first range counts to zero to counteract range counts added by
-		// addOuterBuckets.
-		histogram[startIdx].NumRange = 0
-		histogram[startIdx].DistinctRange = 0
 	}
 	if histogram[len(histogram)-1].UpperBound.IsMax(ctx, evalCtx) && histogram[len(histogram)-1].NumEq == 0 {
 		endIdx = len(histogram) - 1
 	}
-	return histogram[startIdx:endIdx]
+	if startIdx == 0 && endIdx == len(histogram) {
+		return histogram
+	}
+	if startIdx >= endIdx {
+		return nil
+	}
+
+	out := append([]cat.HistogramBucket(nil), histogram[startIdx:endIdx]...)
+	if hasLowerOuter {
+		// Set the first range counts to zero to counteract range counts added by
+		// addOuterBuckets.
+		out[0].NumRange = 0
+		out[0].DistinctRange = 0
+	}
+	return out
 }
 
 // mergePartialStatistic merges a full statistic with a more recent partial

pkg/sql/stats/merge_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 )
@@ -916,6 +917,92 @@ func TestMergedStatistics(t *testing.T) {
 	}
 }
 
+// TestStripOuterBuckets tests stripOuterBuckets which removes outer buckets
+// added by addOuterBuckets before merging partial statistics.
+func TestStripOuterBuckets(t *testing.T) {
+	ctx := context.Background()
+	st := cluster.MakeTestingClusterSettings()
+	evalCtx := eval.NewTestingEvalContext(st)
+	defer evalCtx.Stop(ctx)
+
+	t.Run("no outer buckets returns input", func(t *testing.T) {
+		buckets := []cat.HistogramBucket{
+			{NumEq: 1, NumRange: 0, DistinctRange: 0, UpperBound: tree.NewDInt(10)},
+			{NumEq: 2, NumRange: 1, DistinctRange: 1, UpperBound: tree.NewDInt(20)},
+		}
+		strippedBuckets := stripOuterBuckets(ctx, evalCtx, buckets)
+		if !reflect.DeepEqual(strippedBuckets, buckets) {
+			t.Fatalf("expected buckets unchanged: %v", strippedBuckets)
+		}
+		if len(strippedBuckets) > 0 && &strippedBuckets[0] != &buckets[0] {
+			t.Fatalf("unexpected copy of backing array when no outer buckets")
+		}
+	})
+
+	testCases := []struct {
+		name     string
+		buckets  []cat.HistogramBucket
+		expected []cat.HistogramBucket
+	}{
+		{
+			buckets: []cat.HistogramBucket{
+				{NumEq: 0, UpperBound: tree.NewDInt(math.MinInt64)},
+				{NumEq: 1, NumRange: 10, DistinctRange: 5, UpperBound: tree.NewDInt(30)},
+				{NumEq: 0, UpperBound: tree.NewDInt(math.MaxInt64)},
+			},
+			expected: []cat.HistogramBucket{
+				{NumEq: 1, NumRange: 0, DistinctRange: 0, UpperBound: tree.NewDInt(30)},
+			},
+		},
+		{
+			buckets: []cat.HistogramBucket{
+				{NumEq: 0, UpperBound: tree.NewDInt(math.MinInt64)},
+				{NumEq: 1, NumRange: 10, DistinctRange: 5, UpperBound: tree.NewDInt(30)},
+				{NumEq: 2, NumRange: 4, DistinctRange: 3, UpperBound: tree.NewDInt(40)},
+			},
+			expected: []cat.HistogramBucket{
+				{NumEq: 1, NumRange: 0, DistinctRange: 0, UpperBound: tree.NewDInt(30)},
+				{NumEq: 2, NumRange: 4, DistinctRange: 3, UpperBound: tree.NewDInt(40)},
+			},
+		},
+		{
+			buckets: []cat.HistogramBucket{
+				{NumEq: 3, NumRange: 0, DistinctRange: 0, UpperBound: tree.NewDInt(30)},
+				{NumEq: 2, NumRange: 4, DistinctRange: 3, UpperBound: tree.NewDInt(40)},
+				{NumEq: 0, UpperBound: tree.NewDInt(math.MaxInt64)},
+			},
+			expected: []cat.HistogramBucket{
+				{NumEq: 3, NumRange: 0, DistinctRange: 0, UpperBound: tree.NewDInt(30)},
+				{NumEq: 2, NumRange: 4, DistinctRange: 3, UpperBound: tree.NewDInt(40)},
+			},
+		},
+	}
+	for i, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			buckets := append([]cat.HistogramBucket(nil), tc.buckets...)
+			bucketsCopy := append([]cat.HistogramBucket(nil), buckets...)
+			strippedBuckets := stripOuterBuckets(ctx, evalCtx, buckets)
+
+			if !reflect.DeepEqual(strippedBuckets, tc.expected) {
+				t.Fatalf("test case %d incorrect, stripped buckets:\n%v\nexpected:\n%v",
+					i, strippedBuckets, tc.expected)
+			}
+			if !reflect.DeepEqual(buckets, bucketsCopy) {
+				t.Fatalf("test case %d unexpected mutation of input buckets:\n%v\nexpected:\n%v",
+					i, buckets, bucketsCopy)
+			}
+			if len(strippedBuckets) > 0 {
+				for bi := range buckets {
+					if &strippedBuckets[0] == &buckets[bi] {
+						t.Fatalf("test case %d expected stripped buckets result to copy"+
+							" input slice, but shares backing array", i)
+					}
+				}
+			}
+		})
+	}
+}
+
 func (tabStat *TableStatistic) RoundDistinctRanges() {
 	for i := range tabStat.Histogram {
 		tabStat.Histogram[i].DistinctRange = math.Round(tabStat.Histogram[i].DistinctRange)