Merge #148576 #148796

148576: sql: enable create_table_with_schema_locked by default r=fqazi a=fqazi

To achieve more optimal change feed behavior, users previously had to manually enable the `schema_locked` setting on every new table.

This change simplifies the process by enabling the `create_table_with_schema_locked session` variable by default. The SQL package is also updated to automatically disable `schema_locked` when explicit transactions or the legacy schema changer are used.

In support of this change, tests and workloads across various packages have been updated to handle tables as schema-locked by default. This includes updates to tests in:

- workload/movr
- kvfollowereadccl
- telemetryccl
- spanconfigccl
- crosscluster
- acceptance
- importccl
- partitionccl
- rttanalysis
- multiregion
- changefeedccl
- backup

Note: The test updates have a commit per package to make things easier to follow. They will be squashed before merging.

Fixes: #129694
Release note (SQL changes): The `create_table_with_schema_locked` session variable is now enabled by default. As a result, all new tables will be created as `schema-locked`. This setting must be manually disabled before running operations that do not support schema-locking automatically, such as explicit transactions or `ALTER TABLE ... SET LOCALITY`.

148796: workflows: run update_releases on release-25.3 r=jlinder a=rail

Release note: none
Epic: none

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>

Author: 3 people

.github/workflows/update_releases.yaml

@@ -32,6 +32,7 @@ jobs:
           - "release-24.3"
           - "release-25.1"
           - "release-25.2"
+          - "release-25.3"
     name: Update pkg/testutils/release/cockroach_releases.yaml on ${{ matrix.branch }}
     runs-on: ubuntu-latest
     steps:

pkg/backup/backup_test.go

@@ -8777,8 +8777,15 @@ func TestBackupOnlyPublicIndexes(t *testing.T) {
 	// appear in the backup once it is PUBLIC.
 	var g errgroup.Group
 	g.Go(func() error {
+		// This test intentionally uses hooks that require the legacy schema changer. Because
+		// the legacy schema changer cannot toggle schema_locked for backfilling schema changes,
+		// we will manually do that here.
+		_, err := sqlDB.DB.ExecContext(ctx, `ALTER TABLE data.bank SET (schema_locked=false)`)
+		if err != nil {
+			return errors.Wrap(err, "unsetting schema_locked")
+		}
 		// We use the underlying DB since the goroutine should not call t.Fatal.
-		_, err := sqlDB.DB.ExecContext(ctx,
+		_, err = sqlDB.DB.ExecContext(ctx,
 			`CREATE INDEX new_balance_idx ON data.bank(balance)`)
 		return errors.Wrap(err, "creating index")
 	})
@@ -11158,7 +11165,7 @@ CREATE TABLE child_pk (k INT8 PRIMARY KEY REFERENCES parent);
 		sqlDB.Exec(t, `CREATE DATABASE test`)
 		sqlDB.Exec(t, fmt.Sprintf(`RESTORE TABLE test.circular FROM LATEST IN $1 AS OF SYSTEM TIME %s`, ts[0]), localFoo)
 		require.Equal(t, [][]string{
-			{"test.public.circular", "CREATE TABLE public.circular (\n\tk INT8 NOT NULL,\n\tselfid INT8 NULL,\n\tCONSTRAINT circular_pkey PRIMARY KEY (k ASC),\n\tCONSTRAINT self_fk FOREIGN KEY (selfid) REFERENCES public.circular(selfid) NOT VALID,\n\tUNIQUE INDEX circular_selfid_key (selfid ASC)\n);"},
+			{"test.public.circular", "CREATE TABLE public.circular (\n\tk INT8 NOT NULL,\n\tselfid INT8 NULL,\n\tCONSTRAINT circular_pkey PRIMARY KEY (k ASC),\n\tCONSTRAINT self_fk FOREIGN KEY (selfid) REFERENCES public.circular(selfid) NOT VALID,\n\tUNIQUE INDEX circular_selfid_key (selfid ASC)\n) WITH (schema_locked = true);"},
 		}, sqlDB.QueryStr(t, `SHOW CREATE TABLE test.circular`))
 		sqlDB.Exec(t, fmt.Sprintf(`RESTORE TABLE test.parent, test.child FROM LATEST IN $1 AS OF SYSTEM TIME %s `, ts[0]), localFoo)
 		sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)

pkg/backup/show_test.go

@@ -229,7 +229,7 @@ ORDER BY object_type, object_name`, full)
 		b INT8 NULL,
 		CONSTRAINT tablea_pkey PRIMARY KEY (a ASC),
 		INDEX tablea_b_idx (b ASC)
-	)`
+	) WITH (schema_locked = true)`
 		expectedCreateView := "CREATE VIEW viewa (\n\ta\n) AS SELECT a FROM data.public.tablea"
 		expectedCreateSeq := `CREATE SEQUENCE seqa MINVALUE 1 MAXVALUE 20 INCREMENT 2 START 1`
 
@@ -263,13 +263,13 @@ ORDER BY object_type, object_name`, full)
 				b INT8 NULL,
 				CONSTRAINT fkreftable_pkey PRIMARY KEY (a ASC),
 				CONSTRAINT fkreftable_b_fkey FOREIGN KEY (b) REFERENCES public.fksrc(a)
