From 9e732f778178c836c8d925be26b7ab4dc143b615 Mon Sep 17 00:00:00 2001
From: Marius Posta <marius@cockroachlabs.com>
Date: Mon, 29 Mar 2021 11:45:23 -0400
Subject: [PATCH] sql: fix error when adding and dropping a constraint in same
 txn.

This commit fixes a bug in which an internal error would be returned
when dropping a foreign key currently in the table's mutations slice.
Instead, we return a more appropriate unimplemented error.

Fixes #60786.

Release note (bug fix): Dropping a foreign key that was added in the
same transaction no longer triggers an internal error. This bug has been
present since at least version 20.1.
---
 pkg/sql/catalog/tabledesc/structured.go       | 19 +++++++++++++--
 .../logictest/testdata/logic_test/alter_table | 23 +++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/pkg/sql/catalog/tabledesc/structured.go b/pkg/sql/catalog/tabledesc/structured.go
index e4ff121b2366..8484ae295ca9 100644
--- a/pkg/sql/catalog/tabledesc/structured.go
+++ b/pkg/sql/catalog/tabledesc/structured.go
@@ -2816,7 +2816,6 @@ func (desc *Mutable) DropConstraint(
 				return nil
 			}
 		}
-		return errors.Errorf("constraint %q not found on table %q", name, desc.Name)
 
 	case descpb.ConstraintTypeFK:
 		if detail.FK.Validity == descpb.ConstraintValidity_Validating {
@@ -2849,13 +2848,29 @@ func (desc *Mutable) DropConstraint(
 				return nil
 			}
 		}
-		return errors.AssertionFailedf("constraint %q not found on table %q", name, desc.Name)
 
 	default:
 		return unimplemented.Newf(fmt.Sprintf("drop-constraint-%s", detail.Kind),
 			"constraint %q has unsupported type", tree.ErrNameString(name))
 	}
 
+	// Check if the constraint can be found in a mutation, complain appropriately.
+	for i := range desc.Mutations {
+		m := &desc.Mutations[i]
+		if m.GetConstraint() != nil && m.GetConstraint().Name == name {
+			switch m.Direction {
+			case descpb.DescriptorMutation_ADD:
+				return unimplemented.NewWithIssueDetailf(42844,
+					"drop-constraint-mutation",
+					"constraint %q in the middle of being added, try again later", name)
+			case descpb.DescriptorMutation_DROP:
+				return unimplemented.NewWithIssueDetailf(42844,
+					"drop-constraint-mutation",
+					"constraint %q in the middle of being dropped", name)
+			}
+		}
+	}
+	return errors.AssertionFailedf("constraint %q not found on table %q", name, desc.Name)
 }
 
 // RenameConstraint renames a constraint.
diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table
index c8ed4e9d048e..b5aa1139eed2 100644
--- a/pkg/sql/logictest/testdata/logic_test/alter_table
+++ b/pkg/sql/logictest/testdata/logic_test/alter_table
@@ -1407,3 +1407,26 @@ CREATE TABLE t54629 (c INT NOT NULL, UNIQUE INDEX (c));
 ALTER TABLE t54629 ADD CONSTRAINT pk PRIMARY KEY (c);
 INSERT INTO t54629 VALUES (1);
 DELETE FROM t54629 WHERE c = 1;
+
+# Regression test for #60786. Handle in-transaction constraint ADD+DROP correctly.
+subtest regression_60786
+
+statement ok
+CREATE TABLE t60786(i INT PRIMARY KEY);
+
+statement error pgcode 0A000 constraint "fk" in the middle of being added, try again later
+BEGIN;
+CREATE TABLE child_60786(i INT PRIMARY KEY);
+ALTER TABLE t60786 ADD CONSTRAINT fk FOREIGN KEY (i) REFERENCES child_60786(i) NOT VALID;
+ALTER TABLE t60786 DROP CONSTRAINT fk CASCADE
+
+statement ok
+ROLLBACK
+
+statement error pgcode 0A000 constraint "ck" in the middle of being added, try again later
+BEGIN;
+ALTER TABLE t60786 ADD CONSTRAINT ck CHECK(i > 0) NOT VALID;
+ALTER TABLE t60786 DROP CONSTRAINT ck CASCADE
+
+statement ok
+ROLLBACK