Skip to content

Commit

Permalink
Merge #28612
Browse files Browse the repository at this point in the history
28612: cli,sql: improve the UX for viewing and updating zone configurations r=knz a=knz

Fixes  #23791.

Summary of visible changes:

- new statement forms:
  - `ALTER ... CONFIGURE ZONE USING num_replicas = 123, ...` to edit a zone config. The compact constraint syntax is still supported, e.g. `USING constraints = '[+foo]'`.
  - `ALTER ... CONFIGURE ZONE USING DEFAULT` to reset a zone config to inherited defaults
  - `ALTER ... CONFIGURE ZONE DISCARD` to remove a zone config

- compatibility forms with previous versions of CockroachDB, probably should not be documented but will ease transition:
  - `ALTER ... CONFIGURE ZONE = '...yaml...'` to set multiple zone parameters at once using YAML
  - `ALTER ... CONFIGURE ZONE = ''` ensures the zone config exists but is otherwise a no-op (it does not reset it to defaults if it already existed)
  - `ALTER ... CONFIGURE ZONE = NULL` to remove a zone config

- the `ALTER ... CONFIGURE ZONE` statement forms now can be prepared on pgwire and accept placeholders.

- `crdb_internal.zones` and `SHOW ZONE CONFIGURATION(s)` have a new column `config_sql` in addition to the existing `config_yaml` and `config_protobuf`. This new columns reports the ALTER syntax that reproduces the zone config with the new syntax.

- `cockroach zone` sub-commands are now deprecated.

- `cockroach zone rm` now prints no output unless there is an error.


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
  • Loading branch information
craig[bot] and knz committed Sep 6, 2018
2 parents e4b6f7a + bfc190d commit 47c7bfe
Show file tree
Hide file tree
Showing 55 changed files with 1,315 additions and 560 deletions.
4 changes: 2 additions & 2 deletions docs/generated/sql/bnf/set_var.bnf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
set_stmt ::=
'SET' ( 'SESSION' | ) var_name 'TO' var_value ( ( ',' var_value ) )*
| 'SET' ( 'SESSION' | ) var_name '=' var_value ( ( ',' var_value ) )*
'SET' ( 'SESSION' | ) var_name '=' var_value ( ( ',' var_value ) )*
| 'SET' ( 'SESSION' | ) var_name 'TO' var_value ( ( ',' var_value ) )*
| set_csetting_stmt
| set_transaction_stmt
| use_stmt
1 change: 1 addition & 0 deletions docs/generated/sql/bnf/show_var.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ show_stmt ::=
| show_tables_stmt
| show_trace_stmt
| show_users_stmt
| show_zone_stmt
66 changes: 56 additions & 10 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ show_stmt ::=
| show_tables_stmt
| show_trace_stmt
| show_users_stmt
| show_zone_stmt

transaction_stmt ::=
begin_stmt
Expand All @@ -189,6 +190,7 @@ alter_ddl_stmt ::=
| alter_view_stmt
| alter_sequence_stmt
| alter_database_stmt
| alter_range_stmt

alter_user_stmt ::=
alter_user_password_stmt
Expand Down Expand Up @@ -421,8 +423,7 @@ set_session_stmt ::=
| 'SET' 'SESSION' 'CHARACTERISTICS' 'AS' 'TRANSACTION' transaction_mode_list

set_csetting_stmt ::=
'SET' 'CLUSTER' 'SETTING' var_name '=' var_value
| 'SET' 'CLUSTER' 'SETTING' var_name 'TO' var_value
'SET' 'CLUSTER' 'SETTING' var_name to_or_eq var_value

set_transaction_stmt ::=
'SET' 'TRANSACTION' transaction_mode_list
Expand Down Expand Up @@ -500,6 +501,15 @@ show_trace_stmt ::=
show_users_stmt ::=
'SHOW' 'USERS'

