Skip to content

Commit

Permalink
sql: hide display of partitions on SHOW CREATE for REGIONAL BY ROW ta…
Browse files Browse the repository at this point in the history
…bles

This commit hides the partition zone configuration for REGIONAL BY ROW
tables.

This is achieved by the following:
* Introduce a is_multi_region column for crdb_internal.create_statements
* Introduce a builtin
  `crdb_internal.filter_multiregion_fields_from_zone_config_sql` which
  removes multi region zone config fields.
* Filter the partition statements out from the SHOW CREATE TABLE
  delegate function.
* Change REGIONAL BY ROW tests to read from SHOW PARTITIONS so that
  we can still see the zone configurations and partitions.

Release justification: low risk change to new functionality

Release note (sql change): Introduced a new is_multi_region column to
crdb_internal.create_statements, which informs whether the database of
the object is a multi-region database.

Release note (sql change): Introduced a new built-in
`crdb_internal.filter_multiregion_fields_from_zone_config_sql` which
removes multi-region fields from a CONFIGURE ZONE statement.
  • Loading branch information
otan committed Feb 24, 2021
1 parent b1d518f commit 350a29c
Show file tree
Hide file tree
Showing 16 changed files with 2,357 additions and 1,917 deletions.
5 changes: 5 additions & 0 deletions docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -2315,6 +2315,11 @@ 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(dbName: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns a flat log of CREATE table statements followed by
ALTER table statements that add table constraints. The flat log can be used
to recreate a database.’</p>
Expand Down
1,929 changes: 1,217 additions & 712 deletions pkg/ccl/logictestccl/testdata/logic_test/alter_table_locality

Large diffs are not rendered by default.

852 changes: 450 additions & 402 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup

Large diffs are not rendered by default.

1,273 changes: 570 additions & 703 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -83,48 +83,12 @@ query T
SELECT create_statement FROM [SHOW CREATE TABLE regional_by_row_table_as]
----
CREATE TABLE public.regional_by_row_table_as (
pk INT8 NOT NULL,
a INT8 NULL,
b INT8 NULL,
cr public.crdb_internal_region NOT NULL AS (CASE WHEN pk <= 10:::INT8 THEN 'us-east-1':::public.crdb_internal_region ELSE 'ap-southeast-2':::public.crdb_internal_region END) STORED,
CONSTRAINT "primary" PRIMARY KEY (pk ASC),
INDEX regional_by_row_table_as_a_idx (a ASC),
UNIQUE INDEX regional_by_row_table_as_b_key (b ASC),
FAMILY fam_0_pk_a_b_crdb_region_col (pk, a, b, cr)
) LOCALITY REGIONAL BY ROW AS cr;
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table_as@primary CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table_as@regional_by_row_table_as_a_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ap-southeast-2" OF INDEX multi_region_test_db.public.regional_by_row_table_as@regional_by_row_table_as_b_key CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ap-southeast-2: 2}',
lease_preferences = '[[+region=ap-southeast-2]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table_as@primary CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table_as@regional_by_row_table_as_a_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "ca-central-1" OF INDEX multi_region_test_db.public.regional_by_row_table_as@regional_by_row_table_as_b_key CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=ca-central-1: 2}',
lease_preferences = '[[+region=ca-central-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table_as@primary CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table_as@regional_by_row_table_as_a_idx CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]';
ALTER PARTITION "us-east-1" OF INDEX multi_region_test_db.public.regional_by_row_table_as@regional_by_row_table_as_b_key CONFIGURE ZONE USING
num_voters = 5,
voter_constraints = '{+region=us-east-1: 2}',
lease_preferences = '[[+region=us-east-1]]'
pk INT8 NOT NULL,
a INT8 NULL,
b INT8 NULL,
cr public.crdb_internal_region NOT NULL AS (CASE WHEN pk <= 10:::INT8 THEN 'us-east-1':::public.crdb_internal_region ELSE 'ap-southeast-2':::public.crdb_internal_region END) STORED,
CONSTRAINT "primary" PRIMARY KEY (pk ASC),
INDEX regional_by_row_table_as_a_idx (a ASC),
UNIQUE INDEX regional_by_row_table_as_b_key (b ASC),
FAMILY fam_0_pk_a_b_crdb_region_col (pk, a, b, cr)
) LOCALITY REGIONAL BY ROW AS cr
6 changes: 3 additions & 3 deletions pkg/ccl/multiregionccl/regional_by_row_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,15 @@ func TestRepartitionFailureRollback(t *testing.T) {
defer cleanup()

_, err := sqlDB.Exec(
`CREATE DATABASE db WITH PRIMARY REGION "us-east1" REGIONS "us-east2";
`CREATE DATABASE db WITH PRIMARY REGION "us-east1" REGIONS "us-east2";
CREATE TABLE db.t(k INT PRIMARY KEY) LOCALITY REGIONAL BY ROW`)
require.NoError(t, err)
if err != nil {
t.Error(err)
}

_, err = sqlDB.Exec(`BEGIN;
ALTER DATABASE db ADD REGION "us-east3";
_, err = sqlDB.Exec(`BEGIN;
ALTER DATABASE db ADD REGION "us-east3";
ALTER DATABASE db DROP REGION "us-east2";
COMMIT;`)
require.Error(t, err, "boom")
Expand Down
20 changes: 2 additions & 18 deletions pkg/ccl/multiregionccl/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,7 @@ func TestShowCreateTable(t *testing.T) {
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
INDEX a_idx (a ASC),
FAMILY "primary" (a, crdb_region, rowid)
) LOCALITY REGIONAL BY ROW;
ALTER PARTITION "us-west1" OF INDEX mrdb.public.%[1]s@a_idx CONFIGURE ZONE USING
num_voters = 3,
voter_constraints = '[+region=us-west1]',
lease_preferences = '[[+region=us-west1]]';
ALTER PARTITION "us-west1" OF INDEX mrdb.public.%[1]s@primary CONFIGURE ZONE USING
num_voters = 3,
voter_constraints = '[+region=us-west1]',
lease_preferences = '[[+region=us-west1]]'`,
) LOCALITY REGIONAL BY ROW`,
Database: "mrdb",
},
{
Expand All @@ -98,15 +90,7 @@ ALTER PARTITION "us-west1" OF INDEX mrdb.public.%[1]s@primary CONFIGURE ZONE USI
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
INDEX a_idx (a ASC),
FAMILY "primary" (a, crdb_region_col, rowid)
) LOCALITY REGIONAL BY ROW AS crdb_region_col;
ALTER PARTITION "us-west1" OF INDEX mrdb.public.%[1]s@a_idx CONFIGURE ZONE USING
num_voters = 3,
voter_constraints = '[+region=us-west1]',
lease_preferences = '[[+region=us-west1]]';
ALTER PARTITION "us-west1" OF INDEX mrdb.public.%[1]s@primary CONFIGURE ZONE USING
num_voters = 3,
voter_constraints = '[+region=us-west1]',
lease_preferences = '[[+region=us-west1]]'`,
) LOCALITY REGIONAL BY ROW AS crdb_region_col`,
Database: "mrdb",
},
}
Expand Down
21 changes: 21 additions & 0 deletions pkg/config/zonepb/zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,27 @@ var NamedZonesByID = func() map[uint32]string {
return out
}()

