nit: let's remove the if isRBR check here. panicIfRegionChangeUnderwayOnRBRTable already has that check in it.

also is the idx != nil check too broad? previously, we'd only call panicIfRegionChangeUnderwayOnRBRTable if a UNIQUE column was being added.

should we also have a test with SURVIVE ZONE FAILURE, or do you feel like this is sufficient?

re:nit -- ah oki right

re: broadness of idx != nil check -- i aligned this with legacy's implementation

cockroach/pkg/sql/add_column.go

Lines 79 to 92 in 42b9060

	idx := cdd.PrimaryKeyOrUniqueIndexDescriptor
	incTelemetryForNewColumn(d, col)
	// Ensure all new indexes are partitioned appropriately.
	if idx != nil {
	if n.tableDesc.IsLocalityRegionalByRow() {
	if err := params.p.checkNoRegionChangeUnderway(
	params.ctx,
	n.tableDesc.GetParentID(),
	"add a UNIQUE COLUMN on a REGIONAL BY ROW table",
	); err != nil {
	return err
	}
	}

; iiuc this should still be equivalent since primary keys are also unique, but lmk if that's not what it going on here

cockroach/pkg/sql/catalog/tabledesc/table.go

Lines 42 to 44 in 4ab689e

	// PrimaryKeyOrUniqueIndexDescriptor is the index descriptor for the index implied by a
	// PRIMARY KEY or UNIQUE column.
	PrimaryKeyOrUniqueIndexDescriptor *descpb.IndexDescriptor

i feel like this is sufficient -- at least for a DSC end_to_end test, but what were you thinking about testing? the zone config generated from a SURVIVE ZONE FAILURE (which is code i touched, but by porting it over to a new pkg) exists:

cockroach/pkg/ccl/logictestccl/testdata/logic_test/multi_region

Lines 283 to 293 in 6ebb172

	SHOW ZONE CONFIGURATION FOR DATABASE multi_region_test_survive_zone_failure_db
	----
	DATABASE multi_region_test_survive_zone_failure_db ALTER DATABASE multi_region_test_survive_zone_failure_db CONFIGURE ZONE USING
	range_min_bytes = 134217728,
	range_max_bytes = 536870912,
	gc.ttlseconds = 14400,
	num_replicas = 5,
	num_voters = 3,
	constraints = '{+region=ap-southeast-2: 1, +region=ca-central-1: 1, +region=us-east-1: 1}',
	voter_constraints = '[+region=us-east-1]',
	lease_preferences = '[[+region=us-east-1]]'

if this was what you were thinking about

that is basically all i had in mind. i'm guessing the schema change plan isn't that different whether the database has REGION FAILURE or ZONE FAILURE then?