Skip to content

Commit

Permalink
Merge #40501 #40519
Browse files Browse the repository at this point in the history
40501: sql: Reorder show ranges output to be clearer r=rohany a=rohany

The output of show ranges had column orderings that at a quick glance
could lead to users making the wrong conclusions about their
partitioning setup. This PR adjusts the columns and adds more readily
accessible information about the lease_holder node's locality.

Fixes #40467.

Release note (sql change): Reorder columns in show ranges output

40519: workload: ignore syntax error on INJECT STATISTICS against v2.0 r=nvanbenschoten a=nvanbenschoten

Fixes #40463.

Release note: None

Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
  • Loading branch information
3 people committed Sep 5, 2019
3 parents 01c7cf5 + 5f8a9d7 + f06944b commit 55c33fd
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 13 deletions.
15 changes: 12 additions & 3 deletions pkg/ccl/workloadccl/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"net/url"
"path/filepath"
"strconv"
"strings"
"sync/atomic"

"cloud.google.com/go/storage"
Expand Down Expand Up @@ -495,9 +496,17 @@ func injectStatistics(qualifiedTableName string, table *workload.Table, sqlDB *g
if err != nil {
return err
}
_, err = sqlDB.Exec(fmt.Sprintf(
`ALTER TABLE %s INJECT STATISTICS '%s'`, qualifiedTableName, encoded))
return err
if _, err := sqlDB.Exec(
fmt.Sprintf(`ALTER TABLE %s INJECT STATISTICS '%s'`, qualifiedTableName, encoded),
); err != nil {
if strings.Contains(err.Error(), "syntax error") {
// This syntax was added in v2.1, so ignore the syntax error
// if run against versions earlier than this.
return nil
}
return err
}
return nil
}

// makeQualifiedTableName constructs a qualified table name from the specified
Expand Down
10 changes: 7 additions & 3 deletions pkg/sql/delegate/show_ranges.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ func (d *delegator) delegateShowRanges(n *tree.ShowRanges) (tree.Statement, erro
END AS end_key,
range_id,
range_size / 1000000 as range_size_mb,
replicas,
lease_holder,
replica_localities,
gossip_nodes.locality as lease_holder_locality,
replicas,
replica_localities
FROM %[1]s.crdb_internal.ranges AS r
LEFT JOIN crdb_internal.gossip_nodes ON lease_holder = node_id
WHERE database_name=%[2]s
ORDER BY table_name, r.start_key
`
Expand All @@ -71,10 +73,12 @@ SELECT
CASE WHEN r.end_key >= x'%s' THEN NULL ELSE crdb_internal.pretty_key(r.end_key, 2) END AS end_key,
range_id,
range_size / 1000000 as range_size_mb,
replicas,
lease_holder,
gossip_nodes.locality as lease_holder_locality,
replicas,
replica_localities
FROM crdb_internal.ranges AS r
LEFT JOIN crdb_internal.gossip_nodes ON lease_holder = node_id
WHERE (r.start_key < x'%s')
AND (r.end_key > x'%s') ORDER BY r.start_key
`,
Expand Down
23 changes: 16 additions & 7 deletions pkg/sql/show_ranges_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,29 @@ func TestShowRangesWithLocality(t *testing.T) {
sqlDB.Exec(t, `CREATE TABLE t (x INT PRIMARY KEY)`)
sqlDB.Exec(t, `ALTER TABLE t SPLIT AT SELECT i FROM generate_series(0, 20) AS g(i)`)

const replicasColIdx = 4
const localitiesColIdx = 6
const leaseHolderIdx = 0
const leaseHolderLocalityIdx = 1
const replicasColIdx = 2
const localitiesColIdx = 3
replicas := make([]int, 3)

result := sqlDB.QueryStr(t, `SHOW RANGES FROM TABLE t`)
q := `SELECT lease_holder, lease_holder_locality, replicas, replica_localities from [SHOW RANGES FROM TABLE t]`
result := sqlDB.QueryStr(t, q)
// Because StartTestCluster changes the locality no matter what the
// arguments are, we expect whatever the test server sets up.
for _, row := range result {
// Verify the leaseholder localities.
leaseHolder := row[leaseHolderIdx]
leaseHolderLocalityExpected := fmt.Sprintf(`region=test,dc=dc%s`, leaseHolder)
if row[leaseHolderLocalityIdx] != leaseHolderLocalityExpected {
t.Fatalf("expected %s found %s", leaseHolderLocalityExpected, row[leaseHolderLocalityIdx])
}

// Verify the replica localities.
_, err := fmt.Sscanf(row[replicasColIdx], "{%d,%d,%d}", &replicas[0], &replicas[1], &replicas[2])
if err != nil {
t.Fatal(err)
}

// Because StartTestCluster changes the locality no matter what the
// arguments are, we expect whatever the test server sets up.
var builder strings.Builder
builder.WriteString("{")
for i, replica := range replicas {
Expand All @@ -59,7 +69,6 @@ func TestShowRangesWithLocality(t *testing.T) {
}
builder.WriteString("}")
expected := builder.String()

if row[localitiesColIdx] != expected {
t.Fatalf("expected %s found %s", expected, row[localitiesColIdx])
}
Expand Down

0 comments on commit 55c33fd

Please sign in to comment.