Merge #37983

37983: opt: fix issue with dangling pointer in statisticsBuilder r=rytaft a=rytaft

This commit fixes an issue where the `statisticsBuilder` was using
an invalid pointer to update statistics for some multi-column stats
in the local table statistics cache. The fix is to re-fetch the
pointer to the stats just before updating them, in case the
previously fetched address is no longer valid.

This problem is very hard to reproduce, so I had to use the somewhat
complex query from the issue for a regression test.

Fixes #37953

Release note: None

Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>

pkg/sql/opt/memo/statistics_builder.go

@@ -386,6 +386,8 @@ func (sb *statisticsBuilder) colStatLeaf(
 				colStat.NullCount = s.RowCount * unknownNullCountRatio
 			} else {
 				colStatLeaf := sb.colStatLeaf(nullableCols, s, fd, notNullCols)
+				// Fetch the colStat again since it may now have a different address.
+				colStat, _ = s.ColStats.Lookup(colSet)
 				colStat.NullCount = colStatLeaf.NullCount
 			}
 		}
@@ -414,6 +416,8 @@ func (sb *statisticsBuilder) colStatLeaf(
 				nullCount += colStatLeaf.NullCount * (1 - nullCount/s.RowCount)
 			}
 		})
+		// Fetch the colStat again since it may now have a different address.
+		colStat, _ = s.ColStats.Lookup(colSet)
 		colStat.DistinctCount = min(distinctCount, s.RowCount)
 		colStat.NullCount = min(nullCount, s.RowCount)
 	}

pkg/sql/opt/memo/testdata/stats/scan

@@ -522,3 +522,95 @@ scan a
  ├── stats: [rows=10]
  ├── key: (1)
  └── ordering: +1
+
+# Regression test for #37953.
+exec-ddl
+CREATE TABLE t37953 (
+    a UUID NOT NULL,
+    b FLOAT8 NOT NULL,
+    c TIME NOT NULL,
+    d UUID NOT NULL,
+    e VARCHAR,
+    f "char" NULL,
+    g INT4 NOT NULL,
+    h VARCHAR NULL,
+    i REGPROC NULL,
+    j FLOAT8 NOT NULL
+)
+----
+TABLE t37953
+ ├── a uuid not null
+ ├── b float not null
+ ├── c time not null
+ ├── d uuid not null
+ ├── e varchar
+ ├── f "char"
+ ├── g int4 not null
+ ├── h varchar
+ ├── i regproc
+ ├── j float not null
+ ├── rowid int not null (hidden)
+ └── INDEX primary
+      └── rowid int not null (hidden)
+
+norm
+WITH
+  subq (col0, col1)
+    AS (
+      SELECT
+        tab1.g AS col0,
+        CASE
+        WHEN ilike_escape(
+          regexp_replace(
+            tab0.h,
+            tab1.e,
+            tab0.f,
+            tab0.e::STRING
+          ),
+          tab1.f,
+          ''
+        )
+        THEN true
+        ELSE false
+        END
+          AS col1
+      FROM
+        t37953 AS tab0, t37953 AS tab1
+      WHERE
+        tab0.j IN (tab1.j,)
+    )
+SELECT
+  1
+FROM
+  subq
+WHERE
+  subq.col1;
+----
+project
+ ├── columns: "?column?":24(int!null)
+ ├── stats: [rows=0.95099005]
+ ├── fd: ()-->(24)
+ ├── select
+ │    ├── columns: col1:23(bool!null)
+ │    ├── stats: [rows=0.95099005, distinct(23)=0.95099005, null(23)=0]
+ │    ├── fd: ()-->(23)
+ │    ├── project
+ │    │    ├── columns: col1:23(bool)
+ │    │    ├── stats: [rows=333333.333, distinct(23)=333333.333, null(23)=16336.65]
+ │    │    ├── inner-join
+ │    │    │    ├── columns: tab0.e:5(varchar) tab0.f:6("char") tab0.h:8(varchar) tab0.j:10(float!null) tab1.e:16(varchar) tab1.f:17("char") tab1.j:21(float!null)
+ │    │    │    ├── stats: [rows=333333.333, distinct(10)=100, null(10)=0, distinct(21)=100, null(21)=0, distinct(5,6,8,16,17)=333333.333, null(5,6,8,16,17)=16336.65]
+ │    │    │    ├── scan tab0
+ │    │    │    │    ├── columns: tab0.e:5(varchar) tab0.f:6("char") tab0.h:8(varchar) tab0.j:10(float!null)
+ │    │    │    │    └── stats: [rows=1000, distinct(10)=100, null(10)=0, distinct(5,6,8)=1000, null(5,6,8)=29.701]
+ │    │    │    ├── scan tab1
+ │    │    │    │    ├── columns: tab1.e:16(varchar) tab1.f:17("char") tab1.j:21(float!null)
+ │    │    │    │    └── stats: [rows=1000, distinct(21)=100, null(21)=0, distinct(16,17)=1000, null(16,17)=19.9]
+ │    │    │    └── filters
+ │    │    │         └── tab0.j IN (tab1.j,) [type=bool, outer=(10,21)]
+ │    │    └── projections
+ │    │         └── CASE WHEN ilike_escape(regexp_replace(tab0.h, tab1.e, tab0.f, tab0.e::STRING), tab1.f, '') THEN true ELSE false END [type=bool, outer=(5,6,8,16,17)]
+ │    └── filters
+ │         └── variable: col1 [type=bool, outer=(23), constraints=(/23: [/true - /true]; tight), fd=()-->(23)]
+ └── projections
+      └── const: 1 [type=int]

