Skip to content

Commit

Permalink
sql: Improve error message when user's modified MR zone config
Browse files Browse the repository at this point in the history
Previously we were returning an inaccurate error message when a user
modified zone config was present on a multi-region table. This was due
to the fact that we'd key off of num_replicas being set in a placeholder
zone config and think that that was the difference between the two zone
configs. In actuality, the difference was in the subzones.

This commit allows for the reporting of the true difference, the
presence of the index in the subzones.

Release note: None
  • Loading branch information
ajstorm committed Apr 13, 2021
1 parent 996d78f commit a5f2857
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
24 changes: 20 additions & 4 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ SET override_multi_region_zone_config = true;
ALTER INDEX tbl1@tbl1_i_idx CONFIGURE ZONE USING num_replicas=10;
SET override_multi_region_zone_config = false

statement error attempting to update zone configuration for table tbl1 which contains modified field "num_replicas"
statement error attempting to update zone config which contains an extra zone configuration for index tbl1@tbl1_i_idx with field num_replicas populated
ALTER TABLE tbl1 SET LOCALITY GLOBAL

# This statement should wipe out the zone configuration we applied on the
Expand Down Expand Up @@ -816,20 +816,36 @@ CREATE TABLE tbl2 (
pk INT PRIMARY KEY,
i INT,
INDEX(i)
) LOCALITY REGIONAL BY ROW;
) LOCALITY REGIONAL BY ROW

statement ok
SET override_multi_region_zone_config = true;
ALTER INDEX tbl2@tbl2_i_idx CONFIGURE ZONE USING num_replicas=10;
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
SET override_multi_region_zone_config = true;
ALTER TABLE tbl2 SET LOCALITY GLOBAL;
SET override_multi_region_zone_config = false

statement ok
ALTER TABLE tbl2 SET LOCALITY REGIONAL BY ROW

statement ok
SET override_multi_region_zone_config = true;
ALTER TABLE tbl2 CONFIGURE ZONE USING num_replicas=10;
SET override_multi_region_zone_config = false

statement error attempting to update zone configuration for table tbl2 which contains modified field "num_replicas"
ALTER TABLE tbl2 SET LOCALITY GLOBAL

statement ok
SET override_multi_region_zone_config = true;
ALTER TABLE tbl2 SET LOCALITY GLOBAL;
SET override_multi_region_zone_config = false;
SET override_multi_region_zone_config = false

statement ok
ALTER TABLE tbl2 SET LOCALITY REGIONAL BY TABLE IN PRIMARY REGION;
ALTER TABLE tbl2 SET LOCALITY REGIONAL BY TABLE IN PRIMARY REGION
15 changes: 15 additions & 0 deletions pkg/config/zonepb/zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ type DiffWithZoneMismatch struct {
func (z *ZoneConfig) DiffWithZone(
other ZoneConfig, fieldList []tree.Name,
) (bool, DiffWithZoneMismatch, error) {
mismatchingNumReplicas := false
for _, fieldName := range fieldList {
switch fieldName {
case "num_replicas":
Expand All @@ -634,6 +635,13 @@ func (z *ZoneConfig) DiffWithZone(
}
if z.NumReplicas == nil || other.NumReplicas == nil ||
*z.NumReplicas != *other.NumReplicas {
// In cases where one of the zone configs are placeholders,
// defer the error reporting to below so that we can correctly
// report on a subzone difference, should one exist.
if z.IsSubzonePlaceholder() || other.IsSubzonePlaceholder() {
mismatchingNumReplicas = true
continue
}
return false, DiffWithZoneMismatch{
Field: "num_replicas",
}, nil
Expand Down Expand Up @@ -825,6 +833,13 @@ func (z *ZoneConfig) DiffWithZone(
}, nil
}
}
// If we've got a mismatch in the num_replicas field and we haven't found
// any other mismatch, report on num_replicas.
if mismatchingNumReplicas {
return false, DiffWithZoneMismatch{
Field: "num_replicas",
}, nil
}
return true, DiffWithZoneMismatch{}, nil
}

Expand Down

0 comments on commit a5f2857

Please sign in to comment.