Merge pull request #62200 from arulajmani/backport21.1-61585

release-21.1: sql: stop duplicating regions on the database descriptor

pkg/BUILD.bazel

@@ -152,6 +152,7 @@ ALL_TESTS = [
     "//pkg/sql/catalog/descs:descs_test",
     "//pkg/sql/catalog/hydratedtables:hydratedtables_test",
     "//pkg/sql/catalog/lease:lease_test",
+    "//pkg/sql/catalog/multiregion:multiregion_test",
     "//pkg/sql/catalog/schemadesc:schemadesc_test",
     "//pkg/sql/catalog/schemaexpr:schemaexpr_test",
     "//pkg/sql/catalog/systemschema:systemschema_test",

pkg/ccl/backupccl/BUILD.bazel

@@ -57,6 +57,7 @@ go_library(
         "//pkg/sql/catalog/dbdesc",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/descs",
+        "//pkg/sql/catalog/multiregion",
         "//pkg/sql/catalog/schemadesc",
         "//pkg/sql/catalog/schemaexpr",
         "//pkg/sql/catalog/systemschema",

pkg/ccl/backupccl/restore_job.go

@@ -32,6 +32,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
@@ -376,7 +377,7 @@ func WriteDescriptors(
 			if err != nil {
 				return err
 			}
-			if dbDesc.GetRegionConfig() != nil {
+			if dbDesc.IsMultiRegion() {
 				if table.GetLocalityConfig() == nil {
 					table.(*tabledesc.Mutable).SetTableLocalityRegionalByTable(tree.PrimaryRegionNotSpecifiedName)
 				}
@@ -1086,7 +1087,7 @@ func createImportingDescriptors(
 				// as the system tables are restored.
 				mrEnumsFound := make(map[descpb.ID]descpb.ID)
 				for _, t := range typesByID {
-					typeDesc := t.TypeDesc()
+					typeDesc := typedesc.NewBuilder(t.TypeDesc()).BuildImmutableType()
 					if typeDesc.GetKind() == descpb.TypeDescriptor_MULTIREGION_ENUM {
 						// Check to see if we've found more than one multi-region enum on any
 						// given database.
@@ -1111,10 +1112,20 @@ func createImportingDescriptors(
 							// configuration.
 							if details.DescriptorCoverage != tree.AllDescriptors {
 								log.Infof(ctx, "restoring zone configuration for database %d", desc.ID)
+								regionNames, err := typeDesc.RegionNames()
+								if err != nil {
+									return err
+								}
+								regionConfig := multiregion.MakeRegionConfig(
+									regionNames,
+									desc.RegionConfig.PrimaryRegion,
+									desc.RegionConfig.SurvivalGoal,
+									desc.RegionConfig.RegionEnumID,
+								)
 								if err := sql.ApplyZoneConfigFromDatabaseRegionConfig(
 									ctx,
 									desc.GetID(),
-									*desc.RegionConfig,
+									regionConfig,
 									txn,
 									p.ExecCfg(),
 								); err != nil {
@@ -1241,11 +1252,15 @@ func createImportingDescriptors(
 								return err
 							}
 
+							regionConfig, err := sql.SynthesizeRegionConfig(ctx, txn, desc.ID, descsCol)
+							if err != nil {
+								return err
+							}
 							if err := sql.ApplyZoneConfigForMultiRegionTable(
 								ctx,
 								txn,
 								p.ExecCfg(),
-								*desc.RegionConfig,
+								regionConfig,
 								mutTable,
 								sql.ApplyZoneConfigForMultiRegionTableOptionTableAndIndexes,
 							); err != nil {

pkg/ccl/backupccl/restore_planning.go

@@ -597,7 +597,7 @@ func allocateDescriptorRewrites(
 				// want to modify the table so that it can exist in the multi-region
 				// database.
 				// https://github.com/cockroachdb/cockroach/issues/59804
-				if parentDB.GetRegionConfig() != nil {
+				if parentDB.IsMultiRegion() {
 					return pgerror.Newf(pgcode.FeatureNotSupported,
 						"cannot restore individual table %d into multi-region database %d",
 						table.GetID(),

pkg/ccl/importccl/import_table_creation.go

@@ -217,10 +217,9 @@ func (so *importSequenceOperators) GetSerialSequenceNameFromColumn(
 }
 
 // CurrentDatabaseRegionConfig is part of the tree.EvalDatabase interface.
-func (so *importSequenceOperators) CurrentDatabaseRegionConfig() (
-	tree.DatabaseRegionConfig,
-	error,
-) {
+func (so *importSequenceOperators) CurrentDatabaseRegionConfig(
+	_ context.Context,
+) (tree.DatabaseRegionConfig, error) {
 	return nil, errors.WithStack(errSequenceOperators)
 }
 

pkg/ccl/logictestccl/testdata/logic_test/multi_region

@@ -28,7 +28,7 @@ CREATE DATABASE region_test_db PRIMARY REGION "ap-southeast-2" SURVIVE ZONE FAIL
 statement ok
 CREATE DATABASE multi_region_test_db PRIMARY REGION "ca-central-1" REGIONS "ap-southeast-2", "us-east-1" SURVIVE REGION FAILURE
 
-statement error cannot DROP REGION on database multi_region_test_db as databases with SURVIVE REGION FAILURE must have at least 3 regions\nHINT: you must first add another region, or configure the database to SURVIVE ZONE FAILURE using ALTER DATABASE multi_region_test_db SURVIVE ZONE FAILURE
+statement error at least 3 regions are required for surviving a region failure\nHINT: you must add additional regions to the database or change the survivability goal
 ALTER DATABASE multi_region_test_db DROP REGION "ap-southeast-2"
 
 statement ok

pkg/ccl/multiregionccl/BUILD.bazel

@@ -11,6 +11,7 @@ go_library(
         "//pkg/sql",
         "//pkg/sql/catalog/catalogkv",
         "//pkg/sql/catalog/descpb",
+        "//pkg/sql/catalog/multiregion",
         "//pkg/sql/catalog/typedesc",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",

pkg/ccl/multiregionccl/multiregion.go

@@ -17,28 +17,29 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 )
 
 func init() {
-	sql.CreateRegionConfigCCL = createRegionConfig
+	sql.InitializeMultiRegionMetadataCCL = initializeMultiRegionMetadata
 	sql.GetMultiRegionEnumAddValuePlacementCCL = getMultiRegionEnumAddValuePlacement
 }
 
-func createRegionConfig(
+func initializeMultiRegionMetadata(
 	ctx context.Context,
 	evalCtx *tree.EvalContext,
 	execCfg *sql.ExecutorConfig,
 	liveRegions sql.LiveClusterRegions,
-	survivalGoal tree.SurvivalGoal,
-	primaryRegion tree.Name,
+	goal tree.SurvivalGoal,
+	primaryRegion descpb.RegionName,
 	regions []tree.Name,
-) (descpb.DatabaseDescriptor_RegionConfig, error) {
+) (*multiregion.RegionConfig, error) {
 	if err := checkClusterSupportsMultiRegion(evalCtx); err != nil {
-		return descpb.DatabaseDescriptor_RegionConfig{}, err
+		return nil, err
 	}
 
 	if err := utilccl.CheckEnterpriseEnabled(
@@ -47,78 +48,71 @@ func createRegionConfig(
 		execCfg.Organization(),
 		"multi-region features",
 	); err != nil {
-		return descpb.DatabaseDescriptor_RegionConfig{}, err
+		return nil, err
 	}
 
-	var regionConfig descpb.DatabaseDescriptor_RegionConfig
-	var err error
-	regionConfig.SurvivalGoal, err = sql.TranslateSurvivalGoal(survivalGoal)
+	survivalGoal, err := sql.TranslateSurvivalGoal(goal)
 	if err != nil {
-		return descpb.DatabaseDescriptor_RegionConfig{}, err
+		return nil, err
 	}
 
-	regionConfig.PrimaryRegion = descpb.RegionName(primaryRegion)
-	if regionConfig.PrimaryRegion != descpb.RegionName(tree.PrimaryRegionNotSpecifiedName) {
-		if err := sql.CheckLiveClusterRegion(liveRegions, regionConfig.PrimaryRegion); err != nil {
-			return descpb.DatabaseDescriptor_RegionConfig{}, err
+	if primaryRegion != descpb.RegionName(tree.PrimaryRegionNotSpecifiedName) {
+		if err := sql.CheckClusterRegionIsLive(liveRegions, primaryRegion); err != nil {
+			return nil, err
 		}
 	}
+	regionNames := make(descpb.RegionNames, 0, len(regions)+1)
+	seenRegions := make(map[descpb.RegionName]struct{}, len(regions)+1)
 	if len(regions) > 0 {
-		if regionConfig.PrimaryRegion == descpb.RegionName(tree.PrimaryRegionNotSpecifiedName) {
-			return descpb.DatabaseDescriptor_RegionConfig{}, pgerror.Newf(
+		if primaryRegion == descpb.RegionName(tree.PrimaryRegionNotSpecifiedName) {
+			return nil, pgerror.Newf(
 				pgcode.InvalidDatabaseDefinition,
 				"PRIMARY REGION must be specified if REGIONS are specified",
 			)
 		}
-		regionConfig.Regions = make([]descpb.DatabaseDescriptor_RegionConfig_Region, 0, len(regions)+1)
-		seenRegions := make(map[descpb.RegionName]struct{}, len(regions)+1)
 		for _, r := range regions {
 			region := descpb.RegionName(r)
-			if err := sql.CheckLiveClusterRegion(liveRegions, region); err != nil {
-				return descpb.DatabaseDescriptor_RegionConfig{}, err
+			if err := sql.CheckClusterRegionIsLive(liveRegions, region); err != nil {
+				return nil, err
 			}
 
 			if _, ok := seenRegions[region]; ok {
-				return descpb.DatabaseDescriptor_RegionConfig{}, pgerror.Newf(
+				return nil, pgerror.Newf(
 					pgcode.InvalidName,
 					"region %q defined multiple times",
 					region,
 				)
 			}
 			seenRegions[region] = struct{}{}
-			regionConfig.Regions = append(
-				regionConfig.Regions,
-				descpb.DatabaseDescriptor_RegionConfig_Region{
-					Name: region,
-				},
-			)
-		}
-		// If PRIMARY REGION is not in REGIONS, add it implicitly.
-		if _, ok := seenRegions[regionConfig.PrimaryRegion]; !ok {
-			regionConfig.Regions = append(
-				regionConfig.Regions,
-				descpb.DatabaseDescriptor_RegionConfig_Region{
-					Name: regionConfig.PrimaryRegion,
-				},
-			)
-		}
-		sort.SliceStable(regionConfig.Regions, func(i, j int) bool {
-			return regionConfig.Regions[i].Name < regionConfig.Regions[j].Name
-		})
-	} else {
-		regionConfig.Regions = []descpb.DatabaseDescriptor_RegionConfig_Region{
-			{Name: regionConfig.PrimaryRegion},
+			regionNames = append(regionNames, region)
 		}
 	}
+	// If PRIMARY REGION is not in REGIONS, add it implicitly.
+	if _, ok := seenRegions[primaryRegion]; !ok {
+		regionNames = append(regionNames, primaryRegion)
+	}
+
+	sort.SliceStable(regionNames, func(i, j int) bool {
+		return regionNames[i] < regionNames[j]
+	})
 
 	// Generate a unique ID for the multi-region enum type descriptor here as
 	// well.
-	id, err := catalogkv.GenerateUniqueDescID(ctx, execCfg.DB, execCfg.Codec)
+	regionEnumID, err := catalogkv.GenerateUniqueDescID(ctx, execCfg.DB, execCfg.Codec)
 	if err != nil {
-		return descpb.DatabaseDescriptor_RegionConfig{}, err
+		return nil, err
+	}
+	regionConfig := multiregion.MakeRegionConfig(
+		regionNames,
+		primaryRegion,
+		survivalGoal,
+		regionEnumID,
+	)
+	if err := multiregion.ValidateRegionConfig(regionConfig); err != nil {
+		return nil, err
 	}
-	regionConfig.RegionEnumID = id
-	return regionConfig, nil
+
+	return &regionConfig, nil
 }
 
 func checkClusterSupportsMultiRegion(evalCtx *tree.EvalContext) error {

pkg/sql/BUILD.bazel

@@ -528,6 +528,7 @@ go_test(
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/descs",
         "//pkg/sql/catalog/lease",
+        "//pkg/sql/catalog/multiregion",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/catalog/typedesc",
         "//pkg/sql/distsql",