Skip to content

Commit

Permalink
Merge #60761
Browse files Browse the repository at this point in the history
60761: sql: hide display of partitions on SHOW CREATE for REGIONAL BY ROW tables r=ajstorm a=otan

Resolves #60656 

sql: hide display of partitions on SHOW CREATE for REGIONAL BY ROW tables

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.



Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
craig[bot] and otan committed Feb 24, 2021
2 parents b4e9377 + 350a29c commit 3bae09b
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 @@ -325,15 +325,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 @@ -346,7 +337,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 @@ -484,7 +475,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 @@ -505,7 +496,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 3bae09b

Please sign in to comment.