Merge pull request #154576 from rafiss/blathers/backport-release-25.4-154495

rafiss · web-flow · commit 93844021e343 · 2025-10-02T16:49:39.000-04:00
release-25.4: sql: add guardrail for approximate max schema objects
diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt
@@ -346,6 +346,7 @@ sql.multiple_modifications_of_table.enabled	boolean	false	if true, allow stateme
 sql.multiregion.drop_primary_region.enabled	boolean	true	allows dropping the PRIMARY REGION of a database if it is the last region	application
 sql.notices.enabled	boolean	true	enable notices in the server/client protocol being sent	application
 sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled	boolean	false	if enabled, uniqueness checks may be planned for mutations of UUID columns updated with gen_random_uuid(); otherwise, uniqueness is assumed due to near-zero collision probability	application
+sql.schema.approx_max_object_count	integer	20000	approximate maximum number of schema objects allowed in the cluster; the check uses cached statistics, so the actual count may slightly exceed this limit; set to 0 to disable	application
 sql.schema.telemetry.recurrence	string	@weekly	cron-tab recurrence for SQL schema telemetry job	system-visible
 sql.spatial.experimental_box2d_comparison_operators.enabled	boolean	false	enables the use of certain experimental box2d comparison operators	application
 sql.sqlcommenter.enabled	boolean	false	enables support for sqlcommenter. Key value parsed from sqlcommenter comments will be included in sql insights and sql logs. See https://google.github.io/sqlcommenter/ for more details.	application
diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html
@@ -301,6 +301,7 @@
 <tr><td><div id="setting-sql-multiregion-drop-primary-region-enabled" class="anchored"><code>sql.multiregion.drop_primary_region.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>allows dropping the PRIMARY REGION of a database if it is the last region</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-notices-enabled" class="anchored"><code>sql.notices.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>enable notices in the server/client protocol being sent</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-optimizer-uniqueness-checks-for-gen-random-uuid-enabled" class="anchored"><code>sql.optimizer.uniqueness_checks_for_gen_random_uuid.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if enabled, uniqueness checks may be planned for mutations of UUID columns updated with gen_random_uuid(); otherwise, uniqueness is assumed due to near-zero collision probability</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
+<tr><td><div id="setting-sql-schema-approx-max-object-count" class="anchored"><code>sql.schema.approx_max_object_count</code></div></td><td>integer</td><td><code>20000</code></td><td>approximate maximum number of schema objects allowed in the cluster; the check uses cached statistics, so the actual count may slightly exceed this limit; set to 0 to disable</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-schema-telemetry-recurrence" class="anchored"><code>sql.schema.telemetry.recurrence</code></div></td><td>string</td><td><code>@weekly</code></td><td>cron-tab recurrence for SQL schema telemetry job</td><td>Advanced/Self-hosted (read-write); Basic/Standard (read-only)</td></tr>
 <tr><td><div id="setting-sql-spatial-experimental-box2d-comparison-operators-enabled" class="anchored"><code>sql.spatial.experimental_box2d_comparison_operators.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>enables the use of certain experimental box2d comparison operators</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-sqlcommenter-enabled" class="anchored"><code>sql.sqlcommenter.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>enables support for sqlcommenter. Key value parsed from sqlcommenter comments will be included in sql insights and sql logs. See https://google.github.io/sqlcommenter/ for more details.</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -2323,6 +2323,7 @@ GO_TARGETS = [
     "//pkg/sql/schemachanger/sctest:sctest",
     "//pkg/sql/schemachanger:schemachanger",
     "//pkg/sql/schemachanger:schemachanger_test",
+    "//pkg/sql/schemaobjectlimit:schemaobjectlimit",
     "//pkg/sql/scrub/scrubtestutils:scrubtestutils",
     "//pkg/sql/scrub:scrub",
     "//pkg/sql/sem/asof:asof",
diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel
@@ -502,6 +502,7 @@ go_library(
         "//pkg/sql/schemachanger/scpb",
         "//pkg/sql/schemachanger/scplan",
         "//pkg/sql/schemachanger/scrun",
+        "//pkg/sql/schemaobjectlimit",
         "//pkg/sql/scrub",
         "//pkg/sql/sem/asof",
         "//pkg/sql/sem/builtins",
diff --git a/pkg/sql/descriptor.go b/pkg/sql/descriptor.go
@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
 	"github.com/cockroachdb/cockroach/pkg/sql/regions"
+	"github.com/cockroachdb/cockroach/pkg/sql/schemaobjectlimit"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -206,6 +207,13 @@ func (p *planner) createDescriptor(
 			"expected new descriptor, not a modification of version %d",
 			descriptor.OriginalVersion())
 	}
+
+	if err := schemaobjectlimit.CheckMaxSchemaObjects(
+		ctx, p.InternalSQLTxn(), p.Descriptors(), p.execCfg.TableStatsCache, p.execCfg.Settings, 1,
+	); err != nil {
+		return err
+	}
+
 	b := p.Txn().NewBatch()
 	kvTrace := p.ExtendedEvalContext().Tracing.KVTracingEnabled()
 	if err := p.Descriptors().WriteDescToBatch(ctx, kvTrace, descriptor, b); err != nil {
diff --git a/pkg/sql/schema_change_plan_node.go b/pkg/sql/schema_change_plan_node.go
@@ -318,6 +318,7 @@ func newSchemaChangerTxnRunDependencies(
 		metaDataUpdater,
 		evalContext.Planner,
 		execCfg.StatsRefresher,
+		execCfg.TableStatsCache,
 		execCfg.DeclarativeSchemaChangerTestingKnobs,
 		kvTrace,
 		schemaChangerJobID,
diff --git a/pkg/sql/schemachanger/scdeps/BUILD.bazel b/pkg/sql/schemachanger/scdeps/BUILD.bazel
@@ -36,12 +36,14 @@ go_library(
         "//pkg/sql/schemachanger/scexec/backfiller",
         "//pkg/sql/schemachanger/scexec/scmutationexec",
         "//pkg/sql/schemachanger/scrun",
+        "//pkg/sql/schemaobjectlimit",
         "//pkg/sql/sem/catconstants",
         "//pkg/sql/sem/eval",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sessiondata",
         "//pkg/sql/sqlerrors",
         "//pkg/sql/sqltelemetry",
+        "//pkg/sql/stats",
         "//pkg/sql/types",
         "//pkg/util/admission/admissionpb",
         "//pkg/util/syncutil",
diff --git a/pkg/sql/schemachanger/scdeps/exec_deps.go b/pkg/sql/schemachanger/scdeps/exec_deps.go
@@ -27,9 +27,11 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/isql"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec/scmutationexec"
+	"github.com/cockroachdb/cockroach/pkg/sql/schemaobjectlimit"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
+	"github.com/cockroachdb/cockroach/pkg/sql/stats"
 	"github.com/cockroachdb/errors"
 )
 
@@ -64,6 +66,7 @@ func NewExecutorDependencies(
 	metadataUpdater scexec.DescriptorMetadataUpdater,
 	temporarySchemaCreator scexec.TemporarySchemaCreator,
 	statsRefresher scexec.StatsRefresher,
+	tableStatsCache *stats.TableStatisticsCache,
 	testingKnobs *scexec.TestingKnobs,
 	kvTrace bool,
 	schemaChangerJobID jobspb.JobID,
@@ -77,6 +80,7 @@ func NewExecutorDependencies(
 			jobRegistry:        jobRegistry,
 			validator:          validator,
 			statsRefresher:     statsRefresher,
+			tableStatsCache:    tableStatsCache,
 			schemaChangerJobID: schemaChangerJobID,
 			schemaChangerJob:   nil,
 			kvTrace:            kvTrace,
@@ -105,6 +109,7 @@ type txnDeps struct {
 	createdJobs         []jobspb.JobID
 	validator           scexec.Validator
 	statsRefresher      scexec.StatsRefresher
+	tableStatsCache     *stats.TableStatisticsCache
 	tableStatsToRefresh []descpb.ID
 	schemaChangerJobID  jobspb.JobID
 	schemaChangerJob    *jobs.Job
@@ -370,6 +375,18 @@ func (d *txnDeps) InitializeSequence(id descpb.ID, startVal int64) {
 	batch.Inc(sequenceKey, startVal)
 }
 
+// CheckMaxSchemaObjects implements the scexec.Catalog interface.
+func (d *txnDeps) CheckMaxSchemaObjects(ctx context.Context, numNewObjects int) error {
+	return schemaobjectlimit.CheckMaxSchemaObjects(
+		ctx,
+		d.txn,
+		d.descsCollection,
+		d.tableStatsCache,
+		d.settings,
+		numNewObjects,
+	)
+}
+
 // Reset implements the scexec.Catalog interface.
 func (d *txnDeps) Reset(ctx context.Context) error {
 	d.descsCollection.ResetUncommitted(ctx)
diff --git a/pkg/sql/schemachanger/scdeps/run_deps.go b/pkg/sql/schemachanger/scdeps/run_deps.go
@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec/backfiller"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scrun"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/sql/stats"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 )
@@ -42,6 +43,7 @@ func NewJobRunDependencies(
 	indexValidator scexec.Validator,
 	metadataUpdaterFactory MetadataUpdaterFactory,
 	statsRefresher scexec.StatsRefresher,
+	tableStatsCache *stats.TableStatisticsCache,
 	testingKnobs *scexec.TestingKnobs,
 	statements []string,
 	sessionData *sessiondata.SessionData,
@@ -66,13 +68,15 @@ func NewJobRunDependencies(
 		sessionData:           sessionData,
 		kvTrace:               kvTrace,
 		statsRefresher:        statsRefresher,
+		tableStatsCache:       tableStatsCache,
 	}
 }
 
 type jobExecutionDeps struct {
 	collectionFactory     *descs.CollectionFactory
 	db                    descs.DB
 	statsRefresher        scexec.StatsRefresher
+	tableStatsCache       *stats.TableStatisticsCache
 	backfiller            scexec.Backfiller
 	spanSplitter          scexec.IndexSpanSplitter
 	merger                scexec.Merger
@@ -120,6 +124,7 @@ func (d *jobExecutionDeps) WithTxnInJob(ctx context.Context, fn scrun.JobTxnFunc
 				jobRegistry:        d.jobRegistry,
 				validator:          d.indexValidator,
 				statsRefresher:     d.statsRefresher,
+				tableStatsCache:    d.tableStatsCache,
 				schemaChangerJobID: d.job.ID(),
 				schemaChangerJob:   d.job,
 				kvTrace:            d.kvTrace,
diff --git a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go
@@ -1550,6 +1550,12 @@ func (s *TestState) InitializeSequence(id descpb.ID, startVal int64) {
 	s.LogSideEffectf("initializing sequence %d with starting value of %d", id, startVal)
 }
 
+// CheckMaxSchemaObjects is part of the scexec.Catalog interface.
+func (s *TestState) CheckMaxSchemaObjects(ctx context.Context, numNewObjects int) error {
+	// In tests, we don't enforce the limit.
+	return nil
+}
+
 // TemporarySchemaName is part of scbuild.TemporarySchemaProvider interface.
 func (s *TestState) TemporarySchemaName() string {
 	return fmt.Sprintf("pg_temp_%d_%d", 123, 456)
diff --git a/pkg/sql/schemachanger/scexec/dependencies.go b/pkg/sql/schemachanger/scexec/dependencies.go
@@ -119,6 +119,11 @@ type Catalog interface {
 
 	// InitializeSequence initializes the initial value for a sequence.
 	InitializeSequence(id descpb.ID, startVal int64)
+
+	// CheckMaxSchemaObjects checks if the number of schema objects in the
+	// cluster plus the new objects being created would exceed the configured
+	// limit. Returns an error if the limit would be exceeded.
+	CheckMaxSchemaObjects(ctx context.Context, numNewObjects int) error
 }
 
 // Telemetry encapsulates metrics gather for the declarative schema changer.
diff --git a/pkg/sql/schemachanger/scexec/exec_immediate_mutation.go b/pkg/sql/schemachanger/scexec/exec_immediate_mutation.go
@@ -218,6 +218,13 @@ func (s *immediateState) exec(ctx context.Context, c Catalog) error {
 	s.descriptorsToDelete.ForEach(func(id descpb.ID) {
 		s.modifiedDescriptors.Remove(id)
 	})
+
+	if len(s.newDescriptors) > 0 {
+		if err := c.CheckMaxSchemaObjects(ctx, len(s.newDescriptors)); err != nil {
+			return err
+		}
+	}
+
 	for _, newDescID := range getOrderedNewDescriptorIDs(s.newDescriptors) {
 		// Create new descs by the ascending order of their ID. This determinism
 		// helps avoid flakes in end-to-end tests in which we assert a particular
diff --git a/pkg/sql/schemachanger/scexec/executor_external_test.go b/pkg/sql/schemachanger/scexec/executor_external_test.go
@@ -76,6 +76,7 @@ func (ti testInfra) newExecDeps(txn descs.Txn) scexec.Dependencies {
 		noopMetadataUpdater{},
 		noopTemporarySchemaCreator{},
 		noopStatsReferesher{},
+		nil, /* tableStatsCache */
 		&scexec.TestingKnobs{},
 		kvTrace,
 		schemaChangerJobID,
diff --git a/pkg/sql/schemachanger/scexec/mocks_generated_test.go b/pkg/sql/schemachanger/scexec/mocks_generated_test.go
diff --git a/pkg/sql/schemachanger/schemachanger_test.go b/pkg/sql/schemachanger/schemachanger_test.go
@@ -1158,3 +1158,98 @@ CREATE TABLE other_schema.t1(n int REFERENCES complex_drop_schema.t1(n));
 		grp.Wait(),
 		`cannot create "complex_drop_schema.sc1" because the target database or schema does not exist`)
 }
+
+// TestApproxMaxSchemaObjects tests that the approximate max schema objects
+// guardrail works correctly with both the legacy and declarative schema changers.
+func TestApproxMaxSchemaObjects(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+
+	// Test with both declarative schema changer modes.
+	for _, useDeclarative := range []bool{false, true} {
+		t.Run(fmt.Sprintf("declarative=%t", useDeclarative), func(t *testing.T) {
+			s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
+			defer s.Stopper().Stop(ctx)
+
+			tdb := sqlutils.MakeSQLRunner(sqlDB)
+
+			// Configure the declarative schema changer mode.
+			if useDeclarative {
+				tdb.Exec(t, `SET use_declarative_schema_changer = 'on'`)
+			} else {
+				tdb.Exec(t, `SET use_declarative_schema_changer = 'off'`)
+			}
+
+			// Get the current count of descriptors to set a realistic limit.
+			var currentCount int
+			tdb.QueryRow(t, `SELECT count(*) FROM system.descriptor`).Scan(&currentCount)
+
+			// Set the limit to be slightly more than current count.
+			maxObjects := currentCount + 5
+			tdb.Exec(t, fmt.Sprintf(`SET CLUSTER SETTING sql.schema.approx_max_object_count = %d`, maxObjects))
+
+			// Create a test database and use it.
+			tdb.Exec(t, `CREATE DATABASE testdb`)
+			tdb.Exec(t, `USE testdb`)
+
+			// Test that different object types are subject to the limit.
+			objectTypes := []string{"table", "database", "schema", "type", "function"}
+			for _, objectType := range objectTypes {
+				t.Run(objectType, func(t *testing.T) {
+					// Increase the limit before each subtest to avoid interference.
+					maxObjects += 10
+					tdb.Exec(t, fmt.Sprintf(`SET CLUSTER SETTING sql.schema.approx_max_object_count = %d`, maxObjects))
+
+					// Try to create objects until we hit the limit.
+					// ANALYZE before starting to ensure stats are fresh.
+					tdb.Exec(t, `ANALYZE system.descriptor`)
+
+					objNum := 0
+					for {
+						var createStmt string
+						switch objectType {
+						case "table":
+							createStmt = fmt.Sprintf(`CREATE TABLE t%d (id INT PRIMARY KEY)`, objNum)
+						case "database":
+							createStmt = fmt.Sprintf(`CREATE DATABASE db%d`, objNum)
+						case "schema":
+							createStmt = fmt.Sprintf(`CREATE SCHEMA sc%d`, objNum)
+						case "type":
+							createStmt = fmt.Sprintf(`CREATE TYPE enum%d AS ENUM ('a', 'b', 'c')`, objNum)
+						case "function":
+							createStmt = fmt.Sprintf(`CREATE FUNCTION f%d() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$`, objNum)
+						}
+
+						_, err := sqlDB.Exec(createStmt)
+						if err != nil {
+							// Check if we got the expected error.
+							if pqErr := (*pq.Error)(nil); errors.As(err, &pqErr) {
+								if string(pqErr.Code) == pgcode.ConfigurationLimitExceeded.String() {
+									// Verify the error message mentions "approximate maximum".
+									testutils.IsError(err, "would exceed approximate maximum")
+									break
+								}
+							}
+							// Some other error occurred.
+							t.Fatal(err)
+						}
+						objNum++
+
+						// Re-analyze periodically to update stats.
+						if objNum%2 == 0 {
+							tdb.Exec(t, `ANALYZE system.descriptor`)
+						}
+
+						// Safety check: if we created way more objects than expected,
+						// something is wrong.
+						if objNum > 30 {
+							t.Fatalf("created %d %ss without hitting limit (max=%d)", objNum, objectType, maxObjects)
+						}
+					}
+				})
+			}
+		})
+	}
+}
diff --git a/pkg/sql/schemachanger/scjob/job.go b/pkg/sql/schemachanger/scjob/job.go
@@ -130,6 +130,7 @@ func (n *newSchemaChangeResumer) run(ctx context.Context, execCtxI interface{})
 			)
 		},
 		execCfg.StatsRefresher,
+		execCfg.TableStatsCache,
 		execCfg.DeclarativeSchemaChangerTestingKnobs,
 		payload.Statement,
 		execCtx.SessionData(),
diff --git a/pkg/sql/schemaobjectlimit/BUILD.bazel b/pkg/sql/schemaobjectlimit/BUILD.bazel
@@ -0,0 +1,19 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "schemaobjectlimit",
+    srcs = ["check.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/sql/schemaobjectlimit",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/keys",
+        "//pkg/settings/cluster",
+        "//pkg/sql/catalog/descs",
+        "//pkg/sql/isql",
+        "//pkg/sql/pgwire/pgcode",
+        "//pkg/sql/pgwire/pgerror",
+        "//pkg/sql/sqlclustersettings",
+        "//pkg/sql/stats",
+        "@com_github_cockroachdb_errors//:errors",
+    ],
+)
diff --git a/pkg/sql/schemaobjectlimit/check.go b/pkg/sql/schemaobjectlimit/check.go
diff --git a/pkg/sql/sqlclustersettings/clustersettings.go b/pkg/sql/sqlclustersettings/clustersettings.go