// MultiRegionZoneConfigFields are the fields on a zone configuration which
// may be set by the system for multi-region objects".
var MultiRegionZoneConfigFields = []tree.Name{
"global_reads",
"num_replicas",
"num_voters",
"constraints",
"voter_constraints",
"lease_preferences",
}

// MultiRegionZoneConfigFieldsSet contain the items in
// MultiRegionZoneConfigFields but in a set form for fast lookup.
var MultiRegionZoneConfigFieldsSet = func() map[tree.Name]struct{} {
ret := make(map[tree.Name]struct{}, len(MultiRegionZoneConfigFields))
for _, f := range MultiRegionZoneConfigFields {
ret[f] = struct{}{}
}
return ret
}()

// ZoneSpecifierFromID creates a tree.ZoneSpecifier for the zone with the
// given ID.
func ZoneSpecifierFromID(
Expand Down
24 changes: 13 additions & 11 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -1839,6 +1839,7 @@ CREATE TABLE crdb_internal.create_statements (
alter_statements STRING[] NOT NULL,
validate_statements STRING[] NOT NULL,
has_partitions BOOL NOT NULL,
is_multi_region BOOL NOT NULL,
INDEX(descriptor_id)
)
`, virtualOnce, false, /* includesIndexEntries */
Expand Down Expand Up @@ -1906,6 +1907,7 @@ CREATE TABLE crdb_internal.create_statements (
alterStmts,
validateStmts,
tree.MakeDBool(tree.DBool(hasPartitions)),
tree.MakeDBool(tree.DBool(db != nil && db.IsMultiRegion())),
)
})

Expand Down Expand Up @@ -2789,20 +2791,20 @@ var crdbInternalZonesTable = virtualSchemaTable{
comment: "decoded zone configurations from system.zones (KV scan)",
schema: `
CREATE TABLE crdb_internal.zones (
zone_id INT NOT NULL,
subzone_id INT NOT NULL,
target STRING,
range_name STRING,
database_name STRING,
table_name STRING,
index_name STRING,
partition_name STRING,
zone_id INT NOT NULL,
subzone_id INT NOT NULL,
target STRING,
range_name STRING,
database_name STRING,
table_name STRING,
index_name STRING,
partition_name STRING,
raw_config_yaml STRING NOT NULL,
raw_config_sql STRING, -- this column can be NULL if there is no specifier syntax
-- possible (e.g. the object was deleted).
-- possible (e.g. the object was deleted).
raw_config_protobuf BYTES NOT NULL,
full_config_yaml STRING NOT NULL,
full_config_sql STRING
full_config_yaml STRING NOT NULL,
full_config_sql STRING
)
`,
populate: func(ctx context.Context, p *planner, _ *dbdesc.Immutable, addRow func(...tree.Datum) error) error {
Expand Down
24 changes: 20 additions & 4 deletions pkg/sql/delegate/show_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,18 @@ func (d *delegator) delegateShowCreate(n *tree.ShowCreate) (tree.Statement, erro

const showCreateQuery = `
WITH zone_configs AS (
SELECT string_agg(raw_config_sql, e';\n' ORDER BY partition_name, index_name) FROM crdb_internal.zones
SELECT
string_agg(
raw_config_sql,
e';\n'
ORDER BY partition_name, index_name
) AS raw,
string_agg(
crdb_internal.filter_multiregion_fields_from_zone_config_sql(raw_config_sql),
e';\n'
ORDER BY partition_name, index_name
) AS mr
FROM crdb_internal.zones
WHERE database_name = %[1]s
AND table_name = %[2]s
AND raw_config_yaml IS NOT NULL
Expand All @@ -36,9 +47,14 @@ SELECT
CASE
WHEN NOT has_partitions
THEN NULL
WHEN (SELECT * FROM zone_configs) IS NULL
THEN e'\n-- Warning: Partitioned table with no zone configurations.'
ELSE concat(e';\n', (SELECT * FROM zone_configs))
WHEN is_multi_region THEN
CASE
WHEN (SELECT mr FROM zone_configs) IS NULL THEN NULL
ELSE concat(e';\n', (SELECT mr FROM zone_configs))
END
WHEN (SELECT raw FROM zone_configs) IS NULL THEN
e'\n-- Warning: Partitioned table with no zone configurations.'
ELSE concat(e';\n', (SELECT raw FROM zone_configs))
END
) AS create_statement
FROM
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,10 @@ SELECT * FROM crdb_internal.builtin_functions WHERE function = ''
----
function signature category details

query ITTITTTTTTTT colnames
query ITTITTTTTTTBB colnames
SELECT * FROM crdb_internal.create_statements WHERE database_name = ''
----
database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions
database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions is_multi_region

query ITITTBTB colnames
SELECT * FROM crdb_internal.table_columns WHERE descriptor_name = ''
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal_tenant
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ SELECT * FROM crdb_internal.builtin_functions WHERE function = ''
----
function signature category details

query ITTITTTTTTTT colnames
query ITTITTTTTTTBB colnames
SELECT * FROM crdb_internal.create_statements WHERE database_name = ''
----
database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions
database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions is_multi_region

query ITITTBTB colnames
SELECT * FROM crdb_internal.table_columns WHERE descriptor_name = ''
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/sequences
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ CREATE SEQUENCE ignored_options_test NO CYCLE
statement ok
CREATE SEQUENCE show_create_test

query ITTITTTTTTTB colnames
query ITTITTTTTTTBB colnames
SELECT * FROM crdb_internal.create_statements WHERE descriptor_name = 'show_create_test'
----
database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions
52 test public 63 sequence show_create_test CREATE SEQUENCE public.show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 PUBLIC CREATE SEQUENCE public.show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 {} {} false
database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions is_multi_region
52 test public 63 sequence show_create_test CREATE SEQUENCE public.show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 PUBLIC CREATE SEQUENCE public.show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 {} {} false false

query TT colnames
SHOW CREATE SEQUENCE show_create_test
Expand Down
15 changes: 3 additions & 12 deletions pkg/sql/region_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,15 +323,6 @@ func synthesizeVoterConstraints(
}
}