show_zone_stmt ::=
'SHOW' 'ZONE' 'CONFIGURATION' 'FOR' 'RANGE' zone_name
| 'SHOW' 'ZONE' 'CONFIGURATION' 'FOR' 'DATABASE' database_name
| 'SHOW' 'ZONE' 'CONFIGURATION' 'FOR' 'TABLE' table_name opt_partition
| 'SHOW' 'ZONE' 'CONFIGURATION' 'FOR' 'PARTITION' partition_name 'OF' 'TABLE' table_name
| 'SHOW' 'ZONE' 'CONFIGURATION' 'FOR' 'INDEX' table_name_with_index
| 'SHOW' 'ZONE' 'CONFIGURATIONS'
| 'SHOW' 'ALL' 'ZONE' 'CONFIGURATIONS'

begin_stmt ::=
'BEGIN' opt_transaction begin_transaction
| 'START' 'TRANSACTION' begin_transaction
Expand Down Expand Up @@ -533,13 +543,15 @@ alter_table_stmt ::=
alter_onetable_stmt
| alter_split_stmt
| alter_scatter_stmt
| alter_zone_table_stmt
| alter_rename_table_stmt

alter_index_stmt ::=
alter_oneindex_stmt
| alter_split_index_stmt
| alter_scatter_index_stmt
| alter_rename_index_stmt
| alter_zone_index_stmt

alter_view_stmt ::=
alter_rename_view_stmt
Expand All @@ -550,6 +562,10 @@ alter_sequence_stmt ::=

alter_database_stmt ::=
alter_rename_database_stmt
| alter_zone_database_stmt

alter_range_stmt ::=
alter_zone_range_stmt

alter_user_password_stmt ::=
'ALTER' 'USER' string_or_placeholder 'WITH' 'PASSWORD' string_or_placeholder
Expand Down Expand Up @@ -1029,6 +1045,10 @@ set_rest_more ::=
transaction_mode_list ::=
( transaction_mode ) ( ( opt_comma transaction_mode ) )*

to_or_eq ::=
'='
| 'TO'

var_value ::=
a_expr
| 'ON'
Expand All @@ -1053,6 +1073,16 @@ opt_compact ::=
'COMPACT'
|

zone_name ::=
unrestricted_name

opt_partition ::=
partition
|

partition_name ::=
unrestricted_name

opt_transaction ::=
'TRANSACTION'
|
Expand Down Expand Up @@ -1087,6 +1117,10 @@ alter_scatter_stmt ::=
'ALTER' 'TABLE' table_name 'SCATTER'
| 'ALTER' 'TABLE' table_name 'SCATTER' 'FROM' '(' expr_list ')' 'TO' '(' expr_list ')'

alter_zone_table_stmt ::=
'ALTER' 'TABLE' table_name set_zone_config
| 'ALTER' 'PARTITION' partition_name 'OF' 'TABLE' table_name set_zone_config

alter_rename_table_stmt ::=
'ALTER' 'TABLE' relation_expr 'RENAME' 'TO' table_name
| 'ALTER' 'TABLE' 'IF' 'EXISTS' relation_expr 'RENAME' 'TO' table_name
Expand All @@ -1108,6 +1142,9 @@ alter_rename_index_stmt ::=
'ALTER' 'INDEX' table_name_with_index 'RENAME' 'TO' index_name
| 'ALTER' 'INDEX' 'IF' 'EXISTS' table_name_with_index 'RENAME' 'TO' index_name

alter_zone_index_stmt ::=
'ALTER' 'INDEX' table_name_with_index set_zone_config

alter_rename_view_stmt ::=
'ALTER' 'VIEW' relation_expr 'RENAME' 'TO' view_name
| 'ALTER' 'VIEW' 'IF' 'EXISTS' relation_expr 'RENAME' 'TO' view_name
Expand All @@ -1123,6 +1160,12 @@ alter_sequence_options_stmt ::=
alter_rename_database_stmt ::=
'ALTER' 'DATABASE' database_name 'RENAME' 'TO' database_name

