sql: gate global_reads zone attribute on cluster version and enterprise license

pkg/ccl/logictestccl/testdata/logic_test/multi_region_mixed_20.2_21.1

@@ -26,3 +26,21 @@ CREATE TABLE t()
 
 statement error cannot alter a table's LOCALITY if its database is not multi-region enabled
 ALTER TABLE t SET LOCALITY REGIONAL BY ROW
+
+statement error pgcode 0A000 global_reads cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING global_reads = true
+
+statement error pgcode 0A000 global_reads cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING global_reads = false
+
+statement error pgcode 0A000 num_voters cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING num_voters = 3
+
+statement error pgcode 0A000 num_voters cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING num_voters = 0
+
+statement error pgcode 0A000 voter_constraints cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING voter_constraints = '[+region=us-east-1]'
+
+statement error pgcode 0A000 voter_constraints cannot be used until cluster version is finalized
+ALTER TABLE t CONFIGURE ZONE USING voter_constraints = ''

pkg/ccl/logictestccl/testdata/logic_test/zone

@@ -1126,3 +1126,32 @@ ALTER PARTITION p3 OF INDEX test.public.t@i2 CONFIGURE ZONE USING
   gc.ttlseconds = 15411;
 ALTER PARTITION p4 OF INDEX test.public.t@i2 CONFIGURE ZONE USING
   gc.ttlseconds = 15418
+
+# Test that the global_reads attribute can be set with an enterprise license.
+# This same test fails in pkg/sql/logictest/testdata/logic_test/zone_config.
+statement ok
+CREATE TABLE global (id INT PRIMARY KEY)
+
+statement ok
+ALTER TABLE global CONFIGURE ZONE USING global_reads = true
+
+# This should reflect in the metrics.
+query T
+SELECT feature_name FROM crdb_internal.feature_usage
+WHERE feature_name IN (
+  'sql.schema.zone_config.table.global_reads'
+) AND usage_count > 0 ORDER BY feature_name
+----
+sql.schema.zone_config.table.global_reads
+
+query TT
+SHOW ZONE CONFIGURATION FOR TABLE global
+----
+TABLE global  ALTER TABLE global CONFIGURE ZONE USING
+              range_min_bytes = 134217728,
+              range_max_bytes = 536870912,
+              gc.ttlseconds = 90000,
+              global_reads = true,
+              num_replicas = 7,
+              constraints = '[]',
+              lease_preferences = '[]'

pkg/ccl/multiregionccl/multiregion_test.go

@@ -117,3 +117,46 @@ func TestMultiRegionAfterEnterpriseDisabled(t *testing.T) {
 		}
 	})
 }
+
+func TestGlobalReadsAfterEnterpriseDisabled(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+	defer utilccl.TestingEnableEnterprise()()
+
+	_, sqlDB, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
+		t, 1 /* numServers */, base.TestingKnobs{}, nil, /* baseDir */
+	)
+	defer cleanup()
+
+	for _, setupQuery := range []string{
+		`CREATE DATABASE test`,
+		`USE test`,
+		`CREATE TABLE t1 ()`,
+		`CREATE TABLE t2 ()`,
+	} {
+		_, err := sqlDB.Exec(setupQuery)
+		require.NoError(t, err)
+	}
+
+	// Can set global_reads with enterprise license enabled.
+	_, err := sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = true`)
+	require.NoError(t, err)
+
+	_, err = sqlDB.Exec(`ALTER TABLE t2 CONFIGURE ZONE USING global_reads = true`)
+	require.NoError(t, err)
+
+	// Can unset global_reads with enterprise license enabled.
+	_, err = sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = false`)
+	require.NoError(t, err)
+
+	defer utilccl.TestingDisableEnterprise()()
+
+	// Cannot set global_reads with enterprise license disabled.
+	_, err = sqlDB.Exec(`ALTER TABLE t1 CONFIGURE ZONE USING global_reads = true`)
+	require.Error(t, err)
+	require.Regexp(t, "use of global_reads requires an enterprise license", err)
+
+	// Can unset global_reads with enterprise license disabled.
+	_, err = sqlDB.Exec(`ALTER TABLE t2 CONFIGURE ZONE USING global_reads = false`)
+	require.NoError(t, err)
+}

pkg/sql/logictest/testdata/logic_test/zone_config

@@ -75,7 +75,6 @@ ALTER TABLE a CONFIGURE ZONE USING
   range_min_bytes = 200000 + 1,
   range_max_bytes = 300000 + 1,
   gc.ttlseconds = 3000 + 600,
-  global_reads = true,
   num_replicas = floor(1.2)::int,
   constraints = '[+region=test]',
   lease_preferences = '[[+region=test]]'
