Merge #157834

157834: sql: unset RBRUsingConstraint during rollback r=rafiss a=rafiss

Previously, if a rollback dropped a constraint due to a FK validation error, it would never update the RBRUsingConstraint field even if the constraint being rolled back was referenced by that field.

Now the field is cleared properly when needed.

fixes #157832

Release note (bug fix): Fixed a bug that could cause a schema change to be stuck in the reverting state if the infer_rbr_region_col_using_constraint storage parameter was being set at the same time as adding a constraint that had a foreign key violation.

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>

Author: craig[bot]

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row_foreign_key

@@ -2237,3 +2237,67 @@ DROP FUNCTION display_child;
 DROP TABLE child;
 
 subtest end
+
+# ==============================================================================
+# Verify that a foreign key violation causes the addition of the
+# rbr_using_constraint to be reverted properly.
+# ==============================================================================
+
+subtest revert_fk_violation
+
+statement ok
+SET create_table_with_schema_locked = false;
+
+statement ok
+CREATE TABLE a (
+  id uuid NOT NULL PRIMARY KEY
+) LOCALITY REGIONAL BY ROW
+
+statement ok
+CREATE TABLE b (
+  id uuid NOT NULL,
+  a_id uuid NULL,
+  CONSTRAINT b_pkey PRIMARY KEY (id ASC),
+  CONSTRAINT a_id_fk FOREIGN KEY (a_id) REFERENCES a (id)
+) LOCALITY REGIONAL BY ROW
+
+# Insert data that will cause a foreign key violation when we try to add the composite FK.
+statement ok
+INSERT INTO a (id, crdb_region) VALUES ('4227eceb-71d6-422c-808b-6a12ec8a1e54', 'us-east-1')
+
+statement ok
+INSERT INTO b (id, a_id, crdb_region) VALUES ('8f6ce660-423e-4823-8e41-bf001a007b46', '4227eceb-71d6-422c-808b-6a12ec8a1e54', 'ca-central-1')
+
+# This statement should fail with a foreign key violation, and the schema change
+# should properly revert without causing infinite retries due to the missing RBRUsingConstraint.
+# Prior to the bugfix, the rollback logic would would drop the constraint but
+# not clear the RBRUsingConstraint field, causing validation to fail during
+# the rollback.
+statement error pgcode 23503 pq: foreign key violation: "b" row crdb_region='ca-central-1', a_id='4227eceb-71d6-422c-808b-6a12ec8a1e54', id='8f6ce660-423e-4823-8e41-bf001a007b46' has no match in "a"
+ALTER TABLE b
+  DROP CONSTRAINT a_id_fk,
+  ADD CONSTRAINT a_crdb_region_id_fk
+    FOREIGN KEY (crdb_region, a_id) REFERENCES a (crdb_region, id) ON UPDATE CASCADE ON DELETE CASCADE,
+  SET (infer_rbr_region_col_using_constraint = a_crdb_region_id_fk)
+
+# Verify the tables are still functional after the failed schema change.
+query I
+SELECT count(*) FROM a
+----
+1
+
+query I
+SELECT count(*) FROM b
+----
+1
+
+statement ok
+DROP TABLE b CASCADE
+
+statement ok
+DROP TABLE a CASCADE
+
+statement ok
+RESET create_table_with_schema_locked;
+
+subtest end

pkg/sql/schema_changer.go

@@ -2531,6 +2531,11 @@ func (sc *SchemaChanger) maybeDropValidatingConstraint(
 			constraint.GetName(),
 		)
 	} else if constraint.AsForeignKey() != nil {
+		// If the constraint being dropped is the regional-by-row constraint,
+		// we must clear the reference to it.
+		if desc.RBRUsingConstraint == constraint.GetConstraintID() {
+			desc.RBRUsingConstraint = descpb.ConstraintID(0)
+		}
 		for i, fk := range desc.OutboundFKs {
 			if fk.Name == constraint.GetName() {
 				desc.OutboundFKs = append(desc.OutboundFKs[:i], desc.OutboundFKs[i+1:]...)