From 9b1319a7d27c4bf12c58ece54688a6c9a2631fa0 Mon Sep 17 00:00:00 2001
From: Xin Hao Zhang <xzhang@cockroachlabs.com>
Date: Wed, 29 Jan 2025 15:51:51 -0500
Subject: [PATCH] server: do not use MustBeDString on nullable result col

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`.
---
 pkg/server/admin.go                                  |  7 +++++--
 .../application_api/storage_inspection_test.go       | 12 ++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/pkg/server/admin.go b/pkg/server/admin.go
index 4bef00350dfd..fa246f59551c 100644
--- a/pkg/server/admin.go
+++ b/pkg/server/admin.go
@@ -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 {
@@ -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 {
diff --git a/pkg/server/application_api/storage_inspection_test.go b/pkg/server/application_api/storage_inspection_test.go
index 35189a3b46ba..f8ed67ba5120 100644
--- a/pkg/server/application_api/storage_inspection_test.go
+++ b/pkg/server/application_api/storage_inspection_test.go
@@ -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.
 
@@ -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`: {