Skip to content

Commit

Permalink
Merge cockroachdb#62780
Browse files Browse the repository at this point in the history 
62780: sql: add crdb_internal.validate_multi_region_zone_configs builtin r=ajstorm,arulajmani a=otan

In addition to the release note, added some pgcodes to validation errors
as well as minor refactoring to make it work and error nicely with the
"overriding" error message.

Release note (sql change): Add a new builtin
`crdb_internal.validate_multi_region_zone_configs` which validates all
zone configurations in the current database is correctly configured.

Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
@otan
craig[bot] and otan committed Mar 31, 2021
2 parents d145e9f + f7f895c commit fe14895
Show file tree
Hide file tree
Showing 8 changed files with 243 additions and 66 deletions.
15 changes: 10 additions & 5 deletions docs/generated/sql/functions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,16 @@ has no relationship with the commit order of concurrent transactions.</p>
<table>
<thead><tr><th>Function &rarr; Returns</th><th>Description</th></tr></thead>
<tbody>
<tr><td><a name="crdb_internal.filter_multiregion_fields_from_zone_config_sql"></a><code>crdb_internal.filter_multiregion_fields_from_zone_config_sql(val: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Takes in a CONFIGURE ZONE SQL statement and returns a modified
SQL statement omitting multi-region related zone configuration fields.
If the CONFIGURE ZONE statement can be inferred by the database’s or
table’s zone configuration this will return NULL.</p>
</span></td></tr>
<tr><td><a name="crdb_internal.validate_multi_region_zone_configs"></a><code>crdb_internal.validate_multi_region_zone_configs() &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Validates all multi-region zone configurations are correctly setup
for the current database, including all tables, indexes and partitions underneath.
Returns an error if validation fails. This builtin uses un-leased versions of the
each descriptor, requiring extra round trips.</p>
</span></td></tr>
<tr><td><a name="default_to_database_primary_region"></a><code>default_to_database_primary_region(val: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the given region if the region has been added to the current database.
Otherwise, this will return the primary region of the current database.
This will error if the current database is not a multi-region database.</p>
Expand Down Expand Up @@ -2372,11 +2382,6 @@ The swap_ordinate_string parameter is a 2-character string naming the ordinates
</span></td></tr>
<tr><td><a name="convert_to"></a><code>convert_to(str: <a href="string.html">string</a>, enc: <a href="string.html">string</a>) &rarr; <a href="bytes.html">bytes</a></code></td><td><span class="funcdesc"><p>Encode the string <code>str</code> as a byte array using encoding <code>enc</code>. Supports encodings ‘UTF8’ and ‘LATIN1’.</p>
</span></td></tr>
<tr><td><a name="crdb_internal.filter_multiregion_fields_from_zone_config_sql"></a><code>crdb_internal.filter_multiregion_fields_from_zone_config_sql(val: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Takes in a CONFIGURE ZONE SQL statement and returns a modified
SQL statement omitting multi-region related zone configuration fields.
If the CONFIGURE ZONE statement can be inferred by the database’s or
table’s zone configuration this will return NULL.</p>
</span></td></tr>
<tr><td><a name="crdb_internal.show_create_all_tables"></a><code>crdb_internal.show_create_all_tables(database_name: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns rows of CREATE table statements followed by
ALTER table statements that add table constraints. The rows are ordered
by dependencies. All foreign keys are added after the creation of the table
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/importccl/import_table_creation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ func (so *importSequenceOperators) CurrentDatabaseRegionConfig(
return nil, errors.WithStack(errSequenceOperators)
}

// ValidateAllMultiRegionZoneConfigsInCurrentDatabase is part of the tree.EvalDatabase interface.
func (so *importSequenceOperators) ValidateAllMultiRegionZoneConfigsInCurrentDatabase(
_ context.Context,
) error {
return errors.WithStack(errSequenceOperators)
}

// Implements the tree.EvalDatabase interface.
func (so *importSequenceOperators) ParseQualifiedTableName(sql string) (*tree.TableName, error) {
name, err := parser.ParseTableName(sql)
Expand Down
30 changes: 27 additions & 3 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region_zone_configs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@ ap-southeast-2 {ap-az1,ap-az2,ap-az3} {} {}
ca-central-1 {ca-az1,ca-az2,ca-az3} {} {}
us-east-1 {us-az1,us-az2,us-az3} {} {}

statement ok
SELECT crdb_internal.validate_multi_region_zone_configs()

statement ok
CREATE DATABASE "mr-zone-configs" primary region "ca-central-1" regions "ap-southeast-2", "us-east-1"

statement ok
use "mr-zone-configs"

statement ok
SELECT crdb_internal.validate_multi_region_zone_configs()

statement ok
ALTER DATABASE "mr-zone-configs" CONFIGURE ZONE USING gc.ttlseconds = 5

Expand All @@ -32,6 +38,9 @@ DATABASE "mr-zone-configs" ALTER DATABASE "mr-zone-configs" CONFIGURE ZONE USIN
voter_constraints = '[+region=ca-central-1]',
lease_preferences = '[[+region=ca-central-1]]'

statement ok
SELECT crdb_internal.validate_multi_region_zone_configs()

statement error attempting to modify protected field "num_voters" of a multi-region zone configuration
ALTER DATABASE "mr-zone-configs" CONFIGURE ZONE USING num_voters = 5

Expand All @@ -53,6 +62,9 @@ SET override_multi_region_zone_config = true;
ALTER DATABASE "mr-zone-configs" CONFIGURE ZONE USING num_voters = 5;
SET override_multi_region_zone_config = false

statement error zone configuration for database "mr-zone-configs" contains incorrectly configured field "num_voters"
SELECT crdb_internal.validate_multi_region_zone_configs()

query TT
SHOW ZONE CONFIGURATION FOR DATABASE "mr-zone-configs"
----
Expand Down Expand Up @@ -308,6 +320,9 @@ CREATE TABLE regional_by_table (
statement ok
ALTER table regional_by_row CONFIGURE ZONE USING gc.ttlseconds = 10

statement ok
SELECT crdb_internal.validate_multi_region_zone_configs()

statement error attempting to modify protected field "num_replicas" of a multi-region zone configuration
ALTER table regional_by_row CONFIGURE ZONE USING num_replicas = 10

Expand All @@ -316,6 +331,9 @@ SET override_multi_region_zone_config = true;
ALTER table regional_by_row CONFIGURE ZONE USING num_replicas = 10;
SET override_multi_region_zone_config = false

statement error zone configuration for table regional_by_row contains incorrectly configured field "num_replicas"
SELECT crdb_internal.validate_multi_region_zone_configs()

query TT
SHOW ZONE CONFIGURATION FOR TABLE regional_by_row
----
Expand Down Expand Up @@ -361,7 +379,7 @@ num_voters = 3,
voter_constraints = '[+region=us-east-1]',
lease_preferences = '[[+region=us-east-1]]' regional_by_row@regional_by_row_i_idx us-east-1

statement error attempting to update zone configuration for table "regional_by_row" which contains modified field "num_replicas"
statement error attempting to update zone configuration for table regional_by_row which contains modified field "num_replicas"
ALTER TABLE regional_by_row SET LOCALITY REGIONAL BY TABLE

statement ok
Expand Down Expand Up @@ -422,6 +440,9 @@ ALTER TABLE regional_by_row SET LOCALITY REGIONAL BY ROW
statement ok
ALTER TABLE regional_by_row SET LOCALITY GLOBAL

statement ok
SELECT crdb_internal.validate_multi_region_zone_configs()

statement ok
SET override_multi_region_zone_config = true;
ALTER index regional_by_row@primary CONFIGURE ZONE USING num_replicas = 10;
Expand All @@ -430,7 +451,10 @@ SET override_multi_region_zone_config = false
statement ok
ALTER TABLE regional_by_row SET LOCALITY GLOBAL

statement error attempting to update zone configuration for table "regional_by_row" which contains a zone configuration on index "primary"
statement error zone configuration for index regional_by_row@"primary" contains incorrectly configured field "num_replicas"
SELECT crdb_internal.validate_multi_region_zone_configs()

statement error attempting to update zone configuration for index regional_by_row@"primary" which contains modified field "num_replicas"
ALTER TABLE regional_by_row SET LOCALITY REGIONAL BY ROW

statement ok
Expand Down Expand Up @@ -465,7 +489,7 @@ INDEX regional_by_row_as@primary ALTER INDEX regional_by_row_as@primary CONFIGU
voter_constraints = '[+region=us-east-1]',
lease_preferences = '[[+region=us-east-1]]'

statement error attempting to update zone configuration for table "regional_by_row_as" which contains a zone configuration on index "primary" with multi-region field "num_replicas" set
statement error attempting to update zone configuration for index regional_by_row_as@"primary" which contains modified field "num_replicas"
ALTER TABLE regional_by_row_as SET LOCALITY REGIONAL BY ROW

statement ok
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/alter_table_locality.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,13 +470,13 @@ func (n *alterTableSetLocalityNode) startExec(params runParams) error {
),
)

toRegionalByRow := newLocality.LocalityLevel == tree.LocalityLevelRow
// We should check index zone configs if moving to REGIONAL BY ROW.
checkIndexZoneConfigs := newLocality.LocalityLevel == tree.LocalityLevelRow
if err := params.p.validateZoneConfigForMultiRegionTableWasNotModifiedByUser(
params.ctx,
n.dbDesc,
n.tableDesc,
toRegionalByRow,
ApplyZoneConfigForMultiRegionTableOptionTableAndIndexes,
checkIndexZoneConfigs,
); err != nil {
return err
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/faketreeeval/evalctx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ func (so *DummySequenceOperators) CurrentDatabaseRegionConfig(
return nil, errors.WithStack(errSequenceOperators)
}

// ValidateAllMultiRegionZoneConfigsInCurrentDatabase is part of the tree.EvalDatabase interface.
func (so *DummySequenceOperators) ValidateAllMultiRegionZoneConfigsInCurrentDatabase(
_ context.Context,
) error {
return errors.WithStack(errSequenceOperators)
}

// ParseQualifiedTableName is part of the tree.EvalDatabase interface.
func (so *DummySequenceOperators) ParseQualifiedTableName(sql string) (*tree.TableName, error) {
return nil, errors.WithStack(errSequenceOperators)
Expand Down Expand Up @@ -186,6 +193,13 @@ func (ep *DummyEvalPlanner) CurrentDatabaseRegionConfig(
return nil, errors.WithStack(errEvalPlanner)
}

// ValidateAllMultiRegionZoneConfigsInCurrentDatabase is part of the tree.EvalDatabase interface.
func (ep *DummyEvalPlanner) ValidateAllMultiRegionZoneConfigsInCurrentDatabase(
_ context.Context,
) error {
return errors.WithStack(errEvalPlanner)
}

// ParseQualifiedTableName is part of the tree.EvalDatabase interface.
func (ep *DummyEvalPlanner) ParseQualifiedTableName(sql string) (*tree.TableName, error) {
return parser.ParseQualifiedTableName(sql)
Expand Down
Loading

0 comments on commit fe14895

Please sign in to comment.