sql: fix mixed-version behaviour of create_tenant()

Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None

pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go

@@ -82,6 +82,7 @@ func TestTenantUpgrade(t *testing.T) {
 			cleanupPGUrl()
 		}
 	}
+	expectedInitialTenantVersion, _, _ := v0v1v2()
 	mkTenant := func(t *testing.T, id uint64) (tenantDB *gosql.DB, cleanup func()) {
 		settings := cluster.MakeTestingClusterSettingsWithVersions(
 			clusterversion.TestingBinaryVersion,
@@ -90,7 +91,7 @@ func TestTenantUpgrade(t *testing.T) {
 		)
 		// Initialize the version to the minimum it could be.
 		require.NoError(t, clusterversion.Initialize(ctx,
-			clusterversion.TestingBinaryMinSupportedVersion, &settings.SV))
+			expectedInitialTenantVersion, &settings.SV))
 		tenantArgs := base.TestTenantArgs{
 			TenantID:     roachpb.MustMakeTenantID(id),
 			TestingKnobs: base.TestingKnobs{},
@@ -109,7 +110,7 @@ func TestTenantUpgrade(t *testing.T) {
 
 		// Ensure that the tenant works.
 		initialTenantRunner.CheckQueryResults(t, "SHOW CLUSTER SETTING version",
-			[][]string{{clusterversion.TestingBinaryMinSupportedVersion.String()}})
+			[][]string{{expectedInitialTenantVersion.String()}})
 		initialTenantRunner.Exec(t, "CREATE TABLE t (i INT PRIMARY KEY)")
 		initialTenantRunner.Exec(t, "INSERT INTO t VALUES (1), (2)")
 
@@ -172,11 +173,9 @@ func TestTenantUpgrade(t *testing.T) {
 
 }
 
-// Returns two versions v0, v1, v2 which correspond to adjacent releases. v0 will
-// equal the TestingBinaryMinSupportedVersion to avoid rot in tests using this
-// (as we retire old versions).
+// Returns two versions v0, v1, v2 which correspond to adjacent releases.
 func v0v1v2() (roachpb.Version, roachpb.Version, roachpb.Version) {
-	v0 := clusterversion.TestingBinaryMinSupportedVersion
+	v0 := clusterversion.ByKey(clusterversion.V22_2)
 	v1 := clusterversion.TestingBinaryVersion
 	v2 := clusterversion.TestingBinaryVersion
 	if v1.Internal > 2 {

pkg/jobs/BUILD.bazel

@@ -44,6 +44,8 @@ go_library(
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/descs",
+        "//pkg/sql/pgwire/pgcode",
+        "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/protoreflect",
         "//pkg/sql/sem/builtins",
         "//pkg/sql/sem/tree",

pkg/jobs/registry.go

@@ -29,6 +29,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/server/tracedumper"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/builtins"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -514,28 +516,38 @@ func (r *Registry) CreateJobWithTxn(
 		return nil, err
 	}
 
-	cols := [7]string{"id", "status", "payload", "progress", "claim_session_id", "claim_instance_id", "job_type"}
-	numCols := len(cols)
-	vals := [7]interface{}{jobID, StatusRunning, payloadBytes, progressBytes, s.ID().UnsafeBytes(), r.ID(), jobType.String()}
-	placeholders := func() string {
-		var p strings.Builder
-		for i := 0; i < numCols; i++ {
-			if i > 0 {
-				p.WriteByte(',')
-			}
-			p.WriteByte('$')
-			p.WriteString(strconv.Itoa(i + 1))
+	insertStmt := `INSERT
+		INTO system.jobs (id, status, payload, progress, claim_session_id, claim_instance_id, job_type)
+		VALUES ($1, $2, $3, $4, $5, $6, $7)`
+	vals := []interface{}{
+		jobID,
+		StatusRunning,
+		payloadBytes,
+		progressBytes,
+		s.ID().UnsafeBytes(),
+		r.ID(),
+		jobType.String(),
+	}
+	if r.settings.Version.IsActive(ctx, clusterversion.V23_1AddTypeColumnToJobsTable) {
+		_, err = j.registry.ex.Exec(ctx, "job-row-insert", txn, insertStmt, vals...)
+		if err == nil {
+			return j, nil
 		}
-		return p.String()
-	}
-	// TODO(jayant): remove this version gate in 24.1
-	// To run the upgrade below, migration and schema change jobs will need
-	// to be created using the old schema of the jobs table.
-	if !r.settings.Version.IsActive(ctx, clusterversion.V23_1AddTypeColumnToJobsTable) {
-		numCols -= 1
-	}
-	insertStmt := fmt.Sprintf(`INSERT INTO system.jobs (%s) VALUES (%s)`, strings.Join(cols[:numCols], ","), placeholders())
-	if _, err = j.registry.ex.Exec(ctx, "job-row-insert", txn, insertStmt, vals[:numCols]...,
+		if pgerror.GetPGCode(err) != pgcode.UndefinedColumn {
+			return nil, err
+		}
+		// It's legitimately possible for this INSERT to fail in tenant clusters
+		// whose system schema was bootstrapped using values from V22.2. In that
+		// schema, the job_type column doesn't exist. Yet, the version gate may
+		// still be passed at startup because the active version isn't properly
+		// set yet at that point.
+		// TODO(postamar): remove this once the jobs_type column is baked in.
+	}
+	insertStmt = `INSERT
+		INTO system.jobs (id, status, payload, progress, claim_session_id, claim_instance_id)
+		VALUES ($1, $2, $3, $4, $5, $6)`
+	vals = vals[:len(vals)-1]
+	if _, err = j.registry.ex.Exec(ctx, "job-row-insert-without-type", txn, insertStmt, vals...,
 	); err != nil {
 		return nil, err
 	}

pkg/sql/catalog/bootstrap/BUILD.bazel

@@ -6,6 +6,7 @@ go_library(
     srcs = [
         "kv_writer.go",
         "metadata.go",
+        "previous_release.go",
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap",
     visibility = ["//visibility:public"],