From 5ec47b2330346ccafa671e0d729faf76ca5252fb Mon Sep 17 00:00:00 2001
From: Chris Seto <chriskseto@gmail.com>
Date: Wed, 2 Aug 2023 20:11:10 +0000
Subject: [PATCH] sql: emit NOTICEs when implicitly dropping indexes

Previously, users would have to either know ahead of time or later
discover if an index would be implicitly dropped by dropping a column
that the index referenced.

At best, this was mildly surprising. At worst, workloads could be
dramatically impacted without much indication as to why.

To ensure implicit drops do not go unnoticed, this commit updates `ALTER
TABLE ... DROP COLUMN ...` to emit NOTICEs for each implicitly dropped
index.

Fixes: #106907
Release note (sql change): NOTICEs are now emitted for each index
dropped by an `ALTER TABLE ... DROP COLUMN ...` statement.
---
 pkg/sql/alter_table.go                             |  5 +++++
 pkg/sql/logictest/testdata/logic_test/alter_table  | 14 ++++++++++----
 .../scbuildstmt/alter_table_drop_column.go         |  6 ++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
index 1d45d98116ac..ce87a06afb16 100644
--- a/pkg/sql/alter_table.go
+++ b/pkg/sql/alter_table.go
@@ -1718,6 +1718,11 @@ func dropColumnImpl(
 	}
 
 	for _, idxName := range idxNamesToDelete {
+		params.EvalContext().ClientNoticeSender.BufferClientNotice(params.ctx, pgnotice.Newf(
+			"dropping index %q which depends on column %q",
+			idxName,
+			colToDrop.ColName(),
+		))
 		jobDesc := fmt.Sprintf(
 			"removing index %q dependent on column %q which is being dropped; full details: %s",
 			idxName,
diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table
index 4bd7c58cedee..97337574603a 100644
--- a/pkg/sql/logictest/testdata/logic_test/alter_table
+++ b/pkg/sql/logictest/testdata/logic_test/alter_table
@@ -391,7 +391,9 @@ ALTER TABLE t DROP COLUMN IF EXISTS q
 statement error cannot drop column "e" because view "v" depends on it
 ALTER TABLE t DROP COLUMN IF EXISTS e
 
-statement ok
+# Negative test to assert that NOTICEs are NOT emitted if there are no indexes
+# being dropped.
+statement notice ^$
 ALTER TABLE t DROP COLUMN IF EXISTS e CASCADE
 
 statement ok
@@ -403,12 +405,16 @@ CREATE TABLE o (gf INT REFERENCES t (g), h INT, i INT, INDEX oi (i), INDEX oh (h
 statement error "t_g_key" is referenced by foreign key from table "o"
 ALTER TABLE t DROP COLUMN g
 
-statement ok
+statement notice NOTICE: dropping index "t_g_key" which depends on column "g"
 ALTER TABLE t DROP COLUMN g CASCADE
 
 # Dropping columns that are indexed or stored in indexes drops those indexes
-# too.
-statement ok
+# too. Excuse the nasty regex, the legacy schema changer emits additional
+# NOTICEs about GCing. We're not testing for that, so we just "eat" them.
+# Example: "NOTICE: the data for dropped indexes is reclaimed
+# asynchronously\nHINT: The reclamation delay can be customized in the zone
+# configuration for the table."
+statement notice (NOTICE: dropping index "(oh|oih)" which depends on column "h"(\nNOTICE: .+\nHINT: .+\n)?\n?){2}
 ALTER TABLE o DROP COLUMN h
 
 query TTBITTTBBBF colnames,rowsort
diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go
index fd2f0ffa7254..926ed33bfd96 100644
--- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go
+++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go
@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
@@ -227,6 +228,11 @@ func dropColumn(
 				Table: *tn,
 				Index: tree.UnrestrictedName(indexName.Name),
 			}
+			b.EvalCtx().ClientNoticeSender.BufferClientNotice(b, pgnotice.Newf(
+				"dropping index %q which depends on column %q",
+				indexName.Name,
+				cn.Name,
+			))
 			dropSecondaryIndex(b, &name, behavior, e)
 		case *scpb.View:
 			if behavior != tree.DropCascade {