-			)`
+			) WITH (schema_locked = true)`
 		wantDiffDB := `CREATE TABLE fkreftable (
 				a INT8 NOT NULL,
 				b INT8 NULL,
 				CONSTRAINT fkreftable_pkey PRIMARY KEY (a ASC),
 				CONSTRAINT fkreftable_b_fkey FOREIGN KEY (b) REFERENCES data.public.fksrc(a)
-			)`
+			) WITH (schema_locked = true)`
 
 		showBackupRows = sqlDBRestore.QueryStr(t, fmt.Sprintf(`SELECT create_statement FROM [SHOW BACKUP SCHEMAS FROM LATEST IN '%s'] WHERE object_type='table'`, includedFK))
 		createStmtSameDB := showBackupRows[1][0]
@@ -293,7 +293,7 @@ ORDER BY object_type, object_name`, full)
 				a INT8 NOT NULL,
 				b INT8 NULL,
 				CONSTRAINT fkreftable_pkey PRIMARY KEY (a ASC)
-			)`
+			) WITH (schema_locked = true)`
 
 		showBackupRows = sqlDBRestore.QueryStr(t, fmt.Sprintf(`SELECT create_statement FROM [SHOW BACKUP SCHEMAS FROM LATEST IN '%s'] WHERE object_type='table'`, missingFK))
 		createStmt := showBackupRows[0][0]

pkg/backup/testdata/backup-restore/multiregion-mismatch-table-locality

@@ -22,6 +22,13 @@ defaultdb root <nil> <nil> {} <nil>
 postgres root <nil> <nil> {} <nil>
 system node <nil> <nil> {} <nil>
 
+# We need to currently unset and set schema_locked when
+# modifying table locality, since this functionality is not
+# supported in the declarative schema changer.
+exec-sql
+ALTER TABLE d.t SET (schema_locked=false);
+----
+
 # make our table regional by row
 exec-sql
 ALTER TABLE d.t SET LOCALITY REGIONAL BY ROW;

pkg/backup/testdata/backup-restore/online-restore-auto-stats

@@ -44,7 +44,7 @@ data.public.reg CREATE TABLE public.reg (
 	i INT8 NOT NULL,
 	s STRING NULL,
 	CONSTRAINT reg_pkey PRIMARY KEY (i ASC)
-);
+) WITH (schema_locked = true);
 
 query-sql
 SHOW CREATE TABLE data.stats
@@ -53,7 +53,7 @@ data.public.stats CREATE TABLE public.stats (
 	i INT8 NOT NULL,
 	s STRING NULL,
 	CONSTRAINT stats_pkey PRIMARY KEY (i ASC)
-) WITH (sql_stats_automatic_collection_enabled = true);
+) WITH (sql_stats_automatic_collection_enabled = true, schema_locked = true);
 
 query-sql
 SHOW CREATE TABLE data.nostats