@@ -87,14 +86,12 @@ WHERE feature_name IN (
   'sql.schema.zone_config.table.range_min_bytes',
   'sql.schema.zone_config.table.range_max_bytes',
   'sql.schema.zone_config.table.gc.ttlseconds',
-  'sql.schema.zone_config.table.global_reads',
   'sql.schema.zone_config.table.num_replicas',
   'sql.schema.zone_config.table.constraints'
 ) AND usage_count > 0 ORDER BY feature_name
 ----
 sql.schema.zone_config.table.constraints
 sql.schema.zone_config.table.gc.ttlseconds
-sql.schema.zone_config.table.global_reads
 sql.schema.zone_config.table.num_replicas
 sql.schema.zone_config.table.range_max_bytes
 sql.schema.zone_config.table.range_min_bytes
@@ -106,7 +103,6 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
     range_min_bytes = 200001,
     range_max_bytes = 300001,
     gc.ttlseconds = 3600,
-    global_reads = true,
     num_replicas = 1,
     constraints = '[+region=test]',
     lease_preferences = '[[+region=test]]'
@@ -122,7 +118,6 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
     range_min_bytes = 200001,
     range_max_bytes = 400000,
     gc.ttlseconds = 3600,
-    global_reads = true,
     num_replicas = 1,
     constraints = '[+region=test]',
     lease_preferences = '[[+region=test]]'
@@ -159,7 +154,6 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
     range_min_bytes = 200001,
     range_max_bytes = 400000,
     gc.ttlseconds = 3600,
-    global_reads = true,
     num_replicas = 1,
     constraints = '[+region=test]',
     lease_preferences = '[[+region=test]]'
@@ -266,6 +260,10 @@ SELECT zone_id, raw_config_sql FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
     voter_constraints = '{+region=test: 1}',
     lease_preferences = '[]'
 
+# Check that global_reads cannot be set without a CCL binary and enterprise license.
+statement error OSS binaries do not include enterprise features
+ALTER TABLE a CONFIGURE ZONE USING global_reads = true
+
 # Check entities for which we can set zone configs.
 subtest test_entity_validity
 

pkg/sql/set_zone_config.go

@@ -16,6 +16,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/config"
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
@@ -63,37 +64,86 @@ type setZoneConfigNode struct {
 var supportedZoneConfigOptions = map[tree.Name]struct {
 	requiredType *types.T
 	setter       func(*zonepb.ZoneConfig, tree.Datum)
+	checkAllowed func(context.Context, *ExecutorConfig, tree.Datum) error // optional
 }{
-	"range_min_bytes": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMinBytes = proto.Int64(int64(tree.MustBeDInt(d))) }},
-	"range_max_bytes": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMaxBytes = proto.Int64(int64(tree.MustBeDInt(d))) }},
-	"global_reads":    {types.Bool, func(c *zonepb.ZoneConfig, d tree.Datum) { c.GlobalReads = proto.Bool(bool(tree.MustBeDBool(d))) }},
-	"num_replicas":    {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumReplicas = proto.Int32(int32(tree.MustBeDInt(d))) }},
-	"num_voters":      {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumVoters = proto.Int32(int32(tree.MustBeDInt(d))) }},
-	"gc.ttlseconds": {types.Int, func(c *zonepb.ZoneConfig, d tree.Datum) {
-		c.GC = &zonepb.GCPolicy{TTLSeconds: int32(tree.MustBeDInt(d))}
-	}},
-	"constraints": {types.String, func(c *zonepb.ZoneConfig, d tree.Datum) {
-		constraintsList := zonepb.ConstraintsList{
-			Constraints: c.Constraints,
-			Inherited:   c.InheritedConstraints,
-		}
-		loadYAML(&constraintsList, string(tree.MustBeDString(d)))
-		c.Constraints = constraintsList.Constraints
-		c.InheritedConstraints = false
-	}},
-	"voter_constraints": {types.String, func(c *zonepb.ZoneConfig, d tree.Datum) {
-		voterConstraintsList := zonepb.ConstraintsList{
-			Constraints: c.VoterConstraints,
-			Inherited:   c.InheritedVoterConstraints,
-		}
-		loadYAML(&voterConstraintsList, string(tree.MustBeDString(d)))
-		c.VoterConstraints = voterConstraintsList.Constraints
-		c.InheritedVoterConstraints = false
-	}},
-	"lease_preferences": {types.String, func(c *zonepb.ZoneConfig, d tree.Datum) {
-		loadYAML(&c.LeasePreferences, string(tree.MustBeDString(d)))
-		c.InheritedLeasePreferences = false
-	}},
+	"range_min_bytes": {
+		requiredType: types.Int,
+		setter:       func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMinBytes = proto.Int64(int64(tree.MustBeDInt(d))) },
+	},
+	"range_max_bytes": {
+		requiredType: types.Int,
+		setter:       func(c *zonepb.ZoneConfig, d tree.Datum) { c.RangeMaxBytes = proto.Int64(int64(tree.MustBeDInt(d))) },
+	},
+	"global_reads": {
+		requiredType: types.Bool,
+		setter:       func(c *zonepb.ZoneConfig, d tree.Datum) { c.GlobalReads = proto.Bool(bool(tree.MustBeDBool(d))) },
+		checkAllowed: func(ctx context.Context, execCfg *ExecutorConfig, d tree.Datum) error {
+			if err := checkVersionActive(ctx, execCfg, clusterversion.NonVotingReplicas, "global_reads"); err != nil {
+				return err
+			}
+			if !tree.MustBeDBool(d) {
+				// Always allow the value to be unset.
+				return nil
+			}
+			return base.CheckEnterpriseEnabled(
+				execCfg.Settings,
+				execCfg.ClusterID(),
+				execCfg.Organization(),
+				"global_reads",
+			)
+		},
+	},
+	"num_replicas": {
+		requiredType: types.Int,
+		setter:       func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumReplicas = proto.Int32(int32(tree.MustBeDInt(d))) },
+	},
+	"num_voters": {
+		requiredType: types.Int,
+		setter:       func(c *zonepb.ZoneConfig, d tree.Datum) { c.NumVoters = proto.Int32(int32(tree.MustBeDInt(d))) },
+		checkAllowed: func(ctx context.Context, execCfg *ExecutorConfig, _ tree.Datum) error {
+			return checkVersionActive(ctx, execCfg, clusterversion.NonVotingReplicas, "num_voters")
+		},
+	},
+	"gc.ttlseconds": {
+		requiredType: types.Int,
+		setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
+			c.GC = &zonepb.GCPolicy{TTLSeconds: int32(tree.MustBeDInt(d))}
+		},
+	},
+	"constraints": {
+		requiredType: types.String,
+		setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
+			constraintsList := zonepb.ConstraintsList{
+				Constraints: c.Constraints,
+				Inherited:   c.InheritedConstraints,
+			}
+			loadYAML(&constraintsList, string(tree.MustBeDString(d)))
+			c.Constraints = constraintsList.Constraints
+			c.InheritedConstraints = false
+		},
+	},
+	"voter_constraints": {
+		requiredType: types.String,
+		setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
+			voterConstraintsList := zonepb.ConstraintsList{
+				Constraints: c.VoterConstraints,
+				Inherited:   c.InheritedVoterConstraints,
+			}
+			loadYAML(&voterConstraintsList, string(tree.MustBeDString(d)))
+			c.VoterConstraints = voterConstraintsList.Constraints
+			c.InheritedVoterConstraints = false
+		},
+		checkAllowed: func(ctx context.Context, execCfg *ExecutorConfig, _ tree.Datum) error {
+			return checkVersionActive(ctx, execCfg, clusterversion.NonVotingReplicas, "voter_constraints")
+		},
+	},
+	"lease_preferences": {
+		requiredType: types.String,
+		setter: func(c *zonepb.ZoneConfig, d tree.Datum) {
+			loadYAML(&c.LeasePreferences, string(tree.MustBeDString(d)))
+			c.InheritedLeasePreferences = false
+		},
+	},
 }
 
 // zoneOptionKeys contains the keys from suportedZoneConfigOptions in
