From 5346cd67c841f78cfb54476181a577efc7966e78 Mon Sep 17 00:00:00 2001
From: Oliver Tan <otan@cockroachlabs.com>
Date: Thu, 10 Sep 2020 18:39:33 -0700
Subject: [PATCH] sql: fix DROP COLUMN bug which ignores containing indexes

AllNonDropIndexes returns an array of pointers to index descriptors.
After deleting one of the indexes, the array of pointers point
to a different element. This ends up accidentally missing some
indexes that require deletion.

Release note (bug fix): Fixed a bug introduced in earlier v20.2 versions
where when attempting to drop a column which is referenced by multiple
indexes fails to drop all relevant indexes.
---
 pkg/sql/alter_table.go                        | 23 +++++++++++--------
 .../logictest/testdata/logic_test/alter_table | 20 ++++++++++++++++
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
index 0df77336674c..485c13d652fc 100644
--- a/pkg/sql/alter_table.go
+++ b/pkg/sql/alter_table.go
@@ -403,6 +403,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 				return pgerror.Newf(pgcode.InvalidColumnReference,
 					"column %q is referenced by the primary key", colToDrop.Name)
 			}
+			var idxNamesToDelete []string
 			for _, idx := range n.tableDesc.AllNonDropIndexes() {
 				// We automatically drop indexes that reference the column
 				// being dropped.
@@ -466,15 +467,19 @@ func (n *alterTableNode) startExec(params runParams) error {
 
 				// Perform the DROP.
 				if containsThisColumn {
-					jobDesc := fmt.Sprintf("removing index %q dependent on column %q which is being"+
-						" dropped; full details: %s", idx.Name, colToDrop.ColName(),
-						tree.AsStringWithFQNames(n.n, params.Ann()))
-					if err := params.p.dropIndexByName(
-						params.ctx, tn, tree.UnrestrictedName(idx.Name), n.tableDesc, false,
-						t.DropBehavior, ignoreIdxConstraint, jobDesc,
-					); err != nil {
-						return err
-					}
+					idxNamesToDelete = append(idxNamesToDelete, idx.Name)
+				}
+			}
+
+			for _, idxName := range idxNamesToDelete {
+				jobDesc := fmt.Sprintf("removing index %q dependent on column %q which is being"+
+					" dropped; full details: %s", idxName, colToDrop.ColName(),
+					tree.AsStringWithFQNames(n.n, params.Ann()))
+				if err := params.p.dropIndexByName(
+					params.ctx, tn, tree.UnrestrictedName(idxName), n.tableDesc, false,
+					t.DropBehavior, ignoreIdxConstraint, jobDesc,
+				); err != nil {
+					return err
 				}
 			}
 
diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table
index 4099d966f79f..fc125fd51800 100644
--- a/pkg/sql/logictest/testdata/logic_test/alter_table
+++ b/pkg/sql/logictest/testdata/logic_test/alter_table
@@ -1364,3 +1364,23 @@ statement ok
 CREATE TABLE t52816 (x INT, y INT);
 ALTER TABLE t52816 RENAME COLUMN x TO x2, RENAME COLUMN y TO y;
 SELECT x2, y FROM t52816
+
+# Regression test for #54196.
+statement ok
+CREATE TABLE IF NOT EXISTS regression_54196 (
+	col1 int,
+	col2 int,
+	col3 int,
+	INDEX (col1, col2),
+	INDEX (col1, col3),
+	INDEX (col2, col3)
+); ALTER TABLE regression_54196 DROP COLUMN col1
+
+query TT
+SELECT index_name, column_name FROM [SHOW INDEXES FROM regression_54196]
+ORDER BY index_name, column_name ASC
+----
+primary                         rowid
+regression_54196_col2_col3_idx  col2
+regression_54196_col2_col3_idx  col3
+regression_54196_col2_col3_idx  rowid