sql: move ValidateNoRepeatKeysInZone to zonepb pkg

This patch refactors the `ValidateNoRepeatKeysInZone` function
to a package accesible to the DSC.

Informs: cockroachdb#117574
Release note: None

pkg/config/zonepb/BUILD.bazel

@@ -15,6 +15,8 @@ go_library(
         "//pkg/clusterversion",
         "//pkg/keys",
         "//pkg/roachpb",
+        "//pkg/sql/pgwire/pgcode",
+        "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/sem/tree",
         "//pkg/util/envutil",
         "//pkg/util/log",

pkg/config/zonepb/zone.go

@@ -21,6 +21,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -1313,3 +1315,62 @@ func init() {
 		))
 	}
 }
+
+// ValidateNoRepeatKeysInZone checks that there are not duplicated values for a
+// particular constraint. For example, constraints
+// [+region=us-east1,+region=us-east2] will be rejected. Additionally, invalid
+// constraints such as [+region=us-east1, -region=us-east1] will also be
+// rejected.
+func ValidateNoRepeatKeysInZone(zone *ZoneConfig) error {
+	for _, leasePreference := range zone.LeasePreferences {
+		if err := validateNoRepeatKeysInConstraints(leasePreference.Constraints); err != nil {
+			return err
+		}
+	}
+	if err := validateNoRepeatKeysInConjunction(zone.Constraints); err != nil {
+		return err
+	}
+	return validateNoRepeatKeysInConjunction(zone.VoterConstraints)
+}
+
+func validateNoRepeatKeysInConjunction(conjunctions []ConstraintsConjunction) error {
+	for _, constraints := range conjunctions {
+		if err := validateNoRepeatKeysInConstraints(constraints.Constraints); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func validateNoRepeatKeysInConstraints(constraints []Constraint) error {
+	// Because we expect to have a small number of constraints, a nested
+	// loop is probably better than allocating a map.
+	for i, curr := range constraints {
+		for _, other := range constraints[i+1:] {
+			// We don't want to enter the other validation logic if both of the constraints
+			// are attributes, due to the keys being the same for attributes.
+			if curr.Key == "" && other.Key == "" {
+				if curr.Value == other.Value {
+					return pgerror.Newf(pgcode.CheckViolation,
+						"incompatible zone constraints: %q and %q", curr, other)
+				}
+			} else {
+				if curr.Type == Constraint_REQUIRED {
+					if other.Type == Constraint_REQUIRED && other.Key == curr.Key ||
+						other.Type == Constraint_PROHIBITED && other.Key == curr.Key && other.Value == curr.Value {
+						return pgerror.Newf(pgcode.CheckViolation,
+							"incompatible zone constraints: %q and %q", curr, other)
+					}
+				} else if curr.Type == Constraint_PROHIBITED {
+					// If we have a -k=v pair, verify that there are not any
+					// +k=v pairs in the constraints.
+					if other.Type == Constraint_REQUIRED && other.Key == curr.Key && other.Value == curr.Value {
+						return pgerror.Newf(pgcode.CheckViolation,
+							"incompatible zone constraints: %q and %q", curr, other)
+					}
+				}
+			}
+		}
+	}
+	return nil
+}

pkg/sql/alter_database.go

@@ -2413,7 +2413,7 @@ func (n *alterDatabaseSetZoneConfigExtensionNode) startExec(params runParams) er
 		}
 
 		// Validate that there are no conflicts in the zone setup.
-		if err := validateNoRepeatKeysInZone(newZone); err != nil {
+		if err := zonepb.ValidateNoRepeatKeysInZone(newZone); err != nil {
 			return err
 		}
 

pkg/sql/region_util.go

@@ -915,7 +915,7 @@ func generateAndValidateZoneConfigForMultiRegionDatabase(
 		return zonepb.ZoneConfig{}, pgerror.Wrap(err, pgcode.CheckViolation, "could not validate zone config")
 	}
 
-	if err := validateNoRepeatKeysInZone(&dbZoneConfig); err != nil {
+	if err := zonepb.ValidateNoRepeatKeysInZone(&dbZoneConfig); err != nil {
 		return zonepb.ZoneConfig{}, err
 	}
 

pkg/sql/set_zone_config.go

@@ -614,7 +614,7 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
 			}
 
 			// Validate that there are no conflicts in the zone setup.
-			if err := validateNoRepeatKeysInZone(&newZone); err != nil {
+			if err := zonepb.ValidateNoRepeatKeysInZone(&newZone); err != nil {
 				return err
 			}
 
@@ -782,64 +782,6 @@ func (n *setZoneConfigNode) FastPathResults() (int, bool) { return n.run.numAffe
 type nodeGetter func(context.Context, *serverpb.NodesRequest) (*serverpb.NodesResponse, error)
 type regionsGetter func(context.Context) (*serverpb.RegionsResponse, error)
 
-// Check that there are not duplicated values for a particular
-// constraint. For example, constraints [+region=us-east1,+region=us-east2]
-// will be rejected. Additionally, invalid constraints such as
-// [+region=us-east1, -region=us-east1] will also be rejected.
-func validateNoRepeatKeysInZone(zone *zonepb.ZoneConfig) error {
-	for _, leasePreference := range zone.LeasePreferences {
-		if err := validateNoRepeatKeysInConstraints(leasePreference.Constraints); err != nil {
-			return err
-		}
-	}
-	if err := validateNoRepeatKeysInConjunction(zone.Constraints); err != nil {
-		return err
-	}
-	return validateNoRepeatKeysInConjunction(zone.VoterConstraints)
-}
-
-func validateNoRepeatKeysInConjunction(conjunctions []zonepb.ConstraintsConjunction) error {
-	for _, constraints := range conjunctions {
-		if err := validateNoRepeatKeysInConstraints(constraints.Constraints); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-func validateNoRepeatKeysInConstraints(constraints []zonepb.Constraint) error {
-	// Because we expect to have a small number of constraints, a nested
-	// loop is probably better than allocating a map.
-	for i, curr := range constraints {
-		for _, other := range constraints[i+1:] {
-			// We don't want to enter the other validation logic if both of the constraints
-			// are attributes, due to the keys being the same for attributes.
-			if curr.Key == "" && other.Key == "" {
-				if curr.Value == other.Value {
-					return pgerror.Newf(pgcode.CheckViolation,
-						"incompatible zone constraints: %q and %q", curr, other)
-				}
-			} else {
-				if curr.Type == zonepb.Constraint_REQUIRED {
-					if other.Type == zonepb.Constraint_REQUIRED && other.Key == curr.Key ||
-						other.Type == zonepb.Constraint_PROHIBITED && other.Key == curr.Key && other.Value == curr.Value {
-						return pgerror.Newf(pgcode.CheckViolation,
-							"incompatible zone constraints: %q and %q", curr, other)
-					}
-				} else if curr.Type == zonepb.Constraint_PROHIBITED {
-					// If we have a -k=v pair, verify that there are not any
-					// +k=v pairs in the constraints.
-					if other.Type == zonepb.Constraint_REQUIRED && other.Key == curr.Key && other.Value == curr.Value {
-						return pgerror.Newf(pgcode.CheckViolation,
-							"incompatible zone constraints: %q and %q", curr, other)
-					}
-				}
-			}
-		}
-	}
-	return nil
-}
-
 // accumulateNewUniqueConstraints returns a list of unique constraints in the
 // given newZone config proto that are not in the currentZone
 func accumulateNewUniqueConstraints(currentZone, newZone *zonepb.ZoneConfig) []zonepb.Constraint {

pkg/sql/set_zone_config_test.go

@@ -58,7 +58,7 @@ func TestValidateNoRepeatKeysInZone(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		err = validateNoRepeatKeysInZone(&zone)
+		err = zonepb.ValidateNoRepeatKeysInZone(&zone)
 		if err != nil && expectSuccess {
 			t.Errorf("expected success for %q; got %v", constraint, err)
 		} else if err == nil && !expectSuccess {