@@ -62,4 +62,4 @@ data.public.nostats CREATE TABLE public.nostats (
 	i INT8 NOT NULL,
 	s STRING NULL,
 	CONSTRAINT nostats_pkey PRIMARY KEY (i ASC)
-) WITH (sql_stats_automatic_collection_enabled = false);
+) WITH (sql_stats_automatic_collection_enabled = false, schema_locked = true);

pkg/backup/testdata/backup-restore/online-restore-cluster

@@ -33,4 +33,3 @@ SELECT count(*) FROM [SHOW JOBS] WHERE description LIKE '%Background Data Downlo
 1
 
 
-

pkg/backup/testdata/backup-restore/restore-regionless-regional-by-row

@@ -27,6 +27,13 @@ defaultdb root <nil> <nil> {} <nil>
 postgres root <nil> <nil> {} <nil>
 system node <nil> <nil> {} <nil>
 
+# We need to currently unset and set schema_locked when
+# modifying table locality, since this functionality is not
+# supported in the declarative schema changer.
+exec-sql
+ALTER TABLE d.t SET (schema_locked=false);
+----
+
 # make our table regional by row
 exec-sql
 ALTER TABLE d.t SET LOCALITY REGIONAL BY ROW;

pkg/backup/testdata/backup-restore/restore-schema-only

@@ -20,7 +20,7 @@ d.public.t1 CREATE TABLE public.t1 (
 	x INT8 NULL,
 	rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
 	CONSTRAINT t1_pkey PRIMARY KEY (rowid ASC)
-);
+) WITH (schema_locked = true);
 COMMENT ON TABLE public.t1 IS 'This comment better get restored from the backed up system table!';
 
 # drop and create defaultDB to ensure it has a higher ID than by default. We will check that when
@@ -104,7 +104,7 @@ d.public.t1 CREATE TABLE public.t1 (
 	x INT8 NULL,
 	rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
 	CONSTRAINT t1_pkey PRIMARY KEY (rowid ASC)
-);
+) WITH (schema_locked = true);
 COMMENT ON TABLE public.t1 IS 'This comment better get restored from the backed up system table!';
 
 # Ensure the defaultdb from the backed up cluster was restored.

pkg/backup/testdata/backup-restore/restore-validation-only

@@ -26,7 +26,7 @@ d.public.t1 CREATE TABLE public.t1 (
 	x INT8 NULL,
 	rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
 	CONSTRAINT t1_pkey PRIMARY KEY (rowid ASC)
-);
+) WITH (schema_locked = true);
 COMMENT ON TABLE public.t1 IS 'This comment better get restored from the backed up system table!';
 
 # drop and create defaultDB to ensure it has a higher ID than by default. We will check that when
@@ -111,7 +111,7 @@ d.public.t1 CREATE TABLE public.t1 (
 	x INT8 NULL,
 	rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
 	CONSTRAINT t1_pkey PRIMARY KEY (rowid ASC)
-);
+) WITH (schema_locked = true);
 COMMENT ON TABLE public.t1 IS 'This comment better get restored from the backed up system table!';
 
 # Ensure the defaultdb from the backed up cluster was restored.

pkg/backup/testdata/backup-restore/row_level_ttl

@@ -39,7 +39,7 @@ CREATE TABLE public.t (
 	id INT8 NOT NULL,
 	crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
 	CONSTRAINT t_pkey PRIMARY KEY (id ASC)
-) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL);
+) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, schema_locked = true);
 
 query-sql
 SELECT count(1) FROM [SHOW SCHEDULES]
@@ -72,7 +72,7 @@ CREATE TABLE public.t (
 	id INT8 NOT NULL,
 	crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
 	CONSTRAINT t_pkey PRIMARY KEY (id ASC)
-) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL);
+) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, schema_locked = true);
 
 query-sql
 SELECT count(1) FROM [SHOW SCHEDULES]
@@ -109,7 +109,7 @@ CREATE TABLE public.t (
 	id INT8 NOT NULL,
 	crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
 	CONSTRAINT t_pkey PRIMARY KEY (id ASC)
-) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL);
+) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, schema_locked = true);
 
 query-sql
 SELECT count(1) FROM [SHOW SCHEDULES]