Merge pull request #136812 from cockroachdb/blathers/backport-release…

…-24.2.6-rc-136759

release-24.2.6-rc: sql/stats: skip over histogram buckets for dropped enum values

pkg/sql/logictest/testdata/logic_test/distsql_stats

@@ -3374,3 +3374,43 @@ upper_bound   range_rows  distinct_range_rows  equal_rows
 'paid'        0           0                    1
 'dispatched'  0           0                    1
 'delivered'   0           0                    1
+
+# Regression test for #67050: make sure we skip over enum values that have been
+# dropped when decoding histograms.
+
+statement ok
+CREATE TYPE e67050 AS ENUM ('a', 'b', 'c')
+
+statement ok
+CREATE TABLE t67050 (x e67050 PRIMARY KEY)
+
+statement ok
+INSERT INTO t67050 VALUES ('a'), ('b'), ('c')
+
+statement ok
+ANALYZE t67050
+
+statement ok
+DELETE FROM t67050 WHERE x = 'a'
+
+statement ok
+ALTER TYPE e67050 DROP VALUE 'a'
+
+query T
+SELECT jsonb_pretty(statistics->0->'histo_buckets') FROM
+[SHOW STATISTICS USING JSON FOR TABLE t67050]
+----
+[
+    {
+        "distinct_range": 0,
+        "num_eq": 1,
+        "num_range": 0,
+        "upper_bound": "b"
+    },
+    {
+        "distinct_range": 0,
+        "num_eq": 1,
+        "num_range": 0,
+        "upper_bound": "c"
+    }
+]

pkg/sql/stats/json.go

@@ -63,29 +63,29 @@ func (js *JSONStatistic) SetHistogram(h *HistogramData) error {
 	// done across databases. If it is a user-defined type, we need the type name
 	// resolution to be for the correct database.
 	js.HistogramColumnType = typ.SQLStringFullyQualified()
-	js.HistogramBuckets = make([]JSONHistoBucket, len(h.Buckets))
+	js.HistogramBuckets = make([]JSONHistoBucket, 0, len(h.Buckets))
 	js.HistogramVersion = h.Version
 	var a tree.DatumAlloc
 	for i := range h.Buckets {
 		b := &h.Buckets[i]
-		js.HistogramBuckets[i].NumEq = b.NumEq
-		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 := DecodeUpperBound(h.Version, typ, &a, b.UpperBound)
 		if err != nil {
+			if h.ColumnType.Family() == types.EnumFamily && errors.Is(err, types.EnumValueNotFound) {
+				// Skip over buckets for enum values that were dropped.
+				continue
+			}
 			return err
 		}
 
-		js.HistogramBuckets[i] = JSONHistoBucket{
+		js.HistogramBuckets = append(js.HistogramBuckets, JSONHistoBucket{
 			NumEq:         b.NumEq,
 			NumRange:      b.NumRange,
 			DistinctRange: b.DistinctRange,
 			UpperBound:    tree.AsStringWithFlags(datum, tree.FmtExport),
-		}
+		})
 	}
 	return nil
 }

pkg/sql/stats/stats_cache.go

@@ -681,41 +681,42 @@ func (sc *TableStatisticsCache) parseStats(
 // the resulting buckets into tabStat.Histogram.
 func DecodeHistogramBuckets(tabStat *TableStatistic) error {
 	h := tabStat.HistogramData
-	var offset int
 	if tabStat.NullCount > 0 {
 		// A bucket for NULL is not persisted, but we create a fake one to
 		// make histograms easier to work with. The length of res.Histogram
 		// is therefore 1 greater than the length of the histogram data
 		// buckets.
 		// TODO(michae2): Combine this with setHistogramBuckets, especially if we
 		// need to change both after #6224 is fixed (NULLS LAST in index ordering).
-		tabStat.Histogram = make([]cat.HistogramBucket, len(h.Buckets)+1)
+		tabStat.Histogram = make([]cat.HistogramBucket, 1, len(h.Buckets)+1)
 		tabStat.Histogram[0] = cat.HistogramBucket{
 			NumEq:         float64(tabStat.NullCount),
 			NumRange:      0,
 			DistinctRange: 0,
 			UpperBound:    tree.DNull,
 		}
-		offset = 1
 	} else {
-		tabStat.Histogram = make([]cat.HistogramBucket, len(h.Buckets))
-		offset = 0
+		tabStat.Histogram = make([]cat.HistogramBucket, 0, len(h.Buckets))
 	}
 
 	// Decode the histogram data so that it's usable by the opt catalog.
 	var a tree.DatumAlloc
-	for i := offset; i < len(tabStat.Histogram); i++ {
-		bucket := &h.Buckets[i-offset]
+	for i := range h.Buckets {
+		bucket := &h.Buckets[i]
 		datum, err := DecodeUpperBound(h.Version, h.ColumnType, &a, bucket.UpperBound)
 		if err != nil {
+			if h.ColumnType.Family() == types.EnumFamily && errors.Is(err, types.EnumValueNotFound) {
+				// Skip over buckets for enum values that were dropped.
+				continue
+			}
 			return err
 		}
-		tabStat.Histogram[i] = cat.HistogramBucket{
+		tabStat.Histogram = append(tabStat.Histogram, cat.HistogramBucket{
 			NumEq:         float64(bucket.NumEq),
 			NumRange:      float64(bucket.NumRange),
 			DistinctRange: bucket.DistinctRange,
 			UpperBound:    datum,
-		}
+		})
 	}
 	return nil
 }

pkg/sql/types/types.go

@@ -2787,6 +2787,8 @@ func (t *T) IsNumeric() bool {
 	}
 }
 
+var EnumValueNotFound = errors.New("could not find enum value")
+
 // EnumGetIdxOfPhysical returns the index within the TypeMeta's slice of
 // enum physical representations that matches the input byte slice.
 func (t *T) EnumGetIdxOfPhysical(phys []byte) (int, error) {
@@ -2799,7 +2801,7 @@ func (t *T) EnumGetIdxOfPhysical(phys []byte) (int, error) {
 			return i, nil
 		}
 	}
-	err := errors.Newf(
+	err := errors.Wrapf(EnumValueNotFound,
 		"could not find %v in enum %q representation %s %s",
 		phys,
 		t.TypeMeta.Name.FQName(true /* explicitCatalog */),