Merge pull request #65833 from ajstorm/backport21.1-63665

release-21.1: sql: Don't clear zone configs on indexes during a RBR transition

pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs

@@ -869,11 +869,41 @@ SET override_multi_region_zone_config = false
 statement error attempting to update zone config which contains an extra zone configuration for index tbl2@tbl2_i_idx with field num_replicas populated
 ALTER TABLE tbl2 SET LOCALITY GLOBAL
 
+statement ok
+ALTER INDEX tbl2@tbl2_i_idx CONFIGURE ZONE USING gc.ttlseconds=10
+
+query TT
+SHOW ZONE CONFIGURATION FOR INDEX tbl2@tbl2_i_idx
+----
+INDEX tbl2@tbl2_i_idx  ALTER INDEX tbl2@tbl2_i_idx CONFIGURE ZONE USING
+                       range_min_bytes = 134217728,
+                       range_max_bytes = 536870912,
+                       gc.ttlseconds = 10,
+                       num_replicas = 10,
+                       num_voters = 3,
+                       constraints = '{+region=us-east-1: 1}',
+                       voter_constraints = '[+region=us-east-1]',
+                       lease_preferences = '[[+region=us-east-1]]'
+
 statement ok
 SET override_multi_region_zone_config = true;
 ALTER TABLE tbl2 SET LOCALITY GLOBAL;
 SET override_multi_region_zone_config = false
 
+# Validate that we don't overwrite gc.ttlseconds
+query TT
+SHOW ZONE CONFIGURATION FOR INDEX tbl2@tbl2_i_idx
+----
+INDEX tbl2@tbl2_i_idx  ALTER INDEX tbl2@tbl2_i_idx CONFIGURE ZONE USING
+                       range_min_bytes = 134217728,
+                       range_max_bytes = 536870912,
+                       gc.ttlseconds = 10,
+                       num_replicas = 3,
+                       num_voters = 3,
+                       constraints = '{+region=us-east-1: 1}',
+                       voter_constraints = '[+region=us-east-1]',
+                       lease_preferences = '[[+region=us-east-1]]'
+
 statement ok
 ALTER TABLE tbl2 SET LOCALITY REGIONAL BY ROW
 

pkg/config/zonepb/zone.go

@@ -849,6 +849,23 @@ func (z *ZoneConfig) DiffWithZone(
 	return true, DiffWithZoneMismatch{}, nil
 }
 
+// ClearFieldsOfAllSubzones uses the supplied fieldList and clears those fields
+// from all of the zone config's subzones.
+func (z *ZoneConfig) ClearFieldsOfAllSubzones(fieldList []tree.Name) {
+	newSubzones := z.Subzones[:0]
+	emptyZone := NewZoneConfig()
+	for _, sz := range z.Subzones {
+		// By copying from an empty zone, we'll end up clearing out all of the
+		// fields in the fieldList.
+		sz.Config.CopyFromZone(*emptyZone, fieldList)
+		// If we haven't emptied out the subzone, append it to the new slice.
+		if !sz.Config.Equal(emptyZone) {
+			newSubzones = append(newSubzones, sz)
+		}
+	}
+	z.Subzones = newSubzones
+}
+
 // StoreSatisfiesConstraint checks whether a store satisfies the given constraint.
 // If the constraint is of the PROHIBITED type, satisfying it means the store
 // not matching the constraint's spec.

pkg/sql/region_util.go

@@ -440,11 +440,10 @@ func dropZoneConfigsForMultiRegionIndexes(
 		regionConfig multiregion.RegionConfig,
 		table catalog.TableDescriptor,
 	) (hasNewSubzones bool, newZoneConfig zonepb.ZoneConfig, err error) {
-		for _, indexID := range indexIDs {
-			for _, region := range regionConfig.Regions() {
-				zoneConfig.DeleteSubzone(uint32(indexID), string(region))
-			}
-		}
+		// Clear all multi-region fields of the subzones. If this leaves them
+		// empty, they will automatically be removed.
+		zoneConfig.ClearFieldsOfAllSubzones(zonepb.MultiRegionZoneConfigFields)
+
 		// Strip placeholder status and spans if there are no more subzones.
 		if len(zoneConfig.Subzones) == 0 && zoneConfig.IsSubzonePlaceholder() {
 			zoneConfig.NumReplicas = nil
@@ -516,15 +515,7 @@ var ApplyZoneConfigForMultiRegionTableOptionTableAndIndexes = func(
 	// have to override to perform the ALTER, we want to wipe out the index
 	// zone config so that the user won't have to override again the next time
 	// the want to ALTER the table locality.
-	newSubzones := zc.Subzones[:0]
-	for _, sz := range zc.Subzones {
-		sz.Config.CopyFromZone(zonepb.ZoneConfig{}, zonepb.MultiRegionZoneConfigFields)
-		// If we haven't emptied out the subzone, append it to the new slice.
-		if !sz.Config.Equal(zonepb.ZoneConfig{}) && !sz.Config.Equal(zonepb.NewZoneConfig()) {
-			newSubzones = append(newSubzones, sz)
-		}
-	}
-	zc.Subzones = newSubzones
+	zc.ClearFieldsOfAllSubzones(zonepb.MultiRegionZoneConfigFields)
 
 	zc.CopyFromZone(*localityZoneConfig, zonepb.MultiRegionZoneConfigFields)
 

pkg/sql/schema_changer.go

@@ -2525,19 +2525,21 @@ func (sc *SchemaChanger) applyZoneConfigChangeForMutation(
 			if err != nil {
 				return err
 			}
-			return ApplyZoneConfigForMultiRegionTable(
+			if err := ApplyZoneConfigForMultiRegionTable(
 				ctx,
 				txn,
 				sc.execCfg,
 				regionConfig,
 				tableDesc,
 				opts...,
-			)
+			); err != nil {
+				return err
+			}
 		}
 
-		// For the plain ALTER PRIMARY KEY case, copy the zone configs over
-		// for any new indexes.
-		// Note this is done even for isDone = true, though not strictly necessary.
+		// In all cases, we now copy the zone configs over for any new indexes.
+		// Note this is done even for isDone = true, though not strictly
+		// necessary.
 		return maybeUpdateZoneConfigsForPKChange(
 			ctx, txn, sc.execCfg, tableDesc, pkSwap,
 		)