var multiRegionZoneConfigFields = []tree.Name{
"global_reads",
"num_replicas",
"num_voters",
"constraints",
"voter_constraints",
"lease_preferences",
}

// zoneConfigForMultiRegionTable generates a ZoneConfig stub for a
// regional-by-table or global table in a multi-region database.
//
Expand All @@ -344,7 +335,7 @@ var multiRegionZoneConfigFields = []tree.Name{
// configuration is required.
//
// Relevant multi-region configured fields (as defined in
// `multiRegionZoneConfigFields`) will be overwritten by the calling function
// `zonepb.MultiRegionZoneConfigFields`) will be overwritten by the calling function
// into an existing ZoneConfig.
func zoneConfigForMultiRegionTable(
localityConfig descpb.TableDescriptor_LocalityConfig,
Expand Down Expand Up @@ -482,7 +473,7 @@ func applyZoneConfigForMultiRegionTableOptionTableNewConfig(
if err != nil {
return false, zonepb.ZoneConfig{}, err
}
zc.CopyFromZone(*localityZoneConfig, multiRegionZoneConfigFields)
zc.CopyFromZone(*localityZoneConfig, zonepb.MultiRegionZoneConfigFields)
return false, zc, nil
}
}
Expand All @@ -503,7 +494,7 @@ var ApplyZoneConfigForMultiRegionTableOptionTableAndIndexes = func(
if err != nil {
return false, zonepb.ZoneConfig{}, err
}
zc.CopyFromZone(*localityZoneConfig, multiRegionZoneConfigFields)
zc.CopyFromZone(*localityZoneConfig, zonepb.MultiRegionZoneConfigFields)

hasNewSubzones := table.IsLocalityRegionalByRow()
if hasNewSubzones {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/builtins/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ go_library(
"//pkg/base",
"//pkg/build",
"//pkg/clusterversion",
"//pkg/config/zonepb",
"//pkg/geo",
"//pkg/geo/geogfn",
"//pkg/geo/geoindex",
Expand Down
36 changes: 36 additions & 0 deletions pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/roachpb"
Expand Down Expand Up @@ -4969,6 +4970,41 @@ the locality flag on node startup. Returns an error if no region is set.`,
tree.VolatilityStable,
),
),
"crdb_internal.filter_multiregion_fields_from_zone_config_sql": makeBuiltin(
tree.FunctionProperties{},
stringOverload1(
func(evalCtx *tree.EvalContext, s string) (tree.Datum, error) {
stmt, err := parser.ParseOne(s)
if err != nil {
// Return the same statement if it does not parse.
// This can happen for invalid zone config state, in which case
// it is better not to error as opposed to blocking SHOW CREATE TABLE.
return tree.NewDString(s), nil //nolint:returnerrcheck
}
zs, ok := stmt.AST.(*tree.SetZoneConfig)
if !ok {
return nil, errors.Newf("invalid CONFIGURE ZONE statement (type %T): %s", stmt.AST, stmt)
}
newKVOptions := zs.Options[:0]
for _, opt := range zs.Options {
if _, ok := zonepb.MultiRegionZoneConfigFieldsSet[opt.Key]; !ok {
newKVOptions = append(newKVOptions, opt)
}
}
if len(newKVOptions) == 0 {
return tree.DNull, nil
}
zs.Options = newKVOptions
return tree.NewDString(zs.String()), nil
},
types.String,
`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.`,
tree.VolatilityStable,
),
),

"crdb_internal.show_create_all_tables": makeBuiltin(
tree.FunctionProperties{},
Expand Down

0 comments on commit 350a29c

Please sign in to comment.