From 472d75f961ec1130807605a807a177fdd57dd101 Mon Sep 17 00:00:00 2001
From: Faizan Qazi <faizan@cockroachlabs.com>
Date: Tue, 4 Apr 2023 17:10:35 -0400
Subject: [PATCH] roachtest/version-upgrade: re-enable schema changer workload

Previously, the schema changer workload was disabled inside
the version upgrade test because of intermittent job errors.
This patch re-enables this workload again for the version
upgrade test. It also updates the test to handle mixed version
declarative schema changer support.

Fixes: #100709

Release note: None
---
 pkg/cmd/roachtest/tests/versionupgrade.go     |  4 --
 .../schemachange/operation_generator.go       | 65 ++++++++++++++++++-
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/pkg/cmd/roachtest/tests/versionupgrade.go b/pkg/cmd/roachtest/tests/versionupgrade.go
index 47e549003bb5..b3b3c76aa5ce 100644
--- a/pkg/cmd/roachtest/tests/versionupgrade.go
+++ b/pkg/cmd/roachtest/tests/versionupgrade.go
@@ -134,10 +134,6 @@ func runVersionUpgrade(ctx context.Context, t test.Test, c cluster.Cluster) {
 	mvt.InMixedVersion(
 		"test schema change step",
 		func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error {
-			if c.IsLocal() {
-				l.Printf("skipping step in bors builds while failures are handled -- #99115")
-				return nil
-			}
 			l.Printf("running schema workload step")
 			runCmd := roachtestutil.NewCommand("./workload run schemachange").Flag("verbose", 1).Flag("max-ops", 10).Flag("concurrency", 2).Arg("{pgurl:1-%d}", len(c.All()))
 			randomNode := h.RandomNode(rng, c.All())
diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go
index 2e960aff5b02..42c994637e04 100644
--- a/pkg/workload/schemachange/operation_generator.go
+++ b/pkg/workload/schemachange/operation_generator.go
@@ -302,9 +302,9 @@ var opDeclarative = []bool{
 	createEnum:              false,
 	createSchema:            false,
 	dropColumn:              true,
-	dropColumnDefault:       true,
+	dropColumnDefault:       false,
 	dropColumnNotNull:       true,
-	dropColumnStored:        true,
+	dropColumnStored:        false,
 	dropConstraint:          true,
 	dropIndex:               true,
 	dropSequence:            true,
@@ -326,6 +326,31 @@ var opDeclarative = []bool{
 	validate:                false,
 }
 
+var opDeclarativeVersion = []clusterversion.Key{
+	addColumn:               clusterversion.V22_2,
+	addForeignKeyConstraint: clusterversion.V23_1,
+	addUniqueConstraint:     clusterversion.V23_1,
+	createIndex:             clusterversion.V23_1,
+	dropColumn:              clusterversion.V22_2,
+	dropColumnNotNull:       clusterversion.V23_1,
+	dropConstraint:          clusterversion.V23_1,
+	dropIndex:               clusterversion.V23_1,
+	dropSequence:            clusterversion.BinaryMinSupportedVersionKey,
+	dropTable:               clusterversion.BinaryMinSupportedVersionKey,
+	dropView:                clusterversion.BinaryMinSupportedVersionKey,
+	dropSchema:              clusterversion.BinaryMinSupportedVersionKey,
+}
+
+func init() {
+	// Assert that an active version is set for all declarative statements.
+	for op := range opDeclarative {
+		if opDeclarative[op] &&
+			opDeclarativeVersion[op] < clusterversion.BinaryMinSupportedVersionKey {
+			panic(errors.AssertionFailedf("declarative op %v doesn't have an active version", op))
+		}
+	}
+}
+
 // adjustOpWeightsForActiveVersion adjusts the weights for the active cockroach
 // version, allowing us to disable certain operations in mixed version scenarios.
 func adjustOpWeightsForCockroachVersion(
@@ -350,6 +375,28 @@ func adjustOpWeightsForCockroachVersion(
 	return tx.Rollback(ctx)
 }
 
+// getSupportedDeclarativeOp generates declarative operations until,
+// a fully supported one is found. This is required for mixed version testing
+// support, where statements may be partially supproted.
+func (og *operationGenerator) getSupportedDeclarativeOp(
+	ctx context.Context, tx pgx.Tx,
+) (opType, error) {
+	for {
+		op := opType(og.params.declarativeOps.Int())
+		if !clusterversion.TestingBinaryMinSupportedVersion.Equal(
+			clusterversion.ByKey(opDeclarativeVersion[op])) {
+			notSupported, err := isClusterVersionLessThan(ctx, tx, clusterversion.ByKey(opDeclarativeVersion[op]))
+			if err != nil {
+				return op, err
+			}
+			if notSupported {
+				continue
+			}
+		}
+		return op, nil
+	}
+}
+
 // randOp attempts to produce a random schema change operation. It returns a
 // triple `(randOp, log, error)`. On success `randOp` is the random schema
 // change constructed. Constructing a random schema change may require a few
@@ -362,7 +409,10 @@ func (og *operationGenerator) randOp(
 		var op opType
 		// The declarative schema changer has a more limited deck of operations.
 		if useDeclarativeSchemaChanger {
-			op = opType(og.params.declarativeOps.Int())
+			op, err = og.getSupportedDeclarativeOp(ctx, tx)
+			if err != nil {
+				return nil, err
+			}
 		} else {
 			op = opType(og.params.ops.Int())
 		}
@@ -3516,6 +3566,15 @@ func (og *operationGenerator) createSchema(ctx context.Context, tx pgx.Tx) (*opS
 	// TODO(jayshrivastava): Support authorization
 	stmt := randgen.MakeSchemaName(ifNotExists, schemaName, tree.MakeRoleSpecWithRoleName(username.RootUserName().Normalized()))
 	opStmt.sql = tree.Serialize(stmt)
+	// Descriptor ID generator may be temporarily unavailable, so
+	// allow uncategorized errors temporarily.
+	potentialDescIDGeneratorError, err := maybeExpectPotentialDescIDGenerationError(ctx, tx)
+	if err != nil {
+		return nil, err
+	}
+	codesWithConditions{
+		{code: pgcode.Uncategorized, condition: potentialDescIDGeneratorError},
+	}.add(opStmt.potentialExecErrors)
 	return opStmt, nil
 }