stats: improve handling of some expected errors

In 25.1 we enabled automatic partial stats collection by default. It
works by scanning an index of the table outside of the bounds according
to the last stats collection (i.e. lower than min and higher than max).
If the last stats collection was such that there is no lower bound (which
can happen when we only have a bucket for NULL values as well as "outer"
buckets), we fail the partial stats collection. Previously, this would
result in failing the stats collection job which is logged at the ERROR
level, but this failure is benign and shouldn't result in a scary
looking log.

To prevent unnecessary user concerns this commit adds the logic to
swallow the benign errors which allows us to short-circuit the
particular stats collection without failing the job. We currently treat
all errors with `ObjectNotInPrerequisiteState` PG code as "benign"
(there are 4 more related to partial stats, apart from the scenario
mentioned above). To have some observability into this swalling behavior
we add a log message at INFO level. Additionally, we don't swallow the
error if the stats collection was executed via EXPLAIN or EXPLAIN ANALYZE
(meaning that it was explicitly user-initiated).

The fact that the job will be marked as having "succeeded" can be
confusing since it won't have a corresponding new statistics, but we find
this behavior better overall.

Additionally, this commit extends the error message to also include the
table and index names as well as their IDs.

Release note (bug fix): In 25.1 version CockroachDB enabled automatic
partial stats collection by default (by flipping
`sql.stats.automatic_partial_collection.enabled` cluster setting to
`true`). Previously, partial stats collection could encounter some
expected scenarios that would be reported with PG error code `55000`
and would show up as failed stats jobs. These errors are benign and will
no longer be reported (the stats job will be marked as "succeeded" but
new stats won't be created).

Author: yuzefovich

docs/generated/settings/settings-for-tenants.txt

@@ -369,7 +369,7 @@ sql.stats.multi_column_collection.enabled	boolean	true	multi-column statistics c
 sql.stats.non_default_columns.min_retention_period	duration	24h0m0s	minimum retention period for table statistics collected on non-default columns	application
 sql.stats.non_indexed_json_histograms.enabled	boolean	false	set to true to collect table statistics histograms on non-indexed JSON columns	application
 sql.stats.persisted_rows.max	integer	1000000	maximum number of rows of statement and transaction statistics that will be persisted in the system tables before compaction begins	application
-sql.stats.post_events.enabled	boolean	false	if set, an event is logged for every CREATE STATISTICS job	application
+sql.stats.post_events.enabled	boolean	false	if set, an event is logged for every successful CREATE STATISTICS job	application
 sql.stats.response.max	integer	20000	the maximum number of statements and transaction stats returned in a CombinedStatements request	application
 sql.stats.response.show_internal.enabled	boolean	false	controls if statistics for internal executions should be returned by the CombinedStatements and if internal sessions should be returned by the ListSessions endpoints. These endpoints are used to display statistics on the SQL Activity pages	application
 sql.stats.system_tables.enabled	boolean	true	when true, enables use of statistics on system tables by the query optimizer	application

docs/generated/settings/settings.html

@@ -324,7 +324,7 @@
 <tr><td><div id="setting-sql-stats-non-default-columns-min-retention-period" class="anchored"><code>sql.stats.non_default_columns.min_retention_period</code></div></td><td>duration</td><td><code>24h0m0s</code></td><td>minimum retention period for table statistics collected on non-default columns</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-non-indexed-json-histograms-enabled" class="anchored"><code>sql.stats.non_indexed_json_histograms.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to collect table statistics histograms on non-indexed JSON columns</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-persisted-rows-max" class="anchored"><code>sql.stats.persisted_rows.max</code></div></td><td>integer</td><td><code>1000000</code></td><td>maximum number of rows of statement and transaction statistics that will be persisted in the system tables before compaction begins</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-sql-stats-post-events-enabled" class="anchored"><code>sql.stats.post_events.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if set, an event is logged for every CREATE STATISTICS job</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-sql-stats-post-events-enabled" class="anchored"><code>sql.stats.post_events.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if set, an event is logged for every successful CREATE STATISTICS job</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-response-max" class="anchored"><code>sql.stats.response.max</code></div></td><td>integer</td><td><code>20000</code></td><td>the maximum number of statements and transaction stats returned in a CombinedStatements request</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-response-show-internal-enabled" class="anchored"><code>sql.stats.response.show_internal.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>controls if statistics for internal executions should be returned by the CombinedStatements and if internal sessions should be returned by the ListSessions endpoints. These endpoints are used to display statistics on the SQL Activity pages</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-system-tables-enabled" class="anchored"><code>sql.stats.system_tables.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>when true, enables use of statistics on system tables by the query optimizer</td><td>Serverless/Dedicated/Self-Hosted</td></tr>

pkg/sql/create_stats.go

@@ -44,7 +44,7 @@ import (
 var createStatsPostEvents = settings.RegisterBoolSetting(
 	settings.ApplicationLevel,
 	"sql.stats.post_events.enabled",
-	"if set, an event is logged for every CREATE STATISTICS job",
+	"if set, an event is logged for every successful CREATE STATISTICS job",
 	false,
 	settings.WithPublic)
 
@@ -854,6 +854,8 @@ func (r *createStatsResumer) Resume(ctx context.Context, execCtx interface{}) er
 		return nil
 	}
 
+	// Note that we'll log an event even if the stats collection didn't occur
+	// due to a benign error.
 	// TODO(rytaft): This creates a new transaction for the CREATE STATISTICS
 	// event. It must be different from the CREATE STATISTICS transaction,
 	// because that transaction must be read-only. In the future we may want

pkg/sql/distsql_physical_planner.go

@@ -4060,6 +4060,10 @@ func (dsp *DistSQLPlanner) createPhysPlanForPlanNode(
 				// Partial stats collections scan a different index for each column.
 				numIndexes = len(details.ColumnStats)
 			}
+			// If the error has pgcode.ObjectNotInPrerequisiteState, we don't
+			// swallow it here and propagate it to the client (since we're
+			// running EXPLAIN or EXPLAIN ANALYZE, the caller would be
+			// interested to know about all errors, benign included).
 			plan, err = dsp.createPlanForCreateStats(
 				ctx, planCtx, planCtx.planner.SemaCtx(), 0 /* jobID */, details,
 				numIndexes, 0, /* curIndex */

pkg/sql/distsql_plan_stats.go

@@ -21,7 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
-	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -349,10 +348,9 @@ func (dsp *DistSQLPlanner) createPartialStatsPlan(
 	)
 
 	var stat *stats.TableStatistic
-	var histogram []cat.HistogramBucket
-	// Find the statistic and histogram from the newest table statistic for our
-	// column that is not partial and not forecasted. The first one we find will
-	// be the latest due to the newest to oldest ordering property of the cache.
+	// Find the statistic from the newest table statistic for our column that is
+	// not partial and not forecasted. The first one we find will be the latest
+	// due to the newest to oldest ordering property of the cache.
 	for _, t := range tableStats {
 		if len(t.ColumnIDs) == 1 && column.GetID() == t.ColumnIDs[0] &&
 			!t.IsPartial() && !t.IsMerged() && !t.IsForecast() {
@@ -372,7 +370,6 @@ func (dsp *DistSQLPlanner) createPartialStatsPlan(
 				)
 			}
 			stat = t
-			histogram = t.Histogram
 			break
 		}
 	}
@@ -382,10 +379,18 @@ func (dsp *DistSQLPlanner) createPartialStatsPlan(
 			"column %s does not have a prior statistic",
 			column.GetName())
 	}
