Skip to content

Commit

Permalink
server: do not use MustBeDString on nullable result col
Browse files Browse the repository at this point in the history
In the data distribution handler we were attempting to read a
`raw_sql_config` on `crdb_internal.zones` using `MustBeDString`
which panics if the value is null.  This column is nullable.
We now allow null values to be read and make the response
value an empty string in that case.

Fixes: #140044

Release note (bug fix):  Data distribution page in
advanced debug will no longer crash if there are null
values for `raw_sql_config` in `crdb_internal.zones`.
  • Loading branch information
xinhaoz committed Jan 31, 2025
1 parent 712e9b2 commit 9b1319a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
7 changes: 5 additions & 2 deletions pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -3054,7 +3054,10 @@ func (s *adminServer) dataDistributionHelper(
for hasNext, err = it.Next(ctx); hasNext; hasNext, err = it.Next(ctx) {
row := it.Cur()
target := string(tree.MustBeDString(row[0]))
zcSQL := tree.MustBeDString(row[1])
var zcSQL string
if zcSQLDatum, ok := tree.AsDString(row[1]); ok {
zcSQL = string(zcSQLDatum)
}
zcBytes := tree.MustBeDBytes(row[2])
var zcProto zonepb.ZoneConfig
if err := protoutil.Unmarshal([]byte(zcBytes), &zcProto); err != nil {
Expand All @@ -3064,7 +3067,7 @@ func (s *adminServer) dataDistributionHelper(
resp.ZoneConfigs[target] = serverpb.DataDistributionResponse_ZoneConfig{
Target: target,
Config: zcProto,
ConfigSQL: string(zcSQL),
ConfigSQL: zcSQL,
}
}
if err != nil {
Expand Down
12 changes: 12 additions & 0 deletions pkg/server/application_api/storage_inspection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,11 @@ func TestAdminAPIDataDistribution(t *testing.T) {
sqlDB.Exec(t, `CREATE DATABASE "sp'ec\ch""ars"`)
sqlDB.Exec(t, `CREATE TABLE "sp'ec\ch""ars"."more\spec'chars" (id INT PRIMARY KEY)`)

// Test for null raw sql config column in crdb_internal.zones,
// see: https://github.com/cockroachdb/cockroach/issues/140044
sqlDB.Exec(t, `CREATE TABLE t (i INT PRIMARY KEY)`)
sqlDB.Exec(t, `ALTER TABLE t CONFIGURE ZONE = ''`)

// Verify that we see their replicas in the DataDistribution response, evenly spread
// across the test cluster's three nodes.

Expand Down Expand Up @@ -250,6 +255,13 @@ func TestAdminAPIDataDistribution(t *testing.T) {
3: 1,
},
},
`public.t`: {
ReplicaCountByNodeId: map[roachpb.NodeID]int64{
1: 1,
2: 1,
3: 1,
},
},
},
},
`sp'ec\ch"ars`: {
Expand Down

0 comments on commit 9b1319a

Please sign in to comment.