Merge pull request #140609 from cockroachdb/blathers/backport-release-25.1-140066

release-25.1: server: do not use MustBeDString on nullable result col

Author: xinhaoz

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 {

pkg/server/application_api/storage_inspection_test.go

@@ -197,7 +197,7 @@ func TestAdminAPIDataDistribution(t *testing.T) {
 
 	firstServer := tc.Server(0).ApplicationLayer()
 
-	sqlDB := sqlutils.MakeSQLRunner(tc.ServerConn(0))
+	sqlDB := sqlutils.MakeSQLRunner(firstServer.SQLConn(t))
 
 	{
 		// TODO(irfansharif): The data-distribution page and underyling APIs don't
@@ -217,6 +217,11 @@ func TestAdminAPIDataDistribution(t *testing.T) {
 		post_id INT REFERENCES roachblog.posts,
 		body text
 	)`)
+
+	// Test for null raw sql config column in crdb_internal.zones,
+	// see: https://github.com/cockroachdb/cockroach/issues/140044
+	//sqlDB.Exec(t, `-- ALTER TABLE roachblog.posts CONFIGURE ZONE = ''`)
+
 	sqlDB.Exec(t, `CREATE SCHEMA roachblog."foo bar"`)
 	sqlDB.Exec(t, `CREATE TABLE roachblog."foo bar".other_stuff(id INT PRIMARY KEY, body TEXT)`)
 	// Test special characters in DB and table names.
@@ -265,31 +270,28 @@ func TestAdminAPIDataDistribution(t *testing.T) {
 		},
 	}
 
-	// Wait for the new tables' ranges to be created and replicated.
-	testutils.SucceedsSoon(t, func() error {
-		var resp serverpb.DataDistributionResponse
-		if err := srvtestutils.GetAdminJSONProto(firstServer, "data_distribution", &resp); err != nil {
-			t.Fatal(err)
-		}
+	require.NoError(t, tc.WaitForFullReplication())
 
-		delete(resp.DatabaseInfo, "system") // delete results for system database.
-		if !reflect.DeepEqual(resp.DatabaseInfo, expectedDatabaseInfo) {
-			return fmt.Errorf("expected %v; got %v", expectedDatabaseInfo, resp.DatabaseInfo)
-		}
+	// Wait for the new tables' ranges to be created and replicated.
+	var resp serverpb.DataDistributionResponse
+	if err := srvtestutils.GetAdminJSONProto(firstServer, "data_distribution", &resp); err != nil {
+		t.Fatal(err)
+	}
 
-		// Don't test anything about the zone configs for now; just verify that something is there.
-		if len(resp.ZoneConfigs) == 0 {
-			return fmt.Errorf("no zone configs returned")
-		}
+	delete(resp.DatabaseInfo, "system") // delete results for system database.
+	if !reflect.DeepEqual(resp.DatabaseInfo, expectedDatabaseInfo) {
+		t.Fatal("unexpected data distribution response", resp.DatabaseInfo)
+		//return fmt.Errorf("expected %v; got %v", expectedDatabaseInfo, resp.DatabaseInfo)
+	}
 
-		return nil
-	})
+	// Don't test anything about the zone configs for now; just verify that something is there.
+	require.NotEmpty(t, resp.ZoneConfigs)
 
 	// Verify that the request still works after a table has been dropped,
 	// and that dropped_at is set on the dropped table.
 	sqlDB.Exec(t, `DROP TABLE roachblog.comments`)
 
-	var resp serverpb.DataDistributionResponse
+	//var resp serverpb.DataDistributionResponse
 	if err := srvtestutils.GetAdminJSONProto(firstServer, "data_distribution", &resp); err != nil {
 		t.Fatal(err)
 	}