-	lowerBound, upperBound, err := bounds.GetUsingExtremesBounds(ctx, planCtx.EvalContext(), histogram)
+	lowerBound, upperBound, err := bounds.GetUsingExtremesBounds(ctx, planCtx.EvalContext(), stat.Histogram)
 	if err != nil {
 		return nil, err
 	}
+	if lowerBound == nil {
+		return nil, pgerror.Newf(
+			pgcode.ObjectNotInPrerequisiteState,
+			"only outer or NULL bounded buckets exist in %s@%s (table ID %d, column IDs %v), "+
+				"so partial stats cannot be collected",
+			scan.desc.GetName(), scan.index.GetName(), stat.TableID, stat.ColumnIDs,
+		)
+	}
 	extremesSpans, err := bounds.ConstructUsingExtremesSpans(lowerBound, upperBound, scan.index)
 	if err != nil {
 		return nil, err
@@ -724,6 +729,11 @@ func (dsp *DistSQLPlanner) createStatsPlan(
 		numIndexes, curIndex), nil
 }
 
+// createPlanForCreateStats creates the DistSQL plan to perform the table stats
+// collection according to CreateStatsDetails.
+//
+// If the returned error has pgcode.ObjectNotInPrerequisiteState, it might be
+// benign and the caller might want to swallow it, depending on the context.
 func (dsp *DistSQLPlanner) createPlanForCreateStats(
 	ctx context.Context,
 	planCtx *PlanningCtx,
@@ -781,6 +791,17 @@ func (dsp *DistSQLPlanner) planAndRunCreateStats(
 
 	physPlan, err := dsp.createPlanForCreateStats(ctx, planCtx, semaCtx, jobId, details, numIndexes, curIndex)
 	if err != nil {
+		if pgerror.GetPGCode(err) == pgcode.ObjectNotInPrerequisiteState {
+			// This error is benign, so we'll swallow it. Concretely, this means
+			// that we'll proceed with collecting partial stats if there are
+			// more to do and the stats job overall will be marked as having
+			// "succeeded". Even though this might seem confusing, it's a better
+			// trade-off than having auto partial stats fail repeatedly due to
+			// expected conditions (like a lower bound doesn't exist) raising
+			// concerns for users. See #149279 for more discussion.
+			log.Infof(ctx, "job %d: stats collection is swallowing benign error %v", jobId, err)
+			return nil
+		}
 		return err
 	}
 

pkg/sql/logictest/testdata/logic_test/distsql_stats

@@ -2702,10 +2702,10 @@ statement ok
 CREATE TABLE xyz (x INT, y INT, z INT, INDEX (x, y));
 
 statement error pq: column x does not have a prior statistic
-CREATE STATISTICS xyz_x ON x FROM xyz USING EXTREMES;
+EXPLAIN ANALYZE CREATE STATISTICS xyz_x ON x FROM xyz USING EXTREMES;
 
 statement error pq: the latest full statistic for column a has no histogram
-CREATE STATISTICS u_partial ON a FROM u USING EXTREMES;
+EXPLAIN ANALYZE CREATE STATISTICS u_partial ON a FROM u USING EXTREMES;
 
 statement error pq: table xy does not contain a non-partial forward index with y as a prefix column
 CREATE STATISTICS xy_y_partial ON y FROM xy USING EXTREMES;
@@ -2724,8 +2724,8 @@ CREATE STATISTICS only_null_stat ON a FROM only_null;
 statement ok
 SELECT crdb_internal.clear_table_stats_cache();
 
-statement error pq: only outer or NULL bounded buckets exist in the index, so partial stats cannot be collected
-CREATE STATISTICS only_null_partial ON a FROM only_null USING EXTREMES;
+statement error pq: only outer or NULL bounded buckets exist in only_null@only_null_a_idx \(table ID \d+, column IDs \[1\]\), so partial stats cannot be collected
+EXPLAIN ANALYZE CREATE STATISTICS only_null_partial ON a FROM only_null USING EXTREMES;
 
 statement ok
 CREATE INDEX ON xy (y) WHERE y > 5;

pkg/sql/stats/bounds/BUILD.bazel

@@ -10,8 +10,6 @@ go_library(
         "//pkg/sql/catalog/catenumpb",
         "//pkg/sql/opt/cat",
         "//pkg/sql/opt/constraint",
-        "//pkg/sql/pgwire/pgcode",
-        "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/sem/eval",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sem/tree/treecmp",

pkg/sql/stats/bounds/extremes.go

@@ -12,8 +12,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/constraint"
-	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
-	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree/treecmp"
@@ -80,6 +78,9 @@ func ConstructUsingExtremesSpans(
 
 // GetUsingExtremesBounds returns a tree.Datum representing the exclusive upper
 // and exclusive lower bounds of the USING EXTREMES span for partial statistics.
+//
+// lowerBound can be nil which is the case when the exclusive lower bound
+// doesn't exist.
 func GetUsingExtremesBounds(
 	ctx context.Context, evalCtx *eval.Context, histogram []cat.HistogramBucket,
 ) (lowerBound tree.Datum, upperBound tree.Datum, _ error) {
@@ -95,8 +96,8 @@ func GetUsingExtremesBounds(
 		upperBound = histogram[len(histogram)-2].UpperBound
 	}
 
-	// Pick the earliest lowerBound that is not null and isn't an outer bucket,
-	// but if none exist, return error
+	// Pick the earliest lowerBound that is not null and isn't an outer bucket.
+	// We'll return nil if none exist which the caller is required to handle.
 	for i := range histogram {
 		hist := &histogram[i]
 		if cmp, err := hist.UpperBound.Compare(ctx, evalCtx, tree.DNull); err != nil {
@@ -106,11 +107,5 @@ func GetUsingExtremesBounds(
 			break
 		}
 	}
-	if lowerBound == nil {
-		return lowerBound, nil,
-			pgerror.Newf(
-				pgcode.ObjectNotInPrerequisiteState,
-				"only outer or NULL bounded buckets exist in the index, so partial stats cannot be collected")
-	}
 	return lowerBound, upperBound, nil
 }