@@ -114,6 +164,16 @@ func loadYAML(dst interface{}, yamlString string) {
 	}
 }
 
+func checkVersionActive(
+	ctx context.Context, execCfg *ExecutorConfig, minVersion clusterversion.Key, option string,
+) error {
+	if !execCfg.Settings.Version.IsActive(ctx, minVersion) {
+		return pgerror.Newf(pgcode.FeatureNotSupported,
+			"%s cannot be used until cluster version is finalized", option)
+	}
+	return nil
+}
+
 func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (planNode, error) {
 	if err := checkSchemaChangeEnabled(
 		ctx,
@@ -182,11 +242,6 @@ func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (pla
 				return nil, pgerror.Newf(pgcode.InvalidParameterValue,
 					"unsupported zone config parameter: %q", tree.ErrString(&opt.Key))
 			}
-			if (opt.Key == "num_voters" || opt.Key == "voter_constraints") &&
-				!p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.NonVotingReplicas) {
-				return nil, pgerror.Newf(pgcode.FeatureNotSupported,
-					"num_voters and voter_constraints cannot be used until cluster version is finalized")
-			}
 			telemetry.Inc(
 				sqltelemetry.SchemaSetZoneConfigCounter(
 					n.ZoneSpecifier.TelemetryName(),
@@ -334,7 +389,13 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
 				return pgerror.Newf(pgcode.InvalidParameterValue,
 					"unsupported NULL value for %q", tree.ErrString(name))
 			}
-			setter := supportedZoneConfigOptions[*name].setter
+			opt := supportedZoneConfigOptions[*name]
+			if opt.checkAllowed != nil {
+				if err := opt.checkAllowed(params.ctx, params.ExecCfg(), datum); err != nil {
+					return err
+				}
+			}
+			setter := opt.setter
 			setters = append(setters, func(c *zonepb.ZoneConfig) { setter(c, datum) })
 			optionsStr = append(optionsStr, fmt.Sprintf("%s = %s", name, datum))
 		}