pkg/sql/opt/props/col_stats_map.go

@@ -98,6 +98,10 @@ func (m *ColStatsMap) Count() int {
 
 // Get returns the nth statistic in the map, by its ordinal position. This
 // position is stable across calls to Get or Add (but not RemoveIntersecting).
+// NOTE: The returned *ColumnStatistic is only valid until this ColStatsMap is
+//       updated via a call to Add() or RemoveIntersecting(). At that point,
+//       the address of the statistic may have changed, so it must be fetched
+//       again using another call to Get() or Lookup().
 func (m *ColStatsMap) Get(nth int) *ColumnStatistic {
 	if nth < initialColStatsCap {
 		return &m.initial[nth]
@@ -107,6 +111,10 @@ func (m *ColStatsMap) Get(nth int) *ColumnStatistic {
 
 // Lookup returns the column statistic indexed by the given column set. If no
 // such statistic exists in the map, then ok=false.
+// NOTE: The returned *ColumnStatistic is only valid until this ColStatsMap is
+//       updated via a call to Add() or RemoveIntersecting(). At that point,
+//       the address of the statistic may have changed, so it must be fetched
+//       again using another call to Lookup() or Get().
 func (m *ColStatsMap) Lookup(cols opt.ColSet) (colStat *ColumnStatistic, ok bool) {
 	// Scan the inlined statistics if there are only a few statistics in the map.
 	if m.count <= initialColStatsCap {
@@ -150,6 +158,10 @@ func (m *ColStatsMap) Lookup(cols opt.ColSet) (colStat *ColumnStatistic, ok bool
 // it does not yet exist in the map, then Add adds a new blank ColumnStatistic
 // and returns it, along with added=true. Otherwise, Add returns the existing
 // ColumnStatistic with added=false.
+// NOTE: The returned *ColumnStatistic is only valid until this ColStatsMap is
+//       updated via another call to Add() or RemoveIntersecting(). At that
+//       point, the address of the statistic may have changed, so it must be
+//       fetched again using Lookup() or Get().
 func (m *ColStatsMap) Add(cols opt.ColSet) (_ *ColumnStatistic, added bool) {
 	// Only add column set if it is not already present in the map.
 	colStat, ok := m.Lookup(cols)