Merge #118291 #118654 #118689

118291: schemachanger: deflate primary index chain logic include UWI constraint r=Xiang-Gu a=Xiang-Gu

Previously, stmts like `ADD COLUMN j INT, ADD UNIQUE WIHOUT INDEX (j)` will fail with an internal error because we forget to process those unique-without-index in the DSC if the primary index chain has been inflated and needs to be deflated.

Fixes #118246
Release note (bug fix): Fix a bug where stmts like `ADD COLUMN j INT, ADD UNIQUE WIHOUT INDEX (j)` would fail with an internal error.

118654: roachtest: make backup-restore/mixed-version initialization more flexible r=DarrylWong a=renatolabs

Previously, that test had a hardcoded wait of 3 minutes. However, now that concurrent steps might have a longer delay, this logic doesn't work.

In this commit, we make the wait logic more flexible by actually waiting until the databases we are interested in exist, which is what the previous hardcoded sleep was supposed to capture.

Fixes: #118628

Release note: None

118689: backupccl: skip `TestRestoreEntryCover_numBackups_12` under `race` r=rail,msbutler a=rickystewart

The memory consumption under `race` for this test is very excessive.

Epic: CRDB-8308
Release note: None

Co-authored-by: Xiang Gu <xiang@cockroachlabs.com>
Co-authored-by: Renato Costa <renato@cockroachlabs.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>

pkg/cmd/roachtest/tests/mixed_version_backup.go

@@ -1495,6 +1495,45 @@ func (mvb *mixedVersionBackup) setClusterSettings(
 	return u.setClusterSettings(ctx, l, rng)
 }
 
+// waitForDBs waits until every database in the `dbs` field
+// exists. Useful in case a mixed-version hook is called concurrently
+// with the process of actually creating the tables (e.g., workload
+// initialization).
+func (mvb *mixedVersionBackup) waitForDBs(
+	ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper,
+) error {
+	retryOptions := retry.Options{
+		InitialBackoff: 10 * time.Second,
+		MaxBackoff:     1 * time.Minute,
+		Multiplier:     1.5,
+		MaxRetries:     20,
+	}
+
+	for _, dbName := range mvb.dbs {
+		r := retry.StartWithCtx(ctx, retryOptions)
+		var err error
+		for r.Next() {
+			q := "SELECT 1 FROM [SHOW DATABASES] WHERE database_name = $1"
+			var n int
+			if err = h.QueryRow(rng, q, dbName).Scan(&n); err == nil {
+				break
+			}
+
+			l.Printf("waiting for DB %s (err: %v)", dbName, err)
+		}
+
+		if err != nil {
+			return fmt.Errorf("failed to wait for DB %s (last error: %w)", dbName, err)
+		}
+	}
+
+	// After every database exists, wait for a small amount of time to
+	// make sure *some* data exists (the workloads could be inserting
+	// data concurrently).
+	time.Sleep(1 * time.Minute)
+	return nil
+}
+
 // maybeTakePreviousVersionBackup creates a backup collection (full +
 // incremental), and is supposed to be called before any nodes are
 // upgraded. This ensures that we are able to restore this backup
@@ -1503,18 +1542,12 @@ func (mvb *mixedVersionBackup) setClusterSettings(
 func (mvb *mixedVersionBackup) maybeTakePreviousVersionBackup(
 	ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper,
 ) error {
-	// Wait here for a few minutes to allow the workloads (which are
-	// initializing concurrently with this step) to store some data in
-	// the cluster by the time the backup is taken. The actual wait time
-	// chosen is somewhat arbitrary: it's less time than workloads
-	// typically need to finish initializing (especially tpcc), so the
-	// backup is taken while data is still being inserted. The actual
-	// time is irrelevant as far as correctness is concerned: we should
-	// be able to restore this backup after upgrading regardless of the
-	// amount of data backed up.
-	wait := 3 * time.Minute
-	l.Printf("waiting for %s", wait)
-	time.Sleep(wait)
+	// Wait here to allow the workloads (which are initializing
+	// concurrently with this step) to store some data in the cluster by
+	// the time the backup is taken.
+	if err := mvb.waitForDBs(ctx, l, rng, h); err != nil {
+		return err
+	}
 
 	if err := mvb.initBackupRestoreTestDriver(ctx, l, rng); err != nil {
 		return err

pkg/sql/logictest/testdata/logic_test/alter_table

@@ -3495,9 +3495,43 @@ ALTER TABLE t_108974_v DROP COLUMN k;
 subtest end
 
 # Regression test for https://github.com/cockroachdb/cockroach/issues/110629.
+subtest 110629
+
 # Subqueries are not allowed in storage parameters.
 statement ok
 CREATE TABLE t_110629 (a INT PRIMARY KEY);
 
 statement error subqueries are not allowed in table storage parameters
 ALTER TABLE t SET ( 'string' = EXISTS ( TABLE error ) );
+
+subtest end
+
+# Regression test for https://github.com/cockroachdb/cockroach/issues/118246
+subtest 118246
+
+statement ok
+CREATE TABLE t_118246 (i INT PRIMARY KEY);
+INSERT INTO t_118246 VALUES (0);
+
+statement ok
+SET experimental_enable_unique_without_index_constraints = true;
+
+statement ok
+ALTER TABLE t_118246 ADD COLUMN j INT, ADD UNIQUE WITHOUT INDEX (j);
+
+statement ok
+ALTER TABLE t_118246 ADD COLUMN k INT DEFAULT 30, ADD UNIQUE WITHOUT INDEX (k);
+
+query T
+SELECT create_statement FROM [SHOW CREATE TABLE t_118246]
+----
+CREATE TABLE public.t_118246 (
+  i INT8 NOT NULL,
+  j INT8 NULL,
+  k INT8 NULL DEFAULT 30:::INT8,
+  CONSTRAINT t_118246_pkey PRIMARY KEY (i ASC),
+  CONSTRAINT unique_j UNIQUE WITHOUT INDEX (j),
+  CONSTRAINT unique_k UNIQUE WITHOUT INDEX (k)
+)
+
+subtest end

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go

@@ -1226,6 +1226,10 @@ func updateElementsToDependOnNewFromOld(
 			if e.IndexIDForValidation == old {
 				e.IndexIDForValidation = new
 			}
+		case *scpb.UniqueWithoutIndexConstraint:
+			if e.IndexIDForValidation == old {
+				e.IndexIDForValidation = new
+			}
 		}
 	})
 }