sql: block ALTER TABLE LOCALITY to RBR with sql_safe_updates

A bug was recently discovered that causes UPDATE and DELETE operations
to fail on a table while it is being converted to REGIONAL BY ROW.

To protect against this, this patch blocks the ALTER TABLE LOCALiTY
statement if the sql_safe_updates flag is on. The error hint provides a
workaround to safely convert the locality.

Release note (sql change): When sql_safe_updates is enabled, the ALTER
TABLE LOCALITY statement will now be blocked when trying to convert an
existing table to REGIONAL BY ROW, unless a region column has already
been added to the table. This protects against undesired behavior that
caused UPDATE or DELETE queries to fail against the table while the
locality change is in progress.

Author: rafiss

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_safe_updates

@@ -0,0 +1,96 @@
+# LogicTest: multiregion-9node-3region-3azs
+#
+# Test for blocking ALTER TABLE LOCALITY to REGIONAL BY ROW with sql_safe_updates enabled
+# This addresses issue #150945 to prevent DML failures caused by the legacy schema
+# changer's simultaneous column addition and primary key alteration.
+
+statement ok
+CREATE DATABASE safe_updates_test_db PRIMARY REGION "us-east-1" REGIONS "ap-southeast-2", "ca-central-1" SURVIVE REGION FAILURE
+
+statement ok
+USE safe_updates_test_db
+
+# Verify safe updates blocks REGIONAL BY ROW conversion for existing tables.
+subtest blocked_by_flag
+
+statement ok
+CREATE TABLE t1 (id UUID PRIMARY KEY, s STRING NOT NULL) WITH (schema_locked=false)
+
+statement ok
+INSERT INTO t1 SELECT gen_random_uuid(), 'test' FROM generate_series(1, 10)
+
+statement ok
+SET sql_safe_updates = true
+
+statement error pq: cannot convert table to REGIONAL BY ROW with sql_safe_updates enabled\nHINT:.*three-step workaround:\n.*1\. ALTER TABLE t1 ADD COLUMN crdb_region.*\n.*2\. ALTER TABLE t1 ALTER COLUMN crdb_region SET DEFAULT.*\n.*3\. ALTER TABLE t1 SET LOCALITY REGIONAL BY ROW;
+ALTER TABLE t1 SET LOCALITY REGIONAL BY ROW
+
+subtest end
+
+# Verify the operation works when safe updates are disabled.
+subtest works_with_flag_off
+
+statement ok
+SET sql_safe_updates = false
+
+statement ok
+ALTER TABLE t1 SET LOCALITY REGIONAL BY ROW
+
+statement ok
+SET sql_safe_updates = true
+
+# Verify DML operations work correctly after safe conversion
+query T
+SELECT DISTINCT s FROM t1
+----
+test
+
+statement ok
+UPDATE t1 SET s = 'updated' WHERE s = 'test'
+
+query T
+SELECT DISTINCT s FROM t1
+----
+updated
+
+subtest end
+
+# Verify the safe 3-step workaround works correctly.
+subtest not_blocked_if_column_exists_already
+
+statement ok
+SET sql_safe_updates = true
+
+statement ok
+CREATE TABLE t2 (id UUID PRIMARY KEY, s STRING NOT NULL) WITH (schema_locked=false)
+
+statement ok
+INSERT INTO t2 SELECT gen_random_uuid(), 'test' FROM generate_series(1, 10)
+
+# Step 1: Add the region column manually.
+statement ok
+ALTER TABLE t2 ADD COLUMN crdb_region public.crdb_internal_region NOT VISIBLE NOT NULL DEFAULT 'us-east-1'::public.crdb_internal_region
+
+# Step 2: Update the default expression to use gateway_region().
+statement ok
+ALTER TABLE t2 ALTER COLUMN crdb_region SET DEFAULT default_to_database_primary_region(gateway_region())::public.crdb_internal_region
+
+# Step 3: Convert to REGIONAL BY ROW (should work now).
+statement ok
+ALTER TABLE t2 SET LOCALITY REGIONAL BY ROW
+
+# Verify DML operations work after manual workaround
+query T
+SELECT DISTINCT s FROM t2
+----
+test
+
+statement ok
+UPDATE t2 SET s = 'updated_safe' WHERE s = 'test'
+
+query T
+SELECT DISTINCT s FROM t2
+----
+updated_safe
+
+subtest end

pkg/ccl/logictestccl/tests/multiregion-9node-3region-3azs/BUILD.bazel

@@ -9,7 +9,7 @@ go_test(
         "//pkg/ccl/logictestccl:testdata",  # keep
     ],
     exec_properties = {"test.Pool": "large"},
-    shard_count = 31,
+    shard_count = 32,
     tags = ["cpu:4"],
     deps = [
         "//pkg/base",

pkg/ccl/logictestccl/tests/multiregion-9node-3region-3azs/generated_test.go

@@ -281,6 +281,32 @@ func (n *alterTableSetLocalityNode) alterTableLocalityToRegionalByRow(
 			)
 		}
 	} else {
+		// Block REGIONAL BY ROW conversion when safe updates are enabled to prevent
+		// DML failures caused by the legacy schema changer's simultaneous column
+		// addition and primary key alteration.
+		// When this operation is implemented in the declarative schema changer, we
+		// should not need this check since the declarative schema changer should
+		// handle the element transitions correctly.
+		if params.p.SessionData().SafeUpdates && !n.tableDesc.Adding() {
+			primaryRegion, err := n.dbDesc.PrimaryRegionName()
+			if err != nil {
+				return err
+			}
+
+			return errors.WithHintf(
+				pgerror.Newf(
+					pgcode.InvalidTableDefinition,
+					"cannot convert table to REGIONAL BY ROW with sql_safe_updates enabled",
+				),
+				"To safely convert to REGIONAL BY ROW, either disable sql_safe_updates; or "+
+					"use the following three-step workaround:\n"+
+					"  1. ALTER TABLE %[1]s ADD COLUMN %[2]s public.crdb_internal_region NOT VISIBLE NOT NULL DEFAULT '%[3]s'::public.crdb_internal_region;\n"+
+					"  2. ALTER TABLE %[1]s ALTER COLUMN %[2]s SET DEFAULT default_to_database_primary_region(gateway_region())::public.crdb_internal_region;\n"+
+					"  3. ALTER TABLE %[1]s SET LOCALITY REGIONAL BY ROW;",
+				tree.Name(n.tableDesc.GetName()), partColName, primaryRegion,
+			)
+		}
+
 		// No crdb_region column is found so we are implicitly creating it.
 		// We insert the column definition before altering the primary key.