From 5b19543f27708b8678bc6be92543a773829fa22a Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Fri, 13 Nov 2020 10:25:41 +1100 Subject: [PATCH] sql: rework SHOW REGIONS to SHOW REGIONS FROM CLUSTER Release note (sql change): SHOW REGIONS functionality is now deferred to SHOW REGIONS FROM CLUSTER. --- docs/generated/sql/bnf/stmt_block.bnf | 2 +- .../interactive_tests/test_demo_node_cmds.tcl | 2 +- pkg/sql/delegate/show_regions.go | 13 +++++++- .../logictest/testdata/logic_test/multiregion | 2 +- pkg/sql/parser/parse_test.go | 2 +- pkg/sql/parser/sql.y | 5 +-- pkg/sql/sem/tree/show.go | 6 ++-- pkg/sql/sqltelemetry/show.go | 31 ++++++++++--------- 8 files changed, 40 insertions(+), 23 deletions(-) diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 53a460beed56..e759d3330ff3 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -664,7 +664,7 @@ show_range_for_row_stmt ::= | 'SHOW' 'RANGE' 'FROM' 'INDEX' table_index_name 'FOR' 'ROW' '(' expr_list ')' show_regions_stmt ::= - 'SHOW' 'REGIONS' + 'SHOW' 'REGIONS' 'FROM' 'CLUSTER' | 'SHOW' 'REGIONS' 'FROM' 'DATABASE' database_name show_roles_stmt ::= diff --git a/pkg/cli/interactive_tests/test_demo_node_cmds.tcl b/pkg/cli/interactive_tests/test_demo_node_cmds.tcl index 0b4827326185..81c1cef41cb3 100644 --- a/pkg/cli/interactive_tests/test_demo_node_cmds.tcl +++ b/pkg/cli/interactive_tests/test_demo_node_cmds.tcl @@ -82,7 +82,7 @@ eexpect "internal server error: tier must be in the form \"key=value\" not \"bla send "\\demo add region=ca-central,zone=a\r" eexpect "node 6 has been added with locality \"region=ca-central,zone=a\"" -send "show regions;\r" +send "show regions from cluster;\r" eexpect "ca-central | \{a\}" eexpect "us-east1 | \{b,c,d\}" eexpect "us-west1 | \{a,b\}" diff --git a/pkg/sql/delegate/show_regions.go b/pkg/sql/delegate/show_regions.go index 98274b54cc18..1b514e0079c5 100644 --- a/pkg/sql/delegate/show_regions.go +++ b/pkg/sql/delegate/show_regions.go @@ -13,11 +13,22 @@ package delegate import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" ) // delegateShowRanges implements the SHOW REGIONS statement. func (d *delegator) delegateShowRegions(n *tree.ShowRegions) (tree.Statement, error) { - sqltelemetry.IncrementShowCounter(sqltelemetry.Regions) + + if n.Database != "" { + sqltelemetry.IncrementShowCounter(sqltelemetry.RegionsFromDatabase) + return nil, unimplemented.New( + "show regions from database", + "SHOW REGIONS FROM DATABASE not yet implemented", + ) + } else { + sqltelemetry.IncrementShowCounter(sqltelemetry.RegionsFromCluster) + } + // TODO (storm): Change this so that it doesn't use hard-coded strings and is // more flexible for custom named sub-regions. query := ` diff --git a/pkg/sql/logictest/testdata/logic_test/multiregion b/pkg/sql/logictest/testdata/logic_test/multiregion index b592e64bf530..3363ff8cd18d 100644 --- a/pkg/sql/logictest/testdata/logic_test/multiregion +++ b/pkg/sql/logictest/testdata/logic_test/multiregion @@ -1,7 +1,7 @@ # LogicTest: multiregion-9node-3region-3azs query TT colnames -SHOW REGIONS +SHOW REGIONS FROM CLUSTER ---- region zones test1 {test1-az1,test1-az2,test1-az3} diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go index 22420c8deb5f..2f18400c5115 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -645,7 +645,7 @@ func TestParse(t *testing.T) { {`SHOW RANGES FROM INDEX t@i`}, {`SHOW RANGES FROM INDEX d.i`}, {`SHOW RANGES FROM INDEX i`}, - {`SHOW REGIONS`}, + {`SHOW REGIONS FROM CLUSTER`}, {`SHOW REGIONS FROM DATABASE d`}, {`SHOW EXPERIMENTAL_FINGERPRINTS FROM TABLE d.t`}, {`SHOW ZONE CONFIGURATIONS`}, diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index d1b35dfee8bb..d61d817a667f 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -5124,10 +5124,10 @@ show_ranges_stmt: // %Help: SHOW REGIONS - shows regions // %Category: DDL // %Text: -// SHOW REGIONS +// SHOW REGIONS CLUSTER // SHOW REGIONS FOR DATABASE show_regions_stmt: - SHOW REGIONS + SHOW REGIONS FROM CLUSTER { $$.val = &tree.ShowRegions{} } @@ -5137,6 +5137,7 @@ show_regions_stmt: Database: tree.Name($5), } } +| SHOW REGIONS error // SHOW HELP: SHOW REGIONS show_locality_stmt: SHOW LOCALITY diff --git a/pkg/sql/sem/tree/show.go b/pkg/sql/sem/tree/show.go index 4d1faacc96ef..240845ab7343 100644 --- a/pkg/sql/sem/tree/show.go +++ b/pkg/sql/sem/tree/show.go @@ -285,10 +285,12 @@ type ShowRegions struct { // Format implements the NodeFormatter interface. func (node *ShowRegions) Format(ctx *FmtCtx) { - ctx.WriteString("SHOW REGIONS") + ctx.WriteString("SHOW REGIONS ") if node.Database != "" { - ctx.WriteString(" FROM DATABASE ") + ctx.WriteString("FROM DATABASE ") node.Database.Format(ctx) + } else { + ctx.WriteString("FROM CLUSTER") } } diff --git a/pkg/sql/sqltelemetry/show.go b/pkg/sql/sqltelemetry/show.go index ec7a8dc4044a..b0fdb471a42b 100644 --- a/pkg/sql/sqltelemetry/show.go +++ b/pkg/sql/sqltelemetry/show.go @@ -24,8 +24,10 @@ const ( _ ShowTelemetryType = iota // Ranges represents the SHOW RANGES command. Ranges - // Regions represents the SHOW REGIONS command. - Regions + // Regions represents the SHOW REGIONS FROM CLUSTER command. + RegionsFromCluster + // Regions represents the SHOW REGIONS FROM DATABASE command. + RegionsFromDatabase // Partitions represents the SHOW PARTITIONS command. Partitions // Locality represents the SHOW LOCALITY command. @@ -49,18 +51,19 @@ const ( ) var showTelemetryNameMap = map[ShowTelemetryType]string{ - Ranges: "ranges", - Partitions: "partitions", - Locality: "locality", - Create: "create", - RangeForRow: "rangeforrow", - Regions: "regions", - Queries: "queries", - Indexes: "indexes", - Constraints: "constraints", - Jobs: "jobs", - Roles: "roles", - Schedules: "schedules", + Ranges: "ranges", + Partitions: "partitions", + Locality: "locality", + Create: "create", + RangeForRow: "rangeforrow", + RegionsFromCluster: "regions_from_cluster", + RegionsFromDatabase: "regions_from_database", + Queries: "queries", + Indexes: "indexes", + Constraints: "constraints", + Jobs: "jobs", + Roles: "roles", + Schedules: "schedules", } func (s ShowTelemetryType) String() string {