alter_zone_database_stmt ::=
'ALTER' 'DATABASE' database_name set_zone_config

alter_zone_range_stmt ::=
'ALTER' 'RANGE' zone_name set_zone_config

complex_db_object_name ::=
name '.' unrestricted_name
| name '.' unrestricted_name '.' unrestricted_name
Expand Down Expand Up @@ -1369,8 +1412,7 @@ attrs ::=
( '.' unrestricted_name ) ( ( '.' unrestricted_name ) )*

generic_set ::=
var_name 'TO' var_list
| var_name '=' var_list
var_name to_or_eq var_list

transaction_mode ::=
transaction_iso_level
Expand All @@ -1385,6 +1427,9 @@ targets_roles ::=
'ROLE' name_list
| targets

partition ::=
'PARTITION' partition_name

single_set_clause ::=
column_name '=' a_expr

Expand All @@ -1394,6 +1439,10 @@ multiple_set_clause ::=
alter_table_cmds ::=
( alter_table_cmd ) ( ( ',' alter_table_cmd ) )*

set_zone_config ::=
'CONFIGURE' 'ZONE' 'USING' var_set_list
| 'CONFIGURE' 'ZONE' 'DISCARD'

opt_column ::=
'COLUMN'
|
Expand Down Expand Up @@ -1739,6 +1788,9 @@ alter_table_cmd ::=
| 'EXPERIMENTAL_AUDIT' 'SET' audit_mode
| partition_by

var_set_list ::=
( var_name '=' var_value ) ( ( ',' var_name '=' var_value ) )*

alter_index_cmd ::=
partition_by

Expand Down Expand Up @@ -2066,9 +2118,6 @@ func_expr_windowless ::=
rowsfrom_list ::=
( rowsfrom_item ) ( ( ',' rowsfrom_item ) )*

partition ::=
'PARTITION' partition_name

opt_name_parens ::=
'(' name ')'
|
Expand Down Expand Up @@ -2135,9 +2184,6 @@ join_outer ::=
rowsfrom_item ::=
func_expr_windowless

partition_name ::=
unrestricted_name

frame_extent ::=
frame_bound
| 'BETWEEN' frame_bound 'AND' frame_bound
Expand Down
15 changes: 12 additions & 3 deletions pkg/ccl/cliccl/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 1
// constraints: [+zone=us-east-1a, +ssd]
// lease_preferences: []
//
// zone get db.t.p0
// db.t@primary
// range_min_bytes: 1048576
Expand All @@ -141,6 +143,7 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 1
// constraints: [+zone=us-east-1a, +ssd]
// lease_preferences: []
// zone get db.t
// .default
// range_min_bytes: 1048576
Expand All @@ -149,6 +152,7 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 1
// constraints: []
// lease_preferences: []
// zone get db.t@t_c2_idx
// .default
// range_min_bytes: 1048576
Expand All @@ -157,13 +161,16 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 1
// constraints: []
// lease_preferences: []
// zone set db.t.p1 --file=./../../cli/testdata/zone_range_max_bytes.yaml
// range_min_bytes: 1048576
// range_max_bytes: 134217728
// gc:
// ttlseconds: 90000
// num_replicas: 3
// constraints: [+zone=us-east-1a, +ssd]
// lease_preferences: []
//
// zone get db.t.p1
// db.t.p1
// range_min_bytes: 1048576
Expand All @@ -172,6 +179,7 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 3
// constraints: [+zone=us-east-1a, +ssd]
// lease_preferences: []
// zone get db.t.p0
// db.t@primary
// range_min_bytes: 1048576
Expand All @@ -180,6 +188,7 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 1
// constraints: [+zone=us-east-1a, +ssd]
// lease_preferences: []
// zone ls
// .default
// .liveness
Expand All @@ -188,7 +197,6 @@ func Example_cclzone() {
// db.t@primary
// system.jobs
// zone rm db.t@primary
// CONFIGURE ZONE 1
// zone get db.t.p0
// .default
// range_min_bytes: 1048576
Expand All @@ -197,6 +205,7 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 1
// constraints: []
// lease_preferences: []
// zone get db.t.p1
// db.t.p1
// range_min_bytes: 1048576
Expand All @@ -205,16 +214,15 @@ func Example_cclzone() {
// ttlseconds: 90000
// num_replicas: 3
// constraints: [+zone=us-east-1a, +ssd]
// lease_preferences: []
// zone ls
// .default
// .liveness
// .meta
// db.t.p1
// system.jobs
// zone rm db.t.p0
// CONFIGURE ZONE 0
// zone rm db.t.p1
// CONFIGURE ZONE 1
// zone ls
// .default
// .liveness
Expand All @@ -228,6 +236,7 @@ func Example_cclzone() {
// num_replicas: 3
// constraints: {+region=us-east-1: 1, '+zone=us-east-1a,+ssd': 1}
// lease_preferences: [[+region=us-east-1], [+zone=us-east-1a]]
//
// zone set db.t@primary --file=./../../cli/testdata/zone_attrs_experimental.yaml
// range_min_bytes: 1048576
// range_max_bytes: 67108864
Expand Down
30 changes: 15 additions & 15 deletions pkg/ccl/partitionccl/partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (t *partitioningTest) parse() error {
subzone.IndexID = uint32(idxDesc.ID)
if len(constraints) > 0 {
fmt.Fprintf(&zoneConfigStmts,
`ALTER INDEX %s@%s EXPERIMENTAL CONFIGURE ZONE 'constraints: [%s]';`,
`ALTER INDEX %s@%s CONFIGURE ZONE USING constraints = '[%s]';`,
t.parsed.tableName, idxDesc.Name, constraints,
)
}
Expand All @@ -170,7 +170,7 @@ func (t *partitioningTest) parse() error {
subzone.IndexID = uint32(index.ID)
if len(constraints) > 0 {
fmt.Fprintf(&zoneConfigStmts,
`ALTER PARTITION %s OF TABLE %s EXPERIMENTAL CONFIGURE ZONE 'constraints: [%s]';`,
`ALTER PARTITION %s OF TABLE %s CONFIGURE ZONE USING constraints = '[%s]';`,
subzone.PartitionName, t.parsed.tableName, constraints,
)
}
Expand Down Expand Up @@ -1303,7 +1303,7 @@ func TestRepartitioning(t *testing.T) {
for _, name := range test.new.parsed.tableDesc.PartitionNames() {
newPartitionNames[name] = struct{}{}
}
rows := sqlDB.QueryStr(t, "SELECT cli_specifier FROM [EXPERIMENTAL SHOW ALL ZONE CONFIGURATIONS] WHERE cli_specifier IS NOT NULL")
rows := sqlDB.QueryStr(t, "SELECT cli_specifier FROM [SHOW ALL ZONE CONFIGURATIONS] WHERE cli_specifier IS NOT NULL")
for _, row := range rows {
zs, err := config.ParseCLIZoneSpecifier(row[0])
if err != nil {
Expand Down Expand Up @@ -1358,10 +1358,10 @@ func TestRemovePartitioningExpiredLicense(t *testing.T) {
sqlDB.Exec(t, `CREATE INDEX i ON t (a) PARTITION BY RANGE (a) (
PARTITION p34 VALUES FROM (3) TO (4)
)`)
sqlDB.Exec(t, `ALTER PARTITION p1 OF TABLE t EXPERIMENTAL CONFIGURE ZONE ''`)
sqlDB.Exec(t, `ALTER PARTITION p34 OF TABLE t EXPERIMENTAL CONFIGURE ZONE ''`)
sqlDB.Exec(t, `ALTER INDEX t@primary EXPERIMENTAL CONFIGURE ZONE ''`)
sqlDB.Exec(t, `ALTER INDEX t@i EXPERIMENTAL CONFIGURE ZONE ''`)
sqlDB.Exec(t, `ALTER PARTITION p1 OF TABLE t CONFIGURE ZONE USING DEFAULT`)
sqlDB.Exec(t, `ALTER PARTITION p34 OF TABLE t CONFIGURE ZONE USING DEFAULT`)
sqlDB.Exec(t, `ALTER INDEX t@primary CONFIGURE ZONE USING DEFAULT`)
sqlDB.Exec(t, `ALTER INDEX t@i CONFIGURE ZONE USING DEFAULT`)

// Remove the enterprise license.
defer utilccl.TestingDisableEnterprise()()
Expand All @@ -1377,20 +1377,20 @@ func TestRemovePartitioningExpiredLicense(t *testing.T) {
// Partitions and zone configs cannot be modified without a valid license.
expectLicenseErr(`ALTER TABLE t PARTITION BY LIST (a) (PARTITION p2 VALUES IN (2))`)
expectLicenseErr(`ALTER INDEX t@i PARTITION BY RANGE (a) (PARTITION p45 VALUES FROM (4) TO (5))`)
expectLicenseErr(`ALTER PARTITION p1 OF TABLE t EXPERIMENTAL CONFIGURE ZONE ''`)
expectLicenseErr(`ALTER PARTITION p34 OF TABLE t EXPERIMENTAL CONFIGURE ZONE ''`)
expectLicenseErr(`ALTER INDEX t@primary EXPERIMENTAL CONFIGURE ZONE ''`)
expectLicenseErr(`ALTER INDEX t@i EXPERIMENTAL CONFIGURE ZONE ''`)
expectLicenseErr(`ALTER PARTITION p1 OF TABLE t CONFIGURE ZONE USING DEFAULT`)
expectLicenseErr(`ALTER PARTITION p34 OF TABLE t CONFIGURE ZONE USING DEFAULT`)
expectLicenseErr(`ALTER INDEX t@primary CONFIGURE ZONE USING DEFAULT`)
expectLicenseErr(`ALTER INDEX t@i CONFIGURE ZONE USING DEFAULT`)

// But they can be removed.
sqlDB.Exec(t, `ALTER TABLE t PARTITION BY NOTHING`)
sqlDB.Exec(t, `ALTER INDEX t@i PARTITION BY NOTHING`)
sqlDB.Exec(t, `ALTER INDEX t@primary EXPERIMENTAL CONFIGURE ZONE NULL`)
sqlDB.Exec(t, `ALTER INDEX t@i EXPERIMENTAL CONFIGURE ZONE NULL`)
sqlDB.Exec(t, `ALTER INDEX t@primary CONFIGURE ZONE DISCARD`)
sqlDB.Exec(t, `ALTER INDEX t@i CONFIGURE ZONE DISCARD`)

// Once removed, they cannot be added back.
expectLicenseErr(`ALTER TABLE t PARTITION BY LIST (a) (PARTITION p2 VALUES IN (2))`)
expectLicenseErr(`ALTER INDEX t@i PARTITION BY RANGE (a) (PARTITION p45 VALUES FROM (4) TO (5))`)
expectLicenseErr(`ALTER INDEX t@primary EXPERIMENTAL CONFIGURE ZONE ''`)
expectLicenseErr(`ALTER INDEX t@i EXPERIMENTAL CONFIGURE ZONE ''`)
expectLicenseErr(`ALTER INDEX t@primary CONFIGURE ZONE USING DEFAULT`)
expectLicenseErr(`ALTER INDEX t@i CONFIGURE ZONE USING DEFAULT`)
}
Loading

0 comments on commit 47c7bfe

Please sign in to comment.