Merge #76116

76116: schemachanger,scpb,catalog: move schema changer state into descriptors r=ajwerner a=ajwerner This work is in support of #73071. The first two commit breaks dependencies so descpb can import scpb. Then we add the state to the descriptor protos. Finally we adopt for use. This does not yet test or synthesize the job. Co-authored-by: Andrew Werner <[email protected]>
cockroachdb · Feb 12, 2022 · 2bc16c3 · 2bc16c3
2 parents 80f1c2c + 0dfd6ee
commit 2bc16c3
Show file tree

Hide file tree

Showing 147 changed files with 6,344 additions and 1,272 deletions.
diff --git a/docs/generated/redact_safe.md b/docs/generated/redact_safe.md
@@ -6,7 +6,6 @@ pkg/base/node_id.go | `*NodeIDContainer`
 pkg/base/node_id.go | `*SQLIDContainer`
 pkg/base/node_id.go | `*StoreIDContainer`
 pkg/cli/exit/exit.go | `Code`
-pkg/jobs/jobspb/wrap.go | `JobID`
 pkg/jobs/jobspb/wrap.go | `Type`
 pkg/kv/kvserver/closedts/ctpb/service.go | `LAI`
 pkg/kv/kvserver/closedts/ctpb/service.go | `SeqNum`
@@ -25,6 +24,7 @@ pkg/roachpb/method.go | `Method`
 pkg/roachpb/tenant.go | `TenantID`
 pkg/rpc/connection_class.go | `ConnectionClass`
 pkg/sql/catalog/catpb/constraint.go | `ForeignKeyAction`
+pkg/sql/catalog/catpb/job_id.go | `JobID`
 pkg/sql/catalog/descpb/structured.go | `ConstraintType`
 pkg/sql/catalog/descpb/structured.go | `ConstraintValidity`
 pkg/sql/catalog/descpb/structured.go | `DescriptorMutation_Direction`

diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -203,12 +203,14 @@ ALL_TESTS = [
     "//pkg/spanconfig/spanconfigtestutils:spanconfigtestutils_test",
     "//pkg/sql/catalog/catalogkeys:catalogkeys_test",
     "//pkg/sql/catalog/catformat:catformat_test",
+    "//pkg/sql/catalog/catpb:catpb_test",
     "//pkg/sql/catalog/catprivilege:catprivilege_test",
     "//pkg/sql/catalog/colinfo:colinfo_test",
     "//pkg/sql/catalog/dbdesc:dbdesc_test",
     "//pkg/sql/catalog/descpb:descpb_test",
     "//pkg/sql/catalog/descs:descs_test",
     "//pkg/sql/catalog/hydratedtables:hydratedtables_test",
+    "//pkg/sql/catalog/internal/validate:validate_test",
     "//pkg/sql/catalog/lease:lease_test",
     "//pkg/sql/catalog/multiregion:multiregion_test",
     "//pkg/sql/catalog/nstree:nstree_test",

diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go
@@ -31,6 +31,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catprivilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descidgen"
@@ -1323,7 +1324,7 @@ func remapPublicSchemas(
 		db.AddSchemaToDatabase(tree.PublicSchema, descpb.DatabaseDescriptor_SchemaInfo{ID: id})
 		// Every database must be initialized with the public schema.
 		// Create the SchemaDescriptor.
-		publicSchemaPrivileges := descpb.NewPublicSchemaPrivilegeDescriptor()
+		publicSchemaPrivileges := catpb.NewPublicSchemaPrivilegeDescriptor()
 		publicSchemaDesc := schemadesc.NewBuilder(&descpb.SchemaDescriptor{
 			ParentID:   db.GetID(),
 			Name:       tree.PublicSchema,
@@ -2132,7 +2133,7 @@ func (r *restoreResumer) dropDescriptors(
 		// and so we don't need to preserve MVCC semantics.
 		tableToDrop.DropTime = dropTime
 		b.Del(catalogkeys.EncodeNameKey(codec, tableToDrop))
-		descsCol.AddDeletedDescriptor(tableToDrop)
+		descsCol.AddDeletedDescriptor(tableToDrop.GetID())
 		if err := descsCol.WriteDescToBatch(ctx, false /* kvTrace */, tableToDrop, b); err != nil {
 			return errors.Wrap(err, "writing dropping table to batch")
 		}
@@ -2160,7 +2161,7 @@ func (r *restoreResumer) dropDescriptors(
 		}
 		// Remove the system.descriptor entry.
 		b.Del(catalogkeys.MakeDescMetadataKey(codec, typDesc.ID))
-		descsCol.AddDeletedDescriptor(mutType)
+		descsCol.AddDeletedDescriptor(mutType.GetID())
 	}
 
 	// Queue a GC job.
@@ -2232,7 +2233,7 @@ func (r *restoreResumer) dropDescriptors(
 
 		b.Del(catalogkeys.EncodeNameKey(codec, mutSchema))
 		b.Del(catalogkeys.MakeDescMetadataKey(codec, mutSchema.GetID()))
-		descsCol.AddDeletedDescriptor(mutSchema)
+		descsCol.AddDeletedDescriptor(mutSchema.GetID())
 		dbsWithDeletedSchemas[mutSchema.GetParentID()] = append(dbsWithDeletedSchemas[mutSchema.GetParentID()], mutSchema)
 	}
 
@@ -2311,7 +2312,7 @@ func (r *restoreResumer) dropDescriptors(
 
 		nameKey := catalogkeys.MakeDatabaseNameKey(codec, db.GetName())
 		b.Del(nameKey)
-		descsCol.AddDeletedDescriptor(db)
+		descsCol.AddDeletedDescriptor(db.GetID())
 		deletedDBs[db.GetID()] = struct{}{}
 	}
 
@@ -2409,7 +2410,7 @@ func getRestoringPrivileges(
 	user security.SQLUsername,
 	wroteDBs map[descpb.ID]catalog.DatabaseDescriptor,
 	descCoverage tree.DescriptorCoverage,
-) (updatedPrivileges *descpb.PrivilegeDescriptor, err error) {
+) (updatedPrivileges *catpb.PrivilegeDescriptor, err error) {
 	switch desc := desc.(type) {
 	case catalog.TableDescriptor:
 		return getRestorePrivilegesForTableOrSchema(
@@ -2438,14 +2439,14 @@ func getRestoringPrivileges(
 		// the restoring cluster match the ones that were on the cluster that was
 		// backed up. So we wipe the privileges on the type.
 		if descCoverage == tree.RequestedDescriptors {
-			updatedPrivileges = descpb.NewBasePrivilegeDescriptor(user)
+			updatedPrivileges = catpb.NewBasePrivilegeDescriptor(user)
 		}
 	case catalog.DatabaseDescriptor:
 		// If the restore is not a cluster restore we cannot know that the users on
 		// the restoring cluster match the ones that were on the cluster that was
 		// backed up. So we wipe the privileges on the database.
 		if descCoverage == tree.RequestedDescriptors {
-			updatedPrivileges = descpb.NewBaseDatabasePrivilegeDescriptor(user)
+			updatedPrivileges = catpb.NewBaseDatabasePrivilegeDescriptor(user)
 		}
 	}
 	return updatedPrivileges, nil
@@ -2460,7 +2461,7 @@ func getRestorePrivilegesForTableOrSchema(
 	wroteDBs map[descpb.ID]catalog.DatabaseDescriptor,
 	descCoverage tree.DescriptorCoverage,
 	privilegeType privilege.ObjectType,
-) (updatedPrivileges *descpb.PrivilegeDescriptor, err error) {
+) (updatedPrivileges *catpb.PrivilegeDescriptor, err error) {
 	if wrote, ok := wroteDBs[desc.GetParentID()]; ok {
 		// If we're creating a new database in this restore, the privileges of the
 		// table and schema should be that of the parent DB.

diff --git a/pkg/ccl/backupccl/restore_schema_change_creation.go b/pkg/ccl/backupccl/restore_schema_change_creation.go
@@ -207,7 +207,7 @@ func createSchemaChangeJobsFromMutations(
 		}
 		newMutationJob := descpb.TableDescriptor_MutationJob{
 			MutationID: mutationID,
-			JobID:      int64(jobID),
+			JobID:      jobID,
 		}
 		mutationJobs = append(mutationJobs, newMutationJob)
 

diff --git a/pkg/ccl/importccl/BUILD.bazel b/pkg/ccl/importccl/BUILD.bazel
@@ -175,6 +175,7 @@ go_test(
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/bootstrap",
         "//pkg/sql/catalog/catformat",
+        "//pkg/sql/catalog/catpb",
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/dbdesc",
         "//pkg/sql/catalog/descpb",

diff --git a/pkg/ccl/importccl/import_job.go b/pkg/ccl/importccl/import_job.go
@@ -1462,7 +1462,7 @@ func (r *importResumer) dropTables(
 			newTableDesc.DropTime = dropTime
 			b.Del(catalogkeys.EncodeNameKey(execCfg.Codec, newTableDesc))
 			tablesToGC = append(tablesToGC, newTableDesc.ID)
-			descsCol.AddDeletedDescriptor(newTableDesc)
+			descsCol.AddDeletedDescriptor(newTableDesc.GetID())
 		} else {
 			// IMPORT did not create this table, so we should not drop it.
 			newTableDesc.SetPublic()

diff --git a/pkg/ccl/importccl/import_table_creation.go b/pkg/ccl/importccl/import_table_creation.go
@@ -75,7 +75,7 @@ func MakeTestingSimpleTableDescriptor(
 			ID:       parentSchemaID,
 			Version:  1,
 			ParentID: parentID,
-			Privileges: descpb.NewPrivilegeDescriptor(
+			Privileges: catpb.NewPrivilegeDescriptor(
 				security.PublicRoleName(),
 				privilege.SchemaPrivileges,
 				privilege.List{},
@@ -185,7 +185,7 @@ func MakeSimpleTableDescriptor(
 		tableID,
 		nil, /* regionConfig */
 		hlc.Timestamp{WallTime: walltime},
-		descpb.NewBasePrivilegeDescriptor(security.AdminRoleName()),
+		catpb.NewBasePrivilegeDescriptor(security.AdminRoleName()),
 		affected,
 		semaCtx,
 		&evalCtx,

diff --git a/pkg/ccl/importccl/read_import_mysql.go b/pkg/ccl/importccl/read_import_mysql.go
@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
@@ -427,7 +428,7 @@ func mysqlTableToCockroach(
 			seqVals[id] = startingValue
 		}
 		var err error
-		privilegeDesc := descpb.NewBasePrivilegeDescriptor(owner)
+		privilegeDesc := catpb.NewBasePrivilegeDescriptor(owner)
 		seqDesc, err = sql.NewSequenceTableDesc(
 			ctx,
 			nil, /* planner */

diff --git a/pkg/ccl/importccl/read_import_pgdump.go b/pkg/ccl/importccl/read_import_pgdump.go
@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descidgen"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
@@ -329,7 +330,7 @@ func createPostgresSequences(
 			schema.GetID(),
 			id,
 			hlc.Timestamp{WallTime: walltime},
-			descpb.NewBasePrivilegeDescriptor(owner),
+			catpb.NewBasePrivilegeDescriptor(owner),
 			tree.PersistencePermanent,
 			// If this is multi-region, this will get added by WriteDescriptors.
 			false, /* isMultiRegion */

diff --git a/pkg/ccl/importccl/testutils_test.go b/pkg/ccl/importccl/testutils_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
@@ -57,7 +58,7 @@ func descForTable(
 		name := parsed[0].AST.(*tree.CreateSequence).Name.String()
 
 		ts := hlc.Timestamp{WallTime: nanos}
-		priv := descpb.NewBasePrivilegeDescriptor(security.AdminRoleName())
+		priv := catpb.NewBasePrivilegeDescriptor(security.AdminRoleName())
 		desc, err := sql.NewSequenceTableDesc(
 			ctx,
 			nil, /* planner */

diff --git a/pkg/ccl/streamingccl/streamclient/BUILD.bazel b/pkg/ccl/streamingccl/streamclient/BUILD.bazel
@@ -19,6 +19,7 @@ go_library(
         "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/catalogkeys",
+        "//pkg/sql/catalog/catpb",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/rowenc",

diff --git a/pkg/ccl/streamingccl/streamclient/random_stream_client.go b/pkg/ccl/streamingccl/streamclient/random_stream_client.go
@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
@@ -273,7 +274,7 @@ func (m *randomStreamClient) getDescriptorAndNamespaceKVForTableID(
 		IngestionDatabaseID,
 		tableID,
 		fmt.Sprintf(RandomStreamSchemaPlaceholder, tableName),
-		descpb.NewBasePrivilegeDescriptor(security.RootUserName()),
+		catpb.NewBasePrivilegeDescriptor(security.RootUserName()),
 	)
 	if err != nil {
 		return